drm/i915: A selection of display port fixes

drm/i915: A selection of display port fixes

[Keith Packard]

 [PATCH 1/5] drm/i915: Use dp_detect_common in hotplug helper
 [PATCH 2/5] drm/i915: Rename i915_dp_detect_common to
 [PATCH 3/5] drm/i915: In intel_dp_init, replace read of DPCD with

These three are simple cleanups to centralize all places where the
DPCD block was read from the device. Now everyone shares the same
function, and that function retries the reads.

 [PATCH 4/5] drm/i915: Delay 250ms before running the hotplug code

I was experimenting with DP hotplugging -- moving the plug in and out
of the jack very slowly and discovered that the hotplug interrupt
occurred well before or after the link for the aux data channel was
connected or disconnected. The result of this was that a sufficiently
rapid cycle back through user mode could easily beat the motion of the
plug and cause the hotplug detection to get the wrong status. Sticking
a 250ms delay before doing anything gives the user sufficient time to
actually get the plug connected or disconnected.

 [PATCH 5/5] drm/i915: DP_PIPE_ENABLED must check transcoder on CPT

This is a fairly nice bug fix to finally have. The symptom I saw was
that going from one two-head configuration to another would sometimes
turn off the monitor which was *not* being modified. For instance, I
would do:

 $ xrandr --output LVDS1 --below DP2

This would always turn off the DP2 monitor, and sometimes it wouldn't
turn back on.

Turns out the bug wasn't that the mode setting code was doing it wrong
and turning the DP2 output off intentionally as a part of the mode
change. Instead, the intel driver was trying to adjust the PCH link
for the LVDS1 output and thought it needed to turn the DP2 output off
because it mistakenly believed the DP2 output was sharing the same
pipe as the LVDS1 output. Just a matter of using the wrong mechanism
to detect which pipe the DP2 output was connected to.

In any case, review and testing appreciated.

-keith

[PATCH 1/5] drm/i915: Use dp_detect_common in hotplug helper function

[Keith Packard]

This uses the common dpcd reading routine, i915_dp_detect_common,
instead of open-coding a call to intel_dp_aux_native_read. Besides
reducing duplicated code, this also gains the read retries which
may be necessary when a cable is first plugged back in and the link
needs to be retrained.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   39 +++++++++++++++++++--------------------
 1 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dcc7ae6..45db810 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1567,6 +1567,20 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 	POSTING_READ(intel_dp->output_reg);
 }
 
+static enum drm_connector_status
+i915_dp_detect_common(struct intel_dp *intel_dp)
+{
+	enum drm_connector_status status = connector_status_disconnected;
+
+	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
+					   sizeof (intel_dp->dpcd)) &&
+	    (intel_dp->dpcd[DP_DPCD_REV] != 0)) {
+		status = connector_status_connected;
+	}
+
+	return status;
+}
+
 /*
  * According to DP spec
  * 5.1.2:
@@ -1579,45 +1593,30 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 static void
 intel_dp_check_link_status(struct intel_dp *intel_dp)
 {
-	int ret;
-
 	if (!intel_dp->base.base.crtc)
 		return;
 
+	/* Try to read receiver status if the link appears to be up */
 	if (!intel_dp_get_link_status(intel_dp)) {
 		intel_dp_link_down(intel_dp);
 		return;
 	}
 
-	/* Try to read receiver status if the link appears to be up */
-	ret = intel_dp_aux_native_read(intel_dp,
-				       0x000, intel_dp->dpcd,
-				       sizeof (intel_dp->dpcd));
-	if (ret != sizeof(intel_dp->dpcd)) {
+	/* Now read the DPCD to see if it's actually running */
+	if (i915_dp_detect_common(intel_dp) != connector_status_connected) {
 		intel_dp_link_down(intel_dp);
 		return;
 	}
 
 	if (!intel_channel_eq_ok(intel_dp)) {
+		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
+			      drm_get_encoder_name(&intel_dp->base.base));
 		intel_dp_start_link_train(intel_dp);
 		intel_dp_complete_link_train(intel_dp);
 	}
 }
 
 static enum drm_connector_status
-i915_dp_detect_common(struct intel_dp *intel_dp)
-{
-	enum drm_connector_status status = connector_status_disconnected;
-
-	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
-					   sizeof (intel_dp->dpcd)) &&
-	    (intel_dp->dpcd[DP_DPCD_REV] != 0))
-		status = connector_status_connected;
-
-	return status;
-}
-
-static enum drm_connector_status
 ironlake_dp_detect(struct intel_dp *intel_dp)
 {
 	enum drm_connector_status status;
-- 
1.7.5.4

[PATCH 2/5] drm/i915: Rename i915_dp_detect_common to intel_dp_get_dpcd

[Keith Packard]

This describes the function better, allowing it to be used where the
DPCD value is relevant.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 45db810..41674e1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1567,18 +1567,16 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 	POSTING_READ(intel_dp->output_reg);
 }
 
-static enum drm_connector_status
-i915_dp_detect_common(struct intel_dp *intel_dp)
+static bool
+intel_dp_get_dpcd(struct intel_dp *intel_dp)
 {
-	enum drm_connector_status status = connector_status_disconnected;
-
 	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
 					   sizeof (intel_dp->dpcd)) &&
 	    (intel_dp->dpcd[DP_DPCD_REV] != 0)) {
-		status = connector_status_connected;
+		return true;
 	}
 
-	return status;
+	return false;
 }
 
 /*
@@ -1603,7 +1601,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 	}
 
 	/* Now read the DPCD to see if it's actually running */
-	if (i915_dp_detect_common(intel_dp) != connector_status_connected) {
+	if (!intel_dp_get_dpcd(intel_dp)) {
 		intel_dp_link_down(intel_dp);
 		return;
 	}
@@ -1617,6 +1615,14 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 }
 
 static enum drm_connector_status
+intel_dp_detect_dpcd(struct intel_dp *intel_dp)
+{
+	if (intel_dp_get_dpcd(intel_dp))
+		return connector_status_connected;
+	return connector_status_disconnected;
+}
+
+static enum drm_connector_status
 ironlake_dp_detect(struct intel_dp *intel_dp)
 {
 	enum drm_connector_status status;
@@ -1629,7 +1635,7 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
 		return status;
 	}
 
-	return i915_dp_detect_common(intel_dp);
+	return intel_dp_detect_dpcd(intel_dp);
 }
 
 static enum drm_connector_status
@@ -1658,7 +1664,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 	if ((temp & bit) == 0)
 		return connector_status_disconnected;
 
-	return i915_dp_detect_common(intel_dp);
+	return intel_dp_detect_dpcd(intel_dp);
 }
 
 /**
-- 
1.7.5.4

[PATCH 3/5] drm/i915: In intel_dp_init, replace read of DPCD with intel_dp_get_dpcd

[Keith Packard]

Eliminates an open-coded read and also gains the retry behaviour of
intel_dp_get_dpcd, which seems like a good idea.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_dp.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 41674e1..9f134d2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2015,7 +2015,7 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 
 	/* Cache some DPCD data in the eDP case */
 	if (is_edp(intel_dp)) {
-		int ret;
+		bool ret;
 		u32 pp_on, pp_div;
 
 		pp_on = I915_READ(PCH_PP_ON_DELAYS);
@@ -2028,11 +2028,9 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 		dev_priv->panel_t12 *= 100; /* t12 in 100ms units */
 
 		ironlake_edp_panel_vdd_on(intel_dp);
-		ret = intel_dp_aux_native_read(intel_dp, DP_DPCD_REV,
-					       intel_dp->dpcd,
-					       sizeof(intel_dp->dpcd));
+		ret = intel_dp_get_dpcd(intel_dp);
 		ironlake_edp_panel_vdd_off(intel_dp);
-		if (ret == sizeof(intel_dp->dpcd)) {
+		if (ret) {
 			if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11)
 				dev_priv->no_aux_handshake =
 					intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
-- 
1.7.5.4

[PATCH 4/5] drm/i915: Delay 250ms before running the hotplug code

[Keith Packard]

If the connector is inserted or removed slowly, the hotplug line may
well change state before the data lines do. So, assume the user isn't
trying to fool us and give them 250ms to get the connector plugged or
unplugged.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9da2a2c..e3ce1c3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -306,6 +306,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	struct intel_encoder *encoder;
 
+	/* Wait a bit so that the connector change can be completed */
+	msleep(250);
 	mutex_lock(&mode_config->mutex);
 	DRM_DEBUG_KMS("running encoder hotplug functions\n");
 
-- 
1.7.5.4

[PATCH 5/5] drm/i915: DP_PIPE_ENABLED must check transcoder on CPT

[Keith Packard]

Display port pipe selection on CPT is not done with a bit in the
output register, rather it is controlled by a couple of bits in the
separate transcoder register which indicate which display port output
is connected to the transcoder.

This patch replaces the simplistic macro DP_PIPE_ENABLED with the
rather more complicated function dp_pipe_enabled which checks the
output register to see if that is enabled, and then goes on to either
check the output register pipe selection bit (on non-CPT) or the
transcoder DP selection bits (on CPT).

Before this patch, any time the mode of pipe A was changed, any
display port outputs on pipe B would get disabled as
intel_disable_pch_ports would ensure that the mode setting operation
could occur on pipe A without interference from other outputs
connected to that pch port

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |    3 --
 drivers/gpu/drm/i915/intel_display.c |   45 +++++++++++++++++++++++++--------
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5d5def7..f731565 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2083,9 +2083,6 @@
 #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)
 #define   DP_LINK_TRAIN_PAT_2		(1 << 28)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5609c06..fc26cb4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -979,11 +979,29 @@ static void assert_transcoder_disabled(struct drm_i915_private *dev_priv,
 	     pipe_name(pipe));
 }
 
+static bool dp_pipe_enabled(struct drm_i915_private *dev_priv, enum pipe pipe,
+			    int reg, u32 port_sel, u32 val)
+{
+	if ((val & DP_PORT_EN) == 0)
+		return false;
+
+	if (HAS_PCH_CPT(dev_priv->dev)) {
+		u32	trans_dp_ctl_reg = TRANS_DP_CTL(pipe);
+		u32	trans_dp_ctl = I915_READ(trans_dp_ctl_reg);
+		if ((trans_dp_ctl & TRANS_DP_PORT_SEL_MASK) != port_sel)
+			return false;
+	} else {
+		if ((val & DP_PIPE_MASK) != (pipe << 30))
+			return false;
+	}
+	return true;
+}
+
 static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
-				   enum pipe pipe, int reg)
+				   enum pipe pipe, int reg, u32 port_sel)
 {
 	u32 val = I915_READ(reg);
-	WARN(DP_PIPE_ENABLED(val, pipe),
+	WARN(dp_pipe_enabled(dev_priv, pipe, reg, port_sel, val),
 	     "PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n",
 	     reg, pipe_name(pipe));
 }
@@ -1003,9 +1021,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
 	int reg;
 	u32 val;
 
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B);
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C);
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D);
+	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B);
+	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C);
+	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL_D);
 
 	reg = PCH_ADPA;
 	val = I915_READ(reg);
@@ -1334,19 +1352,24 @@ static void intel_disable_plane(struct drm_i915_private *dev_priv,
 }
 
 static void disable_pch_dp(struct drm_i915_private *dev_priv,
-			   enum pipe pipe, int reg)
+			   enum pipe pipe, int reg, u32 port_sel)
 {
 	u32 val = I915_READ(reg);
-	if (DP_PIPE_ENABLED(val, pipe))
+	if (dp_pipe_enabled(dev_priv, pipe, reg, port_sel, val)) {
+		DRM_DEBUG_KMS("Disabling pch dp %x on pipe %d\n", reg, 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))
+	if (HDMI_PIPE_ENABLED(val, pipe)) {
+		DRM_DEBUG_KMS("Disabling pch HDMI %x on pipe %d\n",
+			      reg, pipe);
 		I915_WRITE(reg, val & ~PORT_ENABLE);
+	}
 }
 
 /* Disable any ports connected to this transcoder */
@@ -1358,9 +1381,9 @@ static void intel_disable_pch_ports(struct drm_i915_private *dev_priv,
 	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);
+	disable_pch_dp(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B);
+	disable_pch_dp(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C);
+	disable_pch_dp(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL_D);
 
 	reg = PCH_ADPA;
 	val = I915_READ(reg);
-- 
1.7.5.4

Re: [PATCH 4/5] drm/i915: Delay 250ms before running the hotplug code

[Daniel Vetter]

queue_delayed_work? Plays nicer with other workqueue-items.

-Daniel

PS: Scrap my other mail, just noticed that the merged patched r-b'ed
by Jesse uses the mode_config mutex.
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/5] drm/i915: Delay 250ms before running the hotplug code

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 200 bytes --]

On Tue, 26 Jul 2011 09:44:32 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:

> queue_delayed_work? Plays nicer with other workqueue-items.

Yeah, I'll change this.

-- 
keith.packard@intel.com

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

Re: [Intel-gfx] [PATCH 1/5] drm/i915: Use dp_detect_common in hotplug helper function

[Jesse Barnes]

On Mon, 25 Jul 2011 23:36:30 -0700
Keith Packard <keithp@keithp.com> wrote:

> This uses the common dpcd reading routine, i915_dp_detect_common,
> instead of open-coding a call to intel_dp_aux_native_read. Besides
> reducing duplicated code, this also gains the read retries which
> may be necessary when a cable is first plugged back in and the link
> needs to be retrained.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   39 +++++++++++++++++++--------------------
>  1 files changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index dcc7ae6..45db810 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1567,6 +1567,20 @@ intel_dp_link_down(struct intel_dp *intel_dp)
>  	POSTING_READ(intel_dp->output_reg);
>  }
>  
> +static enum drm_connector_status
> +i915_dp_detect_common(struct intel_dp *intel_dp)

Maybe you can fix the prefix of this function while you're at it since
it's not i915 or even i9xx specific?

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [Intel-gfx] [PATCH 2/5] drm/i915: Rename i915_dp_detect_common to intel_dp_get_dpcd

[Jesse Barnes]

On Mon, 25 Jul 2011 23:36:31 -0700
Keith Packard <keithp@keithp.com> wrote:

> This describes the function better, allowing it to be used where the
> DPCD value is relevant.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---

Ah I see you've addressed my previous comment already. :)

You can add my
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> to 1/5 and 2/5.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [Intel-gfx] [PATCH 3/5] drm/i915: In intel_dp_init, replace read of DPCD with intel_dp_get_dpcd

[Jesse Barnes]

On Mon, 25 Jul 2011 23:36:32 -0700
Keith Packard <keithp@keithp.com> wrote:

> Eliminates an open-coded read and also gains the retry behaviour of
> intel_dp_get_dpcd, which seems like a good idea.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |    8 +++-----
>  1 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 41674e1..9f134d2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2015,7 +2015,7 @@ intel_dp_init(struct drm_device *dev, int output_reg)
>  
>  	/* Cache some DPCD data in the eDP case */
>  	if (is_edp(intel_dp)) {
> -		int ret;
> +		bool ret;
>  		u32 pp_on, pp_div;
>  
>  		pp_on = I915_READ(PCH_PP_ON_DELAYS);
> @@ -2028,11 +2028,9 @@ intel_dp_init(struct drm_device *dev, int output_reg)
>  		dev_priv->panel_t12 *= 100; /* t12 in 100ms units */
>  
>  		ironlake_edp_panel_vdd_on(intel_dp);
> -		ret = intel_dp_aux_native_read(intel_dp, DP_DPCD_REV,
> -					       intel_dp->dpcd,
> -					       sizeof(intel_dp->dpcd));
> +		ret = intel_dp_get_dpcd(intel_dp);
>  		ironlake_edp_panel_vdd_off(intel_dp);
> -		if (ret == sizeof(intel_dp->dpcd)) {
> +		if (ret) {
>  			if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11)
>  				dev_priv->no_aux_handshake =
>  					intel_dp->dpcd[DP_MAX_DOWNSPREAD] &

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Now we just have to enable fast link training in the eDP case (and
optionally when we know the DP monitor hasn't changed, just DPMS'd).

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 5/5] drm/i915: DP_PIPE_ENABLED must check transcoder on CPT

[Jesse Barnes]

On Mon, 25 Jul 2011 23:36:34 -0700
Keith Packard <keithp@keithp.com> wrote:

> Display port pipe selection on CPT is not done with a bit in the
> output register, rather it is controlled by a couple of bits in the
> separate transcoder register which indicate which display port output
> is connected to the transcoder.
> 
> This patch replaces the simplistic macro DP_PIPE_ENABLED with the
> rather more complicated function dp_pipe_enabled which checks the
> output register to see if that is enabled, and then goes on to either
> check the output register pipe selection bit (on non-CPT) or the
> transcoder DP selection bits (on CPT).
> 
> Before this patch, any time the mode of pipe A was changed, any
> display port outputs on pipe B would get disabled as
> intel_disable_pch_ports would ensure that the mode setting operation
> could occur on pipe A without interference from other outputs
> connected to that pch port
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---

Ah nice catch.  I expect one day we'll have all the chipset and PCH
differences coded...

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [Intel-gfx] drm/i915: A selection of display port fixes

[Adam Jackson]

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

On Mon, 2011-07-25 at 23:36 -0700, Keith Packard wrote:
> [PATCH 1/5] drm/i915: Use dp_detect_common in hotplug helper
>  [PATCH 2/5] drm/i915: Rename i915_dp_detect_common to
>  [PATCH 3/5] drm/i915: In intel_dp_init, replace read of DPCD with
> 
> These three are simple cleanups to centralize all places where the
> DPCD block was read from the device. Now everyone shares the same
> function, and that function retries the reads.

Reviewed-by: Adam Jackson <ajax@redhat.com>

>  [PATCH 4/5] drm/i915: Delay 250ms before running the hotplug code
> 
> I was experimenting with DP hotplugging -- moving the plug in and out
> of the jack very slowly and discovered that the hotplug interrupt
> occurred well before or after the link for the aux data channel was
> connected or disconnected. The result of this was that a sufficiently
> rapid cycle back through user mode could easily beat the motion of the
> plug and cause the hotplug detection to get the wrong status. Sticking
> a 250ms delay before doing anything gives the user sufficient time to
> actually get the plug connected or disconnected.

At the very least this should instead be queue_delayed_work().  But if
we're going to delay, can we instead set up PCH_PORT_HOTPLUG to do a
100ms delay for us?  I'll try this locally, at any rate.

>  [PATCH 5/5] drm/i915: DP_PIPE_ENABLED must check transcoder on CPT
> 
> <snip>
>
> Turns out the bug wasn't that the mode setting code was doing it wrong
> and turning the DP2 output off intentionally as a part of the mode
> change. Instead, the intel driver was trying to adjust the PCH link
> for the LVDS1 output and thought it needed to turn the DP2 output off
> because it mistakenly believed the DP2 output was sharing the same
> pipe as the LVDS1 output. Just a matter of using the wrong mechanism
> to detect which pipe the DP2 output was connected to.

Reviewed-by: Adam Jackson <ajax@redhat.com>

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel