[PATCH 05/14] drm/i915/dp: Initialize the link parameters during HW readout

[Kandpal, Suraj]

> -----Original Message-----
> From: Deak, Imre <imre.deak at intel.com>
> Sent: Tuesday, July 23, 2024 5:30 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 05/14] drm/i915/dp: Initialize the link parameters during
> HW readout
> 
> On Tue, Jul 23, 2024 at 11:34:58AM +0300, Kandpal, Suraj wrote:
> >
> >
> > > -----Original Message-----
> > > From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf
> > > Of Imre Deak
> > > Sent: Monday, July 22, 2024 10:25 PM
> > > To: intel-gfx at lists.freedesktop.org
> > > Subject: [PATCH 05/14] drm/i915/dp: Initialize the link parameters
> > > during HW readout
> > >
> > > Initialize the DP link parameters during HW readout. These need to
> > > be up- to-date at least for the MST topology probing, which depends
> > > on the link rate and lane count programmed in DPCD. A follow-up
> > > patch will program the DPCD values to reflect the maximum link
> > > parameters before the first MST topology probing, but should do so
> > > only if the link is disabled (link_trained==false).
> > >
> > > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dp.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 1e43e32e05199..421e970b3c180 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -3352,8 +3352,11 @@ void intel_dp_sync_state(struct intel_encoder
> > > *encoder,
> > >
> > >       intel_dp_tunnel_resume(intel_dp, crtc_state, dpcd_updated);
> > >
> > > -     if (crtc_state)
> > > +     if (crtc_state) {
> > >               intel_dp_reset_link_params(intel_dp);
> > > +             intel_dp_set_link_params(intel_dp,
> > > + crtc_state->port_clock,
> > > crtc_state->lane_count);
> > > +             intel_dp->link_trained = true;
> >
> > Why are we setting link_trained as true here.
> 
> link_trained indicates whether the output is enabled or not, which must be
> in sync with crtc_state being NULL (output disabled) or not NULL (output
> enabled).

Okay got it 
LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal at intel.com>

> 
> > Regards,
> > Suraj Kandpal
> > > +     }
> > >  }
> > >
> > >  bool intel_dp_initial_fastset_check(struct intel_encoder *encoder,
> > > --
> > > 2.44.2
> >