diff mbox series

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

Message ID 20240722165503.2084999-6-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/dp_mst: Enable LT fallback for UHBR<->non-UHBR rates | expand

Commit Message

Imre Deak July 22, 2024, 4:54 p.m. UTC
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@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Kandpal, Suraj July 23, 2024, 8:34 a.m. UTC | #1
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Imre
> Deak
> Sent: Monday, July 22, 2024 10:25 PM
> To: intel-gfx@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@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.

Regards,
Suraj Kandpal
> +	}
>  }
> 
>  bool intel_dp_initial_fastset_check(struct intel_encoder *encoder,
> --
> 2.44.2
Imre Deak July 23, 2024, 11:59 a.m. UTC | #2
On Tue, Jul 23, 2024 at 11:34:58AM +0300, Kandpal, Suraj wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Imre
> > Deak
> > Sent: Monday, July 22, 2024 10:25 PM
> > To: intel-gfx@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@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).

> Regards,
> Suraj Kandpal
> > +     }
> >  }
> >
> >  bool intel_dp_initial_fastset_check(struct intel_encoder *encoder,
> > --
> > 2.44.2
>
Kandpal, Suraj July 23, 2024, 1:05 p.m. UTC | #3
> -----Original Message-----
> From: Deak, Imre <imre.deak@intel.com>
> Sent: Tuesday, July 23, 2024 5:30 PM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>
> Cc: intel-gfx@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@lists.freedesktop.org> On Behalf
> > > Of Imre Deak
> > > Sent: Monday, July 22, 2024 10:25 PM
> > > To: intel-gfx@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@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@intel.com>

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

Patch

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;
+	}
 }
 
 bool intel_dp_initial_fastset_check(struct intel_encoder *encoder,