Re: [PATCH] drm/i915: add missing "break"

[Keith Packard <keithp@keithp.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5712 bytes --]

On Thu, 22 Sep 2011 15:13:42 -0300, przanoni@gmail.com wrote:

> It seems to be missing from this commit:
>   "drm/i915: split out PCH refclk update code"

Oh, this code is missing far more than that. It doesn't deal with the
LVDS case at all.

Here's a patch which turns on the right bits of SSC, in the right order,
for any combination of CPU-eDP, PCH-eDP and LVDS.

Jesse: you wrote this stuff, can you review what I did? Also, my two SNB
systems (X220 and MacBook Air) both have a BIOS table that disables SSC,
is there any reason for us to believe the BIOS table on a PCH system?
Can't we always use SSC?

I've tested this on LVDS and CPU-connected eDP with and without SSC and
it seems to work for me.

From ea63d98df915f81c80cf4f91bf9d1bcaece7fce0 Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Thu, 22 Sep 2011 12:01:57 -0700
Subject: [PATCH 1/3] drm/i915: Fix PCH SSC reference clock settings

The PCH refclk settings are global, so we need to look at all of the
encoders, not just the current encoder when deciding how to configure
it. Also, handle systems with more than one panel (any combination of
PCH/non-PCH eDP and LVDS).

Disable SSC clocks when no panels are connected.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_display.c |   88 ++++++++++++++++++++++++---------
 1 files changed, 64 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 04411ad..01a52fc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5113,9 +5113,11 @@ static void ironlake_update_pch_refclk(struct drm_device *dev)
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	struct drm_crtc *crtc;
 	struct intel_encoder *encoder;
-	struct intel_encoder *has_edp_encoder = NULL;
 	u32 temp;
 	bool has_lvds = false;
+	bool has_cpu_edp = false;
+	bool has_pch_edp = false;
+	bool has_panel = false;
 
 	/* We need to take the global config into account */
 	list_for_each_entry(crtc, &mode_config->crtc_list, head) {
@@ -5124,18 +5126,23 @@ static void ironlake_update_pch_refclk(struct drm_device *dev)
 
 		list_for_each_entry(encoder, &mode_config->encoder_list,
 				    base.head) {
-			if (encoder->base.crtc != crtc)
-				continue;
-
 			switch (encoder->type) {
 			case INTEL_OUTPUT_LVDS:
+				has_panel = true;
 				has_lvds = true;
+				break;
 			case INTEL_OUTPUT_EDP:
-				has_edp_encoder = encoder;
+				has_panel = true;
+				if (intel_encoder_is_pch_edp(&encoder->base))
+					has_pch_edp = true;
+				else
+					has_cpu_edp = true;
 				break;
 			}
 		}
 	}
+	DRM_DEBUG_KMS("has_panel %d has_lvds %d has_pch_edp %d has_cpu_edp %d\n",
+		      has_panel, has_lvds, has_pch_edp, has_cpu_edp);
 
 	/* Ironlake: try to setup display ref clock before DPLL
 	 * enabling. This is only under driver's control after
@@ -5146,36 +5153,69 @@ static void ironlake_update_pch_refclk(struct drm_device *dev)
 	/* Always enable nonspread source */
 	temp &= ~DREF_NONSPREAD_SOURCE_MASK;
 	temp |= DREF_NONSPREAD_SOURCE_ENABLE;
-	temp &= ~DREF_SSC_SOURCE_MASK;
-	temp |= DREF_SSC_SOURCE_ENABLE;
-	I915_WRITE(PCH_DREF_CONTROL, temp);
 
-	POSTING_READ(PCH_DREF_CONTROL);
-	udelay(200);
+	if (has_panel) {
+		temp &= ~DREF_SSC_SOURCE_MASK;
+		temp |= DREF_SSC_SOURCE_ENABLE;
 
-	if (has_edp_encoder) {
+		/* SSC must be turned on before enabling the CPU output  */
 		if (intel_panel_use_ssc(dev_priv)) {
+			DRM_DEBUG_KMS("Using SSC on panel\n");
 			temp |= DREF_SSC1_ENABLE;
-			I915_WRITE(PCH_DREF_CONTROL, temp);
-
-			POSTING_READ(PCH_DREF_CONTROL);
-			udelay(200);
 		}
+
+		/* Get SSC going before enabling the outputs */
+		I915_WRITE(PCH_DREF_CONTROL, temp);
+		POSTING_READ(PCH_DREF_CONTROL);
+		udelay(200);
+
 		temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
+		temp &= ~DREF_SUPERSPREAD_SOURCE_MASK;
 
 		/* Enable CPU source on CPU attached eDP */
-		if (!intel_encoder_is_pch_edp(&has_edp_encoder->base)) {
-			if (intel_panel_use_ssc(dev_priv))
+		if (has_cpu_edp) {
+			if (intel_panel_use_ssc(dev_priv)) {
+				DRM_DEBUG_KMS("Using SSC on eDP\n");
 				temp |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
+			}
 			else
 				temp |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
-		} else {
-			/* Enable SSC on PCH eDP if needed */
-			if (intel_panel_use_ssc(dev_priv)) {
-				DRM_ERROR("enabling SSC on PCH\n");
-				temp |= DREF_SUPERSPREAD_SOURCE_ENABLE;
-			}
-		}
+		} else
+			temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
+
+		/* Enable SSC on PCH eDP or LVDS if needed */
+		if ((has_pch_edp || has_lvds) && intel_panel_use_ssc(dev_priv)) {
+			DRM_DEBUG_KMS("Using SSC on PCH eDP or LVDS\n");
+			temp |= DREF_SUPERSPREAD_SOURCE_ENABLE;
+		} else
+			temp |= DREF_SUPERSPREAD_SOURCE_DISABLE;
+
+		I915_WRITE(PCH_DREF_CONTROL, temp);
+		POSTING_READ(PCH_DREF_CONTROL);
+		udelay(200);
+	} else {
+		temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
+		temp &= ~DREF_SUPERSPREAD_SOURCE_MASK;
+
+		DRM_DEBUG_KMS("Disabling SSC entirely\n");
+
+		/* Turn off CPU output */
+		temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
+
+		/* Disable SSC on PCH */
+		temp |= DREF_SUPERSPREAD_SOURCE_DISABLE;
+
+		I915_WRITE(PCH_DREF_CONTROL, temp);
+		POSTING_READ(PCH_DREF_CONTROL);
+		udelay(200);
+
+		/* Turn off the SSC source */
+		temp &= ~DREF_SSC_SOURCE_MASK;
+		temp |= DREF_SSC_SOURCE_DISABLE;
+
+		/* Turn off SSC1 */
+		temp &= ~ DREF_SSC1_ENABLE;
+
 		I915_WRITE(PCH_DREF_CONTROL, temp);
 		POSTING_READ(PCH_DREF_CONTROL);
 		udelay(200);
-- 
1.7.6.3




-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx