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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Srikanth V, NagaVenkata <nagavenkata.srikanth.v@xxxxxxxxx>
> Sent: Monday, September 23, 2024 11:59 AM
> To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>; Murthy, Arun R
> <arun.r.murthy@xxxxxxxxx>; intel-xe@xxxxxxxxxxxxxxxxxxxxx; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Murthy, Arun R <arun.r.murthy@xxxxxxxxx>
> Subject: RE: [PATCH 2/3] drm/i915/dp: read Aux RD interval after reading the
> FFE preset
> 
> > > -----Original Message-----
> > > From: Intel-xe <intel-xe-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> > > Arun R Murthy
> > > Sent: Thursday, September 12, 2024 10:36 AM
> > > To: intel-xe@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > Cc: Murthy, Arun R <arun.r.murthy@xxxxxxxxx>; Srikanth V,
> > > NagaVenkata <nagavenkata.srikanth.v@xxxxxxxxx>
> > > Subject: [PATCH 2/3] drm/i915/dp: read Aux RD interval after reading
> > > the FFE preset
> > >
> > > 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 think mentioning the dp spec reference here would be helpful
> >
> Please refer to Figure 3-52: 128b132b DP DPTC LANEx_CHANNEL_EQ_DONE
> Sequence of DP2.1a spec.
> 
> > >
> > > Signed-off-by: Srikanth V NagaVenkata
> > > <nagavenkata.srikanth.v@xxxxxxxxx>
> > > Signed-off-by: Arun R Murthy <arun.r.murthy@xxxxxxxxx>
> > > ---
> > >  .../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.
> > > +		 */
> >
> > The comment needs to be updated as this is not being done before link
> > status Also a question does this not conflict with the requirement we
> > previously had (reading it before link status) ?
> >

Also  this whole delay us read should be called much below in the sequence from what I can see
In the dp spec just before we adjust the ffe settings at this point

/* Update signal levels and training set as requested. */
                intel_dp_get_adjust_train(intel_dp, crtc_state, DP_PHY_DPRX, link_status);
                if (!intel_dp_update_link_train(intel_dp, crtc_state, DP_PHY_DPRX)) {
                        lt_err(intel_dp, DP_PHY_DPRX, "Failed to update TX FFE settings\n");
                        return false;
                }

Regards,
Suraj Kandpal

> > Regards,
> > Suraj Kandpal
> >
> > Regards,
> > Suraj Kandpal
> >
> > > +		delay_us =
> > > drm_dp_128b132b_read_aux_rd_interval(&intel_dp->aux);
> > >  	}
> > >
> > >  	if (try == max_tries) {
> > > --
> > > 2.25.1





[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux