Patchwork drm/i915: disable PCH ports if needed when disabling a CRTC

login
register
mail settings
Submitter Keng-Yu Lin
Date June 22, 2011, 1:49 a.m.
Message ID <1308707374-15692-2-git-send-email-kengyu@canonical.com>
Download mbox | patch
Permalink /patch/101450/
State New
Headers show

Comments

Keng-Yu Lin - June 22, 2011, 1:49 a.m.
From: Keng-Yu Lin <keng-yu.lin@canonical.com>


BugLink: http://bugs.launchpad.net/bugs/791752

This patch is tailored to apply on top of Ubuntu Natty kernel.

commit 47a05eca72991039e788b25232061f9c9df9ec23
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Mon Feb 7 13:46:40 2011 -0800

    drm/i915: disable PCH ports if needed when disabling a CRTC

    Disable any PCH ports associated with a pipe when disabling it.  This
    should prevent transcoder disable failures due to ports still being on.

    Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
    [ickle: introduce *_PIPE_ENABLED() macro]
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Signed-off-by: Keng-Yu Lin <keng-yu.lin@canonical.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |   15 ++++++++
 drivers/gpu/drm/i915/intel_display.c |   61 +++++++++++++++++++++++++++++----
 2 files changed, 68 insertions(+), 8 deletions(-)
Tim Gardner - June 22, 2011, 1:25 p.m.
On 06/21/2011 07:49 PM, Keng-Yu Lin wrote:
> disable PCH ports

Looks like a reasonable backport.

Acked-by: Tim Gardner <tim.gardner@canonical.com>
Seth Forshee - June 22, 2011, 1:58 p.m.
On Wed, Jun 22, 2011 at 09:49:34AM +0800, Keng-Yu Lin wrote:
> From: Keng-Yu Lin <keng-yu.lin@canonical.com>
> 
> 
> BugLink: http://bugs.launchpad.net/bugs/791752
> 
> This patch is tailored to apply on top of Ubuntu Natty kernel.
> 
> commit 47a05eca72991039e788b25232061f9c9df9ec23
> Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> Date:   Mon Feb 7 13:46:40 2011 -0800
> 
>     drm/i915: disable PCH ports if needed when disabling a CRTC
> 
>     Disable any PCH ports associated with a pipe when disabling it.  This
>     should prevent transcoder disable failures due to ports still being on.
> 
>     Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>     [ickle: introduce *_PIPE_ENABLED() macro]
>     Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Signed-off-by: Keng-Yu Lin <keng-yu.lin@canonical.com>

Backport looks good to me.

Acked-by: Seth Forshee <seth.forshee@canonical.com>
Leann Ogasawara - June 22, 2011, 2 p.m.
On Wed, 2011-06-22 at 09:49 +0800, Keng-Yu Lin wrote:
> From: Keng-Yu Lin <keng-yu.lin@canonical.com>
> 
> 
> BugLink: http://bugs.launchpad.net/bugs/791752
> 
> This patch is tailored to apply on top of Ubuntu Natty kernel.
> 
> commit 47a05eca72991039e788b25232061f9c9df9ec23
> Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> Date:   Mon Feb 7 13:46:40 2011 -0800
> 
>     drm/i915: disable PCH ports if needed when disabling a CRTC
> 
>     Disable any PCH ports associated with a pipe when disabling it.  This
>     should prevent transcoder disable failures due to ports still being on.
> 
>     Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>     [ickle: introduce *_PIPE_ENABLED() macro]
>     Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Signed-off-by: Keng-Yu Lin <keng-yu.lin@canonical.com>

Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h      |   15 ++++++++
>  drivers/gpu/drm/i915/intel_display.c |   61 +++++++++++++++++++++++++++++----
>  2 files changed, 68 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 12c547a..e7cb5df 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1444,6 +1444,7 @@
>  #define   LVDS_PORT_EN			(1 << 31)
>  /* Selects pipe B for LVDS data.  Must be set on pre-965. */
>  #define   LVDS_PIPEB_SELECT		(1 << 30)
> +#define   LVDS_PIPE_MASK		(1 << 30)
>  /* LVDS dithering flag on 965/g4x platform */
>  #define   LVDS_ENABLE_DITHER		(1 << 25)
>  /* Enable border for unscaled (or aspect-scaled) display */
> @@ -1479,6 +1480,9 @@
>  #define   LVDS_B0B3_POWER_DOWN		(0 << 2)
>  #define   LVDS_B0B3_POWER_UP		(3 << 2)
>  
> +#define LVDS_PIPE_ENABLED(V, P) \
> +	(((V) & (LVDS_PIPE_MASK | LVDS_PORT_EN)) == ((P) << 30 | LVDS_PORT_EN))
> +
>  /* Video Data Island Packet control */
>  #define VIDEO_DIP_DATA		0x61178
>  #define VIDEO_DIP_CTL		0x61170
> @@ -2067,6 +2071,10 @@
>  
>  #define   DP_PORT_EN			(1 << 31)
>  #define   DP_PIPEB_SELECT		(1 << 30)
> +#define   DP_PIPE_MASK			(1 << 30)
> +
> +#define DP_PIPE_ENABLED(V, P) \
> +	(((V) & (DP_PIPE_MASK | DP_PORT_EN)) == ((P) << 30 | DP_PORT_EN))
>  
>  /* Link training mode - select a suitable mode for each stage */
>  #define   DP_LINK_TRAIN_PAT_1		(0 << 28)
> @@ -3148,11 +3156,15 @@
>  #define  ADPA_CRT_HOTPLUG_VOLREF_475MV  (1<<17)
>  #define  ADPA_CRT_HOTPLUG_FORCE_TRIGGER (1<<16)
>  
> +#define ADPA_PIPE_ENABLED(V, P) \
> +	(((V) & (ADPA_TRANS_SELECT_MASK | ADPA_DAC_ENABLE)) == ((P) << 30 | ADPA_DAC_ENABLE))
> +
>  /* or SDVOB */
>  #define HDMIB   0xe1140
>  #define  PORT_ENABLE    (1 << 31)
>  #define  TRANSCODER_A   (0)
>  #define  TRANSCODER_B   (1 << 30)
> +#define  TRANSCODER_MASK   (1 << 30)
>  #define  COLOR_FORMAT_8bpc      (0)
>  #define  COLOR_FORMAT_12bpc     (3 << 26)
>  #define  SDVOB_HOTPLUG_ENABLE   (1 << 23)
> @@ -3168,6 +3180,9 @@
>  #define  HSYNC_ACTIVE_HIGH      (1 << 3)
>  #define  PORT_DETECTED          (1 << 2)
>  
> +#define HDMI_PIPE_ENABLED(V, P) \
> +	(((V) & (TRANSCODER_MASK | PORT_ENABLE)) == ((P) << 30 | PORT_ENABLE))
> +
>  /* PCH SDVOB multiplex with HDMIB */
>  #define PCH_SDVOB	HDMIB
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 841558b..88dc3fe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1058,6 +1058,53 @@ void intel_wait_for_pipe_off(struct drm_device *dev, int pipe)
>  	}
>  }
>  
> +static void disable_pch_dp(struct drm_i915_private *dev_priv,
> +			   enum pipe pipe, int reg)
> +{
> +	u32 val = I915_READ(reg);
> +	if (DP_PIPE_ENABLED(val, pipe))
> +		I915_WRITE(reg, val & ~DP_PORT_EN);
> +}
> +
> +static void disable_pch_hdmi(struct drm_i915_private *dev_priv,
> +			     enum pipe pipe, int reg)
> +{
> +	u32 val = I915_READ(reg);
> +	if (HDMI_PIPE_ENABLED(val, pipe))
> +		I915_WRITE(reg, val & ~PORT_ENABLE);
> +}
> +
> +/* Disable any ports connected to this transcoder */
> +static void intel_disable_pch_ports(struct drm_i915_private *dev_priv,
> +				    enum pipe pipe)
> +{
> +	u32 reg, val;
> +
> +	val = I915_READ(PCH_PP_CONTROL);
> +	I915_WRITE(PCH_PP_CONTROL, val | PANEL_UNLOCK_REGS);
> +
> +	disable_pch_dp(dev_priv, pipe, PCH_DP_B);
> +	disable_pch_dp(dev_priv, pipe, PCH_DP_C);
> +	disable_pch_dp(dev_priv, pipe, PCH_DP_D);
> +
> +	reg = PCH_ADPA;
> +	val = I915_READ(reg);
> +	if (ADPA_PIPE_ENABLED(val, pipe))
> +		I915_WRITE(reg, val & ~ADPA_DAC_ENABLE);
> +
> +	reg = PCH_LVDS;
> +	val = I915_READ(reg);
> +	if (LVDS_PIPE_ENABLED(val, pipe)) {
> +		I915_WRITE(reg, val & ~LVDS_PORT_EN);
> +		POSTING_READ(reg);
> +		udelay(100);
> +	}
> +
> +	disable_pch_hdmi(dev_priv, pipe, HDMIB);
> +	disable_pch_hdmi(dev_priv, pipe, HDMIC);
> +	disable_pch_hdmi(dev_priv, pipe, HDMID);
> +}
> +
>  static void i8xx_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>  {
>  	struct drm_device *dev = crtc->dev;
> @@ -2356,14 +2403,12 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  	POSTING_READ(reg);
>  	udelay(100);
>  
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> -		temp = I915_READ(PCH_LVDS);
> -		if (temp & LVDS_PORT_EN) {
> -			I915_WRITE(PCH_LVDS, temp & ~LVDS_PORT_EN);
> -			POSTING_READ(PCH_LVDS);
> -			udelay(100);
> -		}
> -	}
> +	/* This is a horrible layering violation; we should be doing this in
> +	 * the connector/encoder ->prepare instead, but we don't always have
> +	 * enough information there about the config to know whether it will
> +	 * actually be necessary or just cause undesired flicker.
> +	 */
> +	intel_disable_pch_ports(dev_priv, pipe);
>  
>  	/* disable PCH transcoder */
>  	reg = TRANSCONF(plane);
> -- 
> 1.7.4.1
> 
>

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 12c547a..e7cb5df 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1444,6 +1444,7 @@ 
 #define   LVDS_PORT_EN			(1 << 31)
 /* Selects pipe B for LVDS data.  Must be set on pre-965. */
 #define   LVDS_PIPEB_SELECT		(1 << 30)
+#define   LVDS_PIPE_MASK		(1 << 30)
 /* LVDS dithering flag on 965/g4x platform */
 #define   LVDS_ENABLE_DITHER		(1 << 25)
 /* Enable border for unscaled (or aspect-scaled) display */
@@ -1479,6 +1480,9 @@ 
 #define   LVDS_B0B3_POWER_DOWN		(0 << 2)
 #define   LVDS_B0B3_POWER_UP		(3 << 2)
 
+#define LVDS_PIPE_ENABLED(V, P) \
+	(((V) & (LVDS_PIPE_MASK | LVDS_PORT_EN)) == ((P) << 30 | LVDS_PORT_EN))
+
 /* Video Data Island Packet control */
 #define VIDEO_DIP_DATA		0x61178
 #define VIDEO_DIP_CTL		0x61170
@@ -2067,6 +2071,10 @@ 
 
 #define   DP_PORT_EN			(1 << 31)
 #define   DP_PIPEB_SELECT		(1 << 30)
+#define   DP_PIPE_MASK			(1 << 30)
+
+#define DP_PIPE_ENABLED(V, P) \
+	(((V) & (DP_PIPE_MASK | DP_PORT_EN)) == ((P) << 30 | DP_PORT_EN))
 
 /* Link training mode - select a suitable mode for each stage */
 #define   DP_LINK_TRAIN_PAT_1		(0 << 28)
@@ -3148,11 +3156,15 @@ 
 #define  ADPA_CRT_HOTPLUG_VOLREF_475MV  (1<<17)
 #define  ADPA_CRT_HOTPLUG_FORCE_TRIGGER (1<<16)
 
+#define ADPA_PIPE_ENABLED(V, P) \
+	(((V) & (ADPA_TRANS_SELECT_MASK | ADPA_DAC_ENABLE)) == ((P) << 30 | ADPA_DAC_ENABLE))
+
 /* or SDVOB */
 #define HDMIB   0xe1140
 #define  PORT_ENABLE    (1 << 31)
 #define  TRANSCODER_A   (0)
 #define  TRANSCODER_B   (1 << 30)
+#define  TRANSCODER_MASK   (1 << 30)
 #define  COLOR_FORMAT_8bpc      (0)
 #define  COLOR_FORMAT_12bpc     (3 << 26)
 #define  SDVOB_HOTPLUG_ENABLE   (1 << 23)
@@ -3168,6 +3180,9 @@ 
 #define  HSYNC_ACTIVE_HIGH      (1 << 3)
 #define  PORT_DETECTED          (1 << 2)
 
+#define HDMI_PIPE_ENABLED(V, P) \
+	(((V) & (TRANSCODER_MASK | PORT_ENABLE)) == ((P) << 30 | PORT_ENABLE))
+
 /* PCH SDVOB multiplex with HDMIB */
 #define PCH_SDVOB	HDMIB
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 841558b..88dc3fe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1058,6 +1058,53 @@  void intel_wait_for_pipe_off(struct drm_device *dev, int pipe)
 	}
 }
 
+static void disable_pch_dp(struct drm_i915_private *dev_priv,
+			   enum pipe pipe, int reg)
+{
+	u32 val = I915_READ(reg);
+	if (DP_PIPE_ENABLED(val, pipe))
+		I915_WRITE(reg, val & ~DP_PORT_EN);
+}
+
+static void disable_pch_hdmi(struct drm_i915_private *dev_priv,
+			     enum pipe pipe, int reg)
+{
+	u32 val = I915_READ(reg);
+	if (HDMI_PIPE_ENABLED(val, pipe))
+		I915_WRITE(reg, val & ~PORT_ENABLE);
+}
+
+/* Disable any ports connected to this transcoder */
+static void intel_disable_pch_ports(struct drm_i915_private *dev_priv,
+				    enum pipe pipe)
+{
+	u32 reg, val;
+
+	val = I915_READ(PCH_PP_CONTROL);
+	I915_WRITE(PCH_PP_CONTROL, val | PANEL_UNLOCK_REGS);
+
+	disable_pch_dp(dev_priv, pipe, PCH_DP_B);
+	disable_pch_dp(dev_priv, pipe, PCH_DP_C);
+	disable_pch_dp(dev_priv, pipe, PCH_DP_D);
+
+	reg = PCH_ADPA;
+	val = I915_READ(reg);
+	if (ADPA_PIPE_ENABLED(val, pipe))
+		I915_WRITE(reg, val & ~ADPA_DAC_ENABLE);
+
+	reg = PCH_LVDS;
+	val = I915_READ(reg);
+	if (LVDS_PIPE_ENABLED(val, pipe)) {
+		I915_WRITE(reg, val & ~LVDS_PORT_EN);
+		POSTING_READ(reg);
+		udelay(100);
+	}
+
+	disable_pch_hdmi(dev_priv, pipe, HDMIB);
+	disable_pch_hdmi(dev_priv, pipe, HDMIC);
+	disable_pch_hdmi(dev_priv, pipe, HDMID);
+}
+
 static void i8xx_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 {
 	struct drm_device *dev = crtc->dev;
@@ -2356,14 +2403,12 @@  static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	POSTING_READ(reg);
 	udelay(100);
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
-		temp = I915_READ(PCH_LVDS);
-		if (temp & LVDS_PORT_EN) {
-			I915_WRITE(PCH_LVDS, temp & ~LVDS_PORT_EN);
-			POSTING_READ(PCH_LVDS);
-			udelay(100);
-		}
-	}
+	/* This is a horrible layering violation; we should be doing this in
+	 * the connector/encoder ->prepare instead, but we don't always have
+	 * enough information there about the config to know whether it will
+	 * actually be necessary or just cause undesired flicker.
+	 */
+	intel_disable_pch_ports(dev_priv, pipe);
 
 	/* disable PCH transcoder */
 	reg = TRANSCONF(plane);