[Intel-gfx] [PATCHv3] drm/i915/dp: wait on timeout before retry

[Shankar, Uma]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula at intel.com>
> Sent: Friday, July 1, 2022 4:45 PM
> To: Murthy, Arun R <arun.r.murthy at intel.com>; intel-gfx at lists.freedesktop.org
> Cc: ville.syrjala at linux.intel.com; Zanoni, Paulo R <paulo.r.zanoni at intel.com>;
> tprevite at gmail.com; Shankar, Uma <uma.shankar at intel.com>; Murthy, Arun R
> <arun.r.murthy at intel.com>
> Subject: Re: [PATCHv3] drm/i915/dp: wait on timeout before retry
> 
> On Fri, 01 Jul 2022, Arun R Murthy <arun.r.murthy at intel.com> wrote:
> > On linktraining error/timeout before retry need to wait for 400usec as
> > per the DP CTS spec1.2
> >
> > 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.

The source side as per bspec says how much time to wait for sink response before calling timeout:
Reg: DDI_AUX_CTL, BitField: 27:26
Name: Time out timer value 
Description: 
Value	Name	
00b	400us	 (default)			
01b	600us			
10b	800us		
11b	4000us

So this should meet the CTS requirement. For any non-timeout error from sink, s/w should add
explicit delay of minimum 400us before retry (which is being done)

If 400us is not enough this needs to be checked separately or the timeout value in this register can be
changed from default of 400us

Regards,
Uma Shankar

> >
> > v2: fixed checkpatch warning and error
> > v3: used proper indentation
> >
> > Signed-off-by: Arun R Murthy <arun.r.murthy at intel.com>
> 
> Based on the description alone,
> 
> Acked-by: Jani Nikula <jani.nikula at intel.com>
> 
> but would be good if you could get someone to double check the details against
> bspec and CTS.
> 
> > ---
> >  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;
> >  			}
> 
> --
> Jani Nikula, Intel Open Source Graphics Center