> -----Original Message-----
> From: Murthy, Arun R <arun.r.mur...@intel.com>
> Sent: Monday, July 4, 2022 12:54 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: ville.syrj...@linux.intel.com; Zanoni, Paulo R <paulo.r.zan...@intel.com>;
> tprev...@gmail.com; Shankar, Uma <uma.shan...@intel.com>; Nikula, Jani
> <jani.nik...@intel.com>; Murthy, Arun R <arun.r.mur...@intel.com>
> Subject: [PATCH] drm/i915/dp: wait on timeout before retry
> 
> On linktraining error/timeout before retry need to wait for 400usec as per 
> the DP
> CTS spec1.2 Under section 2.7.2 AUX Transaction Response/Reply Time-outs AUX
> Replier (the uPacket RX) must start sending the reply back to the AUX 
> requester (the
> uPacket TX) within the response period of 300μs. The timer for Response 
> Time-out
> starts ticking after the uPacket RX has finished receiving the AUX STOP 
> condition
> which ends the AUX Request transaction.
> The timer is reset either when the Response Time-out period has elapsed or 
> when
> the uPacket RX has started to send the AUX Sync pattern (which follows
> 10 to 16 active pre-charge pulses) for the Reply transaction. If the uPacket 
> TX does
> not receive a reply from the uPacket RX it must wait for a Reply Time-out 
> period of
> 400us before initiating the next AUX Request transaction. The timer for the 
> Reply
> Time-out starts ticking after the uPacket TX has finished sending the AUX STOP
> condition.
> 
> The patch with commit 74ebf294a1dd ("drm/i915: Add a delay in Displayport AUX
> transactions for compliance testing") removes this delay mentioning the 
> hardware
> already meets this requirement, but as per the spec the delay mentioned in 
> the spec
> specifies how long to wait for the receiver response before timeout. So the 
> delay
> here to wait for timeout and not a delay after timeout. The DP spec specifies 
> a delay
> after timeout and hence adding this delay.
> 
> v2: fixed checkpatch warning and error
> v3: used proper indentation
> v4: added DP CEA 1.2 spec details in patch commit msg

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shan...@intel.com>

> Signed-off-by: Arun R Murthy <arun.r.mur...@intel.com>
> Acked-by: Jani Nikula <jani.nik...@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_aux.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> index 2bc119374555..722c9f210690 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> @@ -286,13 +286,9 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>                       /*
>                        * 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_TIME_OUT_ERROR)
> -                             continue;
> -
> -                     if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
> +                     if (status & (DP_AUX_CH_CTL_RECEIVE_ERROR |
> +                                   DP_AUX_CH_CTL_TIME_OUT_ERROR)) {
>                               usleep_range(400, 500);
>                               continue;
>                       }
> --
> 2.25.1

Reply via email to