On Sat, 21 Nov 2015, Rodrigo Vivi <rodrigo.vivi at 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 at 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
