Re: [PATCH 2/3] drm/i915/dp: read Aux RD interval after reading the FFE preset

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 12 Sep 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> DP Source should be reading AUX_RD interval after we get adjusted
> TX_FFE_PRESET_VALUE from the DP Sink. (before actually adjusting
> in DP Source)

I don't think that's correct. See below.

> Signed-off-by: Srikanth V NagaVenkata <nagavenkata.srikanth.v@intel.com>
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_dp_link_training.c    | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index f41b69840ad9..ca179bed46ad 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -1419,12 +1419,6 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>  	for (try = 0; try < max_tries; try++) {
>  		fsleep(delay_us);
>  
> -		/*
> -		 * The delay may get updated. The transmitter shall read the
> -		 * delay before link status during link training.
> -		 */
> -		delay_us = drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
> -
>  		if (drm_dp_dpcd_read_link_status(&intel_dp->aux, link_status) < 0) {
>  			lt_err(intel_dp, DP_PHY_DPRX, "Failed to read link status\n");
>  			return false;
> @@ -1457,6 +1451,12 @@ intel_dp_128b132b_lane_eq(struct intel_dp *intel_dp,
>  			lt_err(intel_dp, DP_PHY_DPRX, "Failed to update TX FFE settings\n");
>  			return false;
>  		}
> +
> +		/*
> +		 * The delay may get updated. The transmitter shall read the
> +		 * delay before link status during link training.
> +		 */
> +		delay_us = drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);

This is clearly an improvement, but Figure 3-52 of DP 2.1a has "Read
AUX_RD_INTERVAL value" before "Adjust requested the TX_FFE_PRESET_VALUE
by a DPRX/LTTPR_UFP setting". Yes, in the same box in the flow chart,
but before.

Sticking with the spec, the read should be placed above this comment:

	/* Update signal levels and training set as requested. */

Be sure to reference the spec in the commit message.


BR,
Jani.


>  	}
>  
>  	if (try == max_tries) {

-- 
Jani Nikula, Intel