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

[Murthy, Arun R]

> On Thu, 12 Sep 2024, Arun R Murthy <arun.r.murthy at 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.
> 
Will correct the statement.

> > Signed-off-by: Srikanth V NagaVenkata
> > <nagavenkata.srikanth.v at intel.com>
> > Signed-off-by: Arun R Murthy <arun.r.murthy at 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.
> 
Yes, agree Will update this in the next version.

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