On Mon, 27 Jun 2022, Arun R Murthy <arun.r.mur...@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.

> 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.

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.mur...@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.

BR,
Jani.

>                               usleep_range(400, 500);
>                               continue;
>                       }

-- 
Jani Nikula, Intel Open Source Graphics Center

Reply via email to