All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Retry DP aux_ch communications with a different clock after failure
@ 2013-07-21 15:00 Chris Wilson
  2013-07-26 11:40 ` Jani Nikula
  0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2013-07-21 15:00 UTC (permalink / raw)
  To: intel-gfx

The w/a db makes the recommendation to both use a non-default value for
the initial clock and then to retry with an alternative clock for
Haswell with the Lakeport PCH.

"On LPT:H, use a divider value of 63 decimal (03Fh). If there is a
failure, retry at least three times with 63, then retry at least three
times with 72 decimal (048h)."

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c | 92 +++++++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c6996ce..4a7ba5e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -276,7 +276,8 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	return status;
 }
 
-static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp)
+static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp,
+				      int index)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
@@ -290,22 +291,27 @@ static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp)
 	 * clock divider.
 	 */
 	if (IS_VALLEYVIEW(dev)) {
-		return 100;
+		return index ? 0 : 100;
 	} else if (intel_dig_port->port == PORT_A) {
+		if (index)
+			return 0;
 		if (HAS_DDI(dev))
-			return DIV_ROUND_CLOSEST(
-				intel_ddi_get_cdclk_freq(dev_priv), 2000);
+			return DIV_ROUND_CLOSEST(intel_ddi_get_cdclk_freq(dev_priv), 2000);
 		else if (IS_GEN6(dev) || IS_GEN7(dev))
 			return 200; /* SNB & IVB eDP input clock at 400Mhz */
 		else
 			return 225; /* eDP input clock at 450Mhz */
 	} else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
 		/* Workaround for non-ULT HSW */
-		return 74;
+		switch (index) {
+		case 0: return 63;
+		case 1: return 72;
+		default: return 0;
+		}
 	} else if (HAS_PCH_SPLIT(dev)) {
-		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
+		return index ? 0 : DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
 	} else {
-		return intel_hrawclk(dev) / 2;
+		return index ? 0 :intel_hrawclk(dev) / 2;
 	}
 }
 
@@ -319,10 +325,10 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
 	uint32_t ch_data = ch_ctl + 4;
+	uint32_t aux_clock_divider;
 	int i, ret, recv_bytes;
 	uint32_t status;
-	uint32_t aux_clock_divider = get_aux_clock_divider(intel_dp);
-	int try, precharge;
+	int try, precharge, clock = 0;
 	bool has_aux_irq = INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev);
 
 	/* dp aux is extremely sensitive to irq latency, hence request the
@@ -353,37 +359,41 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 		goto out;
 	}
 
-	/* Must try at least 3 times according to DP spec */
-	for (try = 0; try < 5; try++) {
-		/* Load the send data into the aux channel data registers */
-		for (i = 0; i < send_bytes; i += 4)
-			I915_WRITE(ch_data + i,
-				   pack_aux(send + i, send_bytes - i));
-
-		/* Send the command and wait for it to complete */
-		I915_WRITE(ch_ctl,
-			   DP_AUX_CH_CTL_SEND_BUSY |
-			   (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
-			   DP_AUX_CH_CTL_TIME_OUT_400us |
-			   (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
-			   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
-			   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT) |
-			   DP_AUX_CH_CTL_DONE |
-			   DP_AUX_CH_CTL_TIME_OUT_ERROR |
-			   DP_AUX_CH_CTL_RECEIVE_ERROR);
-
-		status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
-
-		/* Clear done status and any errors */
-		I915_WRITE(ch_ctl,
-			   status |
-			   DP_AUX_CH_CTL_DONE |
-			   DP_AUX_CH_CTL_TIME_OUT_ERROR |
-			   DP_AUX_CH_CTL_RECEIVE_ERROR);
-
-		if (status & (DP_AUX_CH_CTL_TIME_OUT_ERROR |
-			      DP_AUX_CH_CTL_RECEIVE_ERROR))
-			continue;
+	while ((aux_clock_divider = get_aux_clock_divider(intel_dp, clock++))) {
+		/* Must try at least 3 times according to DP spec */
+		for (try = 0; try < 5; try++) {
+			/* Load the send data into the aux channel data registers */
+			for (i = 0; i < send_bytes; i += 4)
+				I915_WRITE(ch_data + i,
+					   pack_aux(send + i, send_bytes - i));
+
+			/* Send the command and wait for it to complete */
+			I915_WRITE(ch_ctl,
+				   DP_AUX_CH_CTL_SEND_BUSY |
+				   (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
+				   DP_AUX_CH_CTL_TIME_OUT_400us |
+				   (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
+				   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
+				   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT) |
+				   DP_AUX_CH_CTL_DONE |
+				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
+				   DP_AUX_CH_CTL_RECEIVE_ERROR);
+
+			status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
+
+			/* Clear done status and any errors */
+			I915_WRITE(ch_ctl,
+				   status |
+				   DP_AUX_CH_CTL_DONE |
+				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
+				   DP_AUX_CH_CTL_RECEIVE_ERROR);
+
+			if (status & (DP_AUX_CH_CTL_TIME_OUT_ERROR |
+				      DP_AUX_CH_CTL_RECEIVE_ERROR))
+				continue;
+			if (status & DP_AUX_CH_CTL_DONE)
+				break;
+		}
 		if (status & DP_AUX_CH_CTL_DONE)
 			break;
 	}
@@ -1453,7 +1463,7 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	uint32_t aux_clock_divider = get_aux_clock_divider(intel_dp);
+	uint32_t aux_clock_divider = get_aux_clock_divider(intel_dp, 0);
 	int precharge = 0x3;
 	int msg_size = 5;       /* Header(4) + Message(1) */
 
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Retry DP aux_ch communications with a different clock after failure
  2013-07-21 15:00 [PATCH] drm/i915: Retry DP aux_ch communications with a different clock after failure Chris Wilson
@ 2013-07-26 11:40 ` Jani Nikula
  2013-07-26 17:45   ` Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Jani Nikula @ 2013-07-26 11:40 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Sun, 21 Jul 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> The w/a db makes the recommendation to both use a non-default value for
> the initial clock and then to retry with an alternative clock for
> Haswell with the Lakeport PCH.
>
> "On LPT:H, use a divider value of 63 decimal (03Fh). If there is a
> failure, retry at least three times with 63, then retry at least three
> times with 72 decimal (048h)."

Overall I'd prefer adding the outer repeat loop first, keeping existing
behaviour, and only then adding the divider value changes in
get_aux_clock_divider(). But that's in the bikeshedding dept. Either
way,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 92 +++++++++++++++++++++++------------------
>  1 file changed, 51 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c6996ce..4a7ba5e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -276,7 +276,8 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
>  	return status;
>  }
>  
> -static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp)
> +static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp,
> +				      int index)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
> @@ -290,22 +291,27 @@ static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp)
>  	 * clock divider.
>  	 */
>  	if (IS_VALLEYVIEW(dev)) {
> -		return 100;
> +		return index ? 0 : 100;
>  	} else if (intel_dig_port->port == PORT_A) {
> +		if (index)
> +			return 0;
>  		if (HAS_DDI(dev))
> -			return DIV_ROUND_CLOSEST(
> -				intel_ddi_get_cdclk_freq(dev_priv), 2000);
> +			return DIV_ROUND_CLOSEST(intel_ddi_get_cdclk_freq(dev_priv), 2000);
>  		else if (IS_GEN6(dev) || IS_GEN7(dev))
>  			return 200; /* SNB & IVB eDP input clock at 400Mhz */
>  		else
>  			return 225; /* eDP input clock at 450Mhz */
>  	} else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
>  		/* Workaround for non-ULT HSW */
> -		return 74;
> +		switch (index) {
> +		case 0: return 63;
> +		case 1: return 72;
> +		default: return 0;
> +		}
>  	} else if (HAS_PCH_SPLIT(dev)) {
> -		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> +		return index ? 0 : DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
>  	} else {
> -		return intel_hrawclk(dev) / 2;
> +		return index ? 0 :intel_hrawclk(dev) / 2;
>  	}
>  }
>  
> @@ -319,10 +325,10 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
>  	uint32_t ch_data = ch_ctl + 4;
> +	uint32_t aux_clock_divider;
>  	int i, ret, recv_bytes;
>  	uint32_t status;
> -	uint32_t aux_clock_divider = get_aux_clock_divider(intel_dp);
> -	int try, precharge;
> +	int try, precharge, clock = 0;
>  	bool has_aux_irq = INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev);
>  
>  	/* dp aux is extremely sensitive to irq latency, hence request the
> @@ -353,37 +359,41 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  		goto out;
>  	}
>  
> -	/* Must try at least 3 times according to DP spec */
> -	for (try = 0; try < 5; try++) {
> -		/* Load the send data into the aux channel data registers */
> -		for (i = 0; i < send_bytes; i += 4)
> -			I915_WRITE(ch_data + i,
> -				   pack_aux(send + i, send_bytes - i));
> -
> -		/* Send the command and wait for it to complete */
> -		I915_WRITE(ch_ctl,
> -			   DP_AUX_CH_CTL_SEND_BUSY |
> -			   (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> -			   DP_AUX_CH_CTL_TIME_OUT_400us |
> -			   (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> -			   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> -			   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT) |
> -			   DP_AUX_CH_CTL_DONE |
> -			   DP_AUX_CH_CTL_TIME_OUT_ERROR |
> -			   DP_AUX_CH_CTL_RECEIVE_ERROR);
> -
> -		status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> -
> -		/* Clear done status and any errors */
> -		I915_WRITE(ch_ctl,
> -			   status |
> -			   DP_AUX_CH_CTL_DONE |
> -			   DP_AUX_CH_CTL_TIME_OUT_ERROR |
> -			   DP_AUX_CH_CTL_RECEIVE_ERROR);
> -
> -		if (status & (DP_AUX_CH_CTL_TIME_OUT_ERROR |
> -			      DP_AUX_CH_CTL_RECEIVE_ERROR))
> -			continue;
> +	while ((aux_clock_divider = get_aux_clock_divider(intel_dp, clock++))) {
> +		/* Must try at least 3 times according to DP spec */
> +		for (try = 0; try < 5; try++) {
> +			/* Load the send data into the aux channel data registers */
> +			for (i = 0; i < send_bytes; i += 4)
> +				I915_WRITE(ch_data + i,
> +					   pack_aux(send + i, send_bytes - i));
> +
> +			/* Send the command and wait for it to complete */
> +			I915_WRITE(ch_ctl,
> +				   DP_AUX_CH_CTL_SEND_BUSY |
> +				   (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
> +				   DP_AUX_CH_CTL_TIME_OUT_400us |
> +				   (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> +				   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> +				   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT) |
> +				   DP_AUX_CH_CTL_DONE |
> +				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
> +				   DP_AUX_CH_CTL_RECEIVE_ERROR);
> +
> +			status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> +
> +			/* Clear done status and any errors */
> +			I915_WRITE(ch_ctl,
> +				   status |
> +				   DP_AUX_CH_CTL_DONE |
> +				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
> +				   DP_AUX_CH_CTL_RECEIVE_ERROR);
> +
> +			if (status & (DP_AUX_CH_CTL_TIME_OUT_ERROR |
> +				      DP_AUX_CH_CTL_RECEIVE_ERROR))
> +				continue;
> +			if (status & DP_AUX_CH_CTL_DONE)
> +				break;
> +		}
>  		if (status & DP_AUX_CH_CTL_DONE)
>  			break;
>  	}
> @@ -1453,7 +1463,7 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
>  {
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	uint32_t aux_clock_divider = get_aux_clock_divider(intel_dp);
> +	uint32_t aux_clock_divider = get_aux_clock_divider(intel_dp, 0);
>  	int precharge = 0x3;
>  	int msg_size = 5;       /* Header(4) + Message(1) */
>  
> -- 
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Retry DP aux_ch communications with a different clock after failure
  2013-07-26 11:40 ` Jani Nikula
@ 2013-07-26 17:45   ` Daniel Vetter
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2013-07-26 17:45 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, Jul 26, 2013 at 02:40:01PM +0300, Jani Nikula wrote:
> On Sun, 21 Jul 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > The w/a db makes the recommendation to both use a non-default value for
> > the initial clock and then to retry with an alternative clock for
> > Haswell with the Lakeport PCH.
> >
> > "On LPT:H, use a divider value of 63 decimal (03Fh). If there is a
> > failure, retry at least three times with 63, then retry at least three
> > times with 72 decimal (048h)."
> 
> Overall I'd prefer adding the outer repeat loop first, keeping existing
> behaviour, and only then adding the divider value changes in
> get_aux_clock_divider(). But that's in the bikeshedding dept. Either
> way,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-07-26 17:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-21 15:00 [PATCH] drm/i915: Retry DP aux_ch communications with a different clock after failure Chris Wilson
2013-07-26 11:40 ` Jani Nikula
2013-07-26 17:45   ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.