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

[Murthy, Arun R]

> On Mon, 27 Jun 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 id
> > 74ebf294a1dd816bdc248ac733009a8915d59eb5
> > drm/i915: Add a delay in Displayport AUX transactions for compliance
> > testing
> 
> Please reference other commits like this: commit 74ebf294a1dd
> ("drm/i915: Add a delay in Displayport AUX transactions for compliance
> testing")
> 
> I've got this git alias in my .gitconfig:
> 
> 	cite = "!f() { git log -1 '--pretty=format:%H (\"%s\")%n' $1 | sed -e
> 's/\\([0-f]\\{12\\}\\)[0-f]*/\\1/'; }; f"
> 
> And you can use 'git cite <commit-ish>' to give you the right format.
> 

Will correct this!

> > 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.
> 
> Please try to reflow commit messages to about 72 characters.
> 

Commit msg is maintained to be 72 char, but the patch details added might be misleading.
Will correct them!

> I had a bit of a hard time parsing the above, but basically you're saying that
> the hardware handles and flags the timeout, but the DP spec says we need to
> wait 400 us *after* the error condition. Makes sense, though I didn't have
> the time to read the specs to verify.
> 
> >
> > Signed-off-by: Arun R Murthy <arun.r.murthy at 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..a1fef1645d6a 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)) {
> 
> Please check the indentation here. The DP_AUX_CH_* should start in the
> same column.
> 

Done!

Thanks and Regards,
Arun R Murthy
--------------------