diff mbox

[6/8] drm/i915: Remove remaining retries from intel_dp_aux_ch.

Message ID 1448066790-19591-7-git-send-email-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi Nov. 21, 2015, 12:46 a.m. UTC
drm level already takes care of the needed retries so instead of
duplicate the effort here.

If the retry is possible immediately we return EAGAIN. Otherwise,
if we have no idea what caused the error let's assume something
was busy and let drm level handle the wait and retries.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 64 ++++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 29 deletions(-)

Comments

Jani Nikula Nov. 23, 2015, 9:56 a.m. UTC | #1
On Sat, 21 Nov 2015, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> drm level already takes care of the needed retries so instead of
> duplicate the effort here.
>
> If the retry is possible immediately we return EAGAIN. Otherwise,
> if we have no idea what caused the error let's assume something
> was busy and let drm level handle the wait and retries.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 64 ++++++++++++++++++++++-------------------
>  1 file changed, 35 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a8ba243..35048d6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -795,7 +795,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  	uint32_t aux_clock_divider;
>  	int i, ret, recv_bytes;
>  	uint32_t status;
> -	int try, clock = 0;
> +	int clock = 0;
>  	bool has_aux_irq = HAS_AUX_IRQ(dev);
>  	bool vdd;
>  
> @@ -835,41 +835,47 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  							  send_bytes,
>  							  aux_clock_divider);
>  
> -		/* Must try at least 3 times according to DP spec */
> -		for (try = 0; try < 5; try++) {

This now inverses the retries wrt the aux clock divider retries, which
probably breaks the non-ULT HSW workaround:

"WA: For the PCH Aux channels (Aux B/C/D) use an aux 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). See South
Display Engine Registers, DP_AUX_CTL."

It is sad.

BR,
Jani.


> -			/* Load the send data into the aux channel data registers */
> -			for (i = 0; i < send_bytes; i += 4)
> -				I915_WRITE(intel_dp->aux_ch_data_reg[i >> 2],
> -					   intel_dp_pack_aux(send + i,
> -							     send_bytes - i));
> +		/* Load the send data into the aux channel data registers */
> +		for (i = 0; i < send_bytes; i += 4)
> +			I915_WRITE(intel_dp->aux_ch_data_reg[i >> 2],
> +				   intel_dp_pack_aux(send + i,
> +						     send_bytes - i));
>  
> -			/* Send the command and wait for it to complete */
> -			I915_WRITE(ch_ctl, send_ctl);
> +		/* Send the command and wait for it to complete */
> +		I915_WRITE(ch_ctl, send_ctl);
>  
> -			status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
> +		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);
> +		/* 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)
> -				continue;
> +		if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR) {
> +			/*
> +			 * DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
> +			 * 400us delay required for errors and timeouts
> +			 * Timeout errors from the HW already meet this
> +			 * requirement so skip to next iteration
> +			 */
> +			ret = -EAGAIN;
> +			goto out;
> +		}
>  
> -			/* DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
> -			 *   400us delay required for errors and timeouts
> -			 *   Timeout errors from the HW already meet this
> -			 *   requirement so skip to next iteration
> +		if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
> +			/*
> +			 * We don't know what caused the error, so let's
> +			 * return -EBUSY so drm level takes care of
> +			 * the necessary wait for recover and retries
>  			 */
> -			if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
> -				usleep_range(400, 500);
> -				continue;
> -			}
> -			if (status & DP_AUX_CH_CTL_DONE)
> -				goto done;
> +			ret = -EBUSY;
> +			goto out;
>  		}
> +
> +		if (status & DP_AUX_CH_CTL_DONE)
> +			goto done;
>  	}
>  
>  	if ((status & DP_AUX_CH_CTL_DONE) == 0) {
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a8ba243..35048d6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -795,7 +795,7 @@  intel_dp_aux_ch(struct intel_dp *intel_dp,
 	uint32_t aux_clock_divider;
 	int i, ret, recv_bytes;
 	uint32_t status;
-	int try, clock = 0;
+	int clock = 0;
 	bool has_aux_irq = HAS_AUX_IRQ(dev);
 	bool vdd;
 
@@ -835,41 +835,47 @@  intel_dp_aux_ch(struct intel_dp *intel_dp,
 							  send_bytes,
 							  aux_clock_divider);
 
-		/* 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(intel_dp->aux_ch_data_reg[i >> 2],
-					   intel_dp_pack_aux(send + i,
-							     send_bytes - i));
+		/* Load the send data into the aux channel data registers */
+		for (i = 0; i < send_bytes; i += 4)
+			I915_WRITE(intel_dp->aux_ch_data_reg[i >> 2],
+				   intel_dp_pack_aux(send + i,
+						     send_bytes - i));
 
-			/* Send the command and wait for it to complete */
-			I915_WRITE(ch_ctl, send_ctl);
+		/* Send the command and wait for it to complete */
+		I915_WRITE(ch_ctl, send_ctl);
 
-			status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
+		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);
+		/* 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)
-				continue;
+		if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR) {
+			/*
+			 * DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
+			 * 400us delay required for errors and timeouts
+			 * Timeout errors from the HW already meet this
+			 * requirement so skip to next iteration
+			 */
+			ret = -EAGAIN;
+			goto out;
+		}
 
-			/* DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
-			 *   400us delay required for errors and timeouts
-			 *   Timeout errors from the HW already meet this
-			 *   requirement so skip to next iteration
+		if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
+			/*
+			 * We don't know what caused the error, so let's
+			 * return -EBUSY so drm level takes care of
+			 * the necessary wait for recover and retries
 			 */
-			if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
-				usleep_range(400, 500);
-				continue;
-			}
-			if (status & DP_AUX_CH_CTL_DONE)
-				goto done;
+			ret = -EBUSY;
+			goto out;
 		}
+
+		if (status & DP_AUX_CH_CTL_DONE)
+			goto done;
 	}
 
 	if ((status & DP_AUX_CH_CTL_DONE) == 0) {