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

Commit Message

Paulo Zanoni Oct. 2, 2012, 8:51 p.m. UTC

From: Paulo Zanoni <[e-mail]>

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 sequence. So instead of
enabling it at every mode set, enable it once.

Also, we are currently not checking if the desired values are the ones
we're actually reading/writing, so add some code to check the values
and give us warning messages.

Since the disable/enable sequence is very complex, I am not sure that
the single I915_WRITE we have is enough, but keep it for now to avoid
regressions since everybody's machines seem to be working.

Signed-off-by: Paulo Zanoni <[e-mail]>
---
 drivers/gpu/drm/i915/i915_reg.h      |    7 ++++++
 drivers/gpu/drm/i915/intel_ddi.c     |   46 +++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_display.c |    7 ++++++
 drivers/gpu/drm/i915/intel_drv.h     |    1 +
 4 files changed, 55 insertions(+), 6 deletions(-)

Comments

Lespiau, Damien Oct. 3, 2012, 4:47 p.m. UTC | #1

On Tue, Oct 2, 2012 at 9:51 PM, Paulo Zanoni <[e-mail]> wrote:
> From: Paulo Zanoni <[e-mail]>
> +       temp = lcpll_val & LCPLL_CLK_FREQ_MASK;
> +       if ((clk_val == 449 && (temp != LCPLL_CLK_FREQ_450)) ||
> +           (clk_val == 539 && (temp != LCPLL_CLK_FREQ_540))) {
> +               DRM_ERROR("LCPLL and CDCLK frequencies don't match\n");
> +               lcpll_needs_change = true;
> +
> +               lcpll_val &= ~LCPLL_CLK_FREQ_MASK;
> +               if (clk_val == 449)
> +                       lcpll_val |= LCPLL_CLK_FREQ_450;
> +               else
> +                       lcpll_val |= LCPLL_CLK_FREQ_540;
> +       }

This fixup code looks weird to me, have you encountered cases where it
makes it work or is it "just in case"? it's "weird" to me for the
following reasons:
  - It tries to adapt the LCPLL freq depending on the value programmed
in CDCLOCK, but CDCLOCK derives from LCPLL, should not fixup code  try
to do the opposite then (if we want to try to do that at all)?
  - the 540MHz clock is only available is FUSE_STRAP bit 24 is not
set, I guess we'd want to test that

> +       if (lcpll_val & LCPLL_PLL_DISABLE) {
> +               DRM_ERROR("LCPLL is disabled\n");
> +               lcpll_needs_change = true;
> +               lcpll_val &= ~LCPLL_PLL_DISABLE;
> +       }
> +
> +       if (lcpll_needs_change)
> +               I915_WRITE(LCPLL_CTL, lcpll_val);

Are we sure that enabling LCPLL has ever done something? if the driver
would have to enable LCPLL, you'd have to ensure that CDCLOCK is
enabled as well for instance. My gut feeling is that this BIOS domain.

I think I'd just try to drop the initial hunk instead of rewriting it,
but what do I know :p

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d17bef7..ab96706 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4527,8 +4527,15 @@ 
 #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_CLK_FREQ_540		(1<<26)
 #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
 #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
+#define  LCPLL_CD_SOURCE_FCLK		(1<<21)
+
+#define CDCLK_FREQ			0x46200
+#define  CDCLK_FREQ_MASK		0x3FF
 
 /* 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..34a73a1 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,43 @@  void intel_disable_ddi(struct intel_encoder *encoder)
 
 	I915_WRITE(DDI_BUF_CTL(port), temp);
 }
+
+void intel_ddi_pll_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t lcpll_val, clk_val, temp;
+	bool lcpll_needs_change = false;
+
+	/* Check the LCPLL state and fix it if needed. */
+	lcpll_val = I915_READ(LCPLL_CTL);
+	clk_val = I915_READ(CDCLK_FREQ) & CDCLK_FREQ_MASK;
+	DRM_DEBUG_KMS("CDCLK running at %dMHz\n", clk_val + 1);
+
+	temp = lcpll_val & LCPLL_CLK_FREQ_MASK;
+	if ((clk_val == 449 && (temp != LCPLL_CLK_FREQ_450)) ||
+	    (clk_val == 539 && (temp != LCPLL_CLK_FREQ_540))) {
+		DRM_ERROR("LCPLL and CDCLK frequencies don't match\n");
+		lcpll_needs_change = true;
+
+		lcpll_val &= ~LCPLL_CLK_FREQ_MASK;
+		if (clk_val == 449)
+			lcpll_val |= LCPLL_CLK_FREQ_450;
+		else
+			lcpll_val |= LCPLL_CLK_FREQ_540;
+	}
+
+	if (lcpll_val & LCPLL_CD_SOURCE_FCLK) {
+		DRM_ERROR("CDCLK source is not LCPLL\n");
+		lcpll_needs_change = true;
+		lcpll_val &= ~LCPLL_CD_SOURCE_FCLK;
+	}
+
+	if (lcpll_val & LCPLL_PLL_DISABLE) {
+		DRM_ERROR("LCPLL is disabled\n");
+		lcpll_needs_change = true;
+		lcpll_val &= ~LCPLL_PLL_DISABLE;
+	}
+
+	if (lcpll_needs_change)
+		I915_WRITE(LCPLL_CTL, lcpll_val);
+}
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__ */