diff mbox

[01/10] drm/i915: rewrite the LCPLL code

Message ID 1349449561-3599-2-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni Oct. 5, 2012, 3:05 p.m. UTC 
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Right now, we're trying to enable LCPLL at every mode set, but we're
never disabling it. Also, we really don't want to be disabling LCPLL
since it requires a very complex disable/enable sequence. This
register should really be set by the BIOS and we shouldn't be touching
it. Still, let's try to check its value and print some errors in case
we find something wrong. We're also adding intel_ddi_get_cdclk_freq
which will be used later in other places.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  6 ++++++
 drivers/gpu/drm/i915/intel_ddi.c     | 37 ++++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_display.c |  7 +++++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 4 files changed, 45 insertions(+), 6 deletions(-)

Comments

Lespiau, Damien Oct. 10, 2012, 12:47 p.m. UTC | #1 
On Fri, Oct 5, 2012 at 4:05 PM, Paulo Zanoni <przanoni@gmail.com> wrote:

> +static int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
> +{
> +       if (I915_READ(HSW_FUSE_STRAP) & HSW_CDCLK_LIMIT)
> +               return 450;

I don't think reading this fused register is necessary here, it only
really matters when you try to set the LCPLL alternate frequency to
check if the hardware supports it. I'd just rely on the frequency set
bits in LCPLL (if you remove this, it means that the hunk adding the
register definition is not needed any more).

If you really want to check for consistency, then you could warn if
the LCPLL freq has been programmed with 0x1 and HSW_CDCLK_LIMIT is set
(in which case the setting will be ignored). But then, there's nothing
we can do anyway, so well...
Lespiau, Damien Oct. 10, 2012, 12:53 p.m. UTC | #2 
On Wed, Oct 10, 2012 at 1:47 PM, Lespiau, Damien
<damien.lespiau@intel.com> wrote:
> On Fri, Oct 5, 2012 at 4:05 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
>
>> +static int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
>> +{
>> +       if (I915_READ(HSW_FUSE_STRAP) & HSW_CDCLK_LIMIT)
>> +               return 450;
>
> I don't think reading this fused register is necessary here, it only
> really matters when you try to set the LCPLL alternate frequency to
> check if the hardware supports it. I'd just rely on the frequency set
> bits in LCPLL (if you remove this, it means that the hunk adding the
> register definition is not needed any more).
>
> If you really want to check for consistency, then you could warn if
> the LCPLL freq has been programmed with 0x1 and HSW_CDCLK_LIMIT is set
> (in which case the setting will be ignored). But then, there's nothing
> we can do anyway, so well...

Hum, I meant to add, it's just bike-shedding really, it's totally fine
if you want to return 450 if HSW_CDCLK_LIMIT is set without checking
for the programmed value in LCPLL. So really:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f3a06b4..5107cee 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3456,6 +3456,9 @@ 
 #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG		0x9030
 #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB	(1<<11)
 
+#define HSW_FUSE_STRAP		0x42014
+#define  HSW_CDCLK_LIMIT	(1 << 24)
+
 /* PCH */
 
 /* south display engine interrupt: IBX */
@@ -4531,8 +4534,11 @@ 
 #define LCPLL_CTL			0x130040
 #define  LCPLL_PLL_DISABLE		(1<<31)
 #define  LCPLL_PLL_LOCK			(1<<30)
+#define  LCPLL_CLK_FREQ_MASK		(3<<26)
+#define  LCPLL_CLK_FREQ_450		(0<<26)
 #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
 #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
+#define  LCPLL_CD_SOURCE_FCLK		(1<<21)
 
 /* Pipe WM_LINETIME - watermark line time */
 #define PIPE_WM_LINETIME_A		0x45270
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index bfe3754..d912dbf 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -682,12 +682,6 @@  void intel_ddi_mode_set(struct drm_encoder *encoder,
 	DRM_DEBUG_KMS("WR PLL: %dKHz refresh rate with p=%d, n2=%d r2=%d\n",
 		      crtc->mode.clock, p, n2, r2);
 
-	/* Enable LCPLL if disabled */
-	temp = I915_READ(LCPLL_CTL);
-	if (temp & LCPLL_PLL_DISABLE)
-		I915_WRITE(LCPLL_CTL,
-				temp & ~LCPLL_PLL_DISABLE);
-
 	/* Configure WR PLL 1, program the correct divider values for
 	 * the desired frequency and wait for warmup */
 	I915_WRITE(WRPLL_CTL1,
@@ -817,3 +811,34 @@  void intel_disable_ddi(struct intel_encoder *encoder)
 
 	I915_WRITE(DDI_BUF_CTL(port), temp);
 }
+
+static int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
+{
+	if (I915_READ(HSW_FUSE_STRAP) & HSW_CDCLK_LIMIT)
+		return 450;
+	else if ((I915_READ(LCPLL_CTL) & LCPLL_CLK_FREQ_MASK) ==
+		 LCPLL_CLK_FREQ_450)
+		return 450;
+	else
+		return 540;
+}
+
+void intel_ddi_pll_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t val = I915_READ(LCPLL_CTL);
+
+	/* The LCPLL register should be turned on by the BIOS. For now let's
+	 * just check its state and print errors in case something is wrong.
+	 * Don't even try to turn it on.
+	 */
+
+	DRM_DEBUG_KMS("CDCLK running at %dMHz\n",
+		      intel_ddi_get_cdclk_freq(dev_priv));
+
+	if (val & LCPLL_CD_SOURCE_FCLK)
+		DRM_ERROR("CDCLK source is not LCPLL\n");
+
+	if (val & LCPLL_PLL_DISABLE)
+		DRM_ERROR("LCPLL is disabled\n");
+}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6cf0d00..40f98d1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7477,6 +7477,12 @@  static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.page_flip = intel_crtc_page_flip,
 };
 
+static void intel_cpu_pll_init(struct drm_device *dev)
+{
+	if (IS_HASWELL(dev))
+		intel_ddi_pll_init(dev);
+}
+
 static void intel_pch_pll_init(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
@@ -8085,6 +8091,7 @@  void intel_modeset_init(struct drm_device *dev)
 			DRM_DEBUG_KMS("plane %d init failed: %d\n", i, ret);
 	}
 
+	intel_cpu_pll_init(dev);
 	intel_pch_pll_init(dev);
 
 	/* Just disable it once at startup */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 79f8ed6..57566b7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -580,5 +580,6 @@  extern bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 extern void intel_ddi_mode_set(struct drm_encoder *encoder,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode);
+extern void intel_ddi_pll_init(struct drm_device *dev);
 
 #endif /* __INTEL_DRV_H__ */