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

[Jani Nikula <jani.nikula@linux.intel.com>]

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) {

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx