On Fri, May 22, 2026 at 03:50:38PM +0300, Ville Syrjälä wrote: > On Fri, May 22, 2026 at 03:46:50PM +0300, Jani Nikula wrote: > > On Fri, 22 May 2026, Ville Syrjälä <[email protected]> wrote: > > > On Fri, May 22, 2026 at 01:29:39PM +0300, Jani Nikula wrote: > > >> On Fri, 22 May 2026, Imre Deak <[email protected]> wrote: > > >> > On Fri, May 22, 2026 at 12:36:27AM +0300, Ville Syrjälä wrote: > > >> >> On Mon, May 18, 2026 at 02:24:22PM +0300, Imre Deak wrote: > > >> >> > Add helpers to defer and handle link params resets instead of > > >> >> > open-coding the same. Rename intel_dp_reset_link_params() to > > >> >> > intel_dp_reset_link_params_force() to align its name with the new > > >> >> > deferred reset helpers. > > >> >> > > > >> >> > When deferring a reset, return whether a new reset was queued, used > > >> >> > by a > > >> >> > follow-up change. > > >> >> > > > >> >> > Signed-off-by: Imre Deak <[email protected]> > > >> >> > --- > > >> >> > drivers/gpu/drm/i915/display/g4x_dp.c | 2 +- > > >> >> > drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- > > >> >> > drivers/gpu/drm/i915/display/intel_dp.c | 41 > > >> >> > +++++++++++++++---- > > >> >> > drivers/gpu/drm/i915/display/intel_dp.h | 3 +- > > >> >> > .../drm/i915/display/intel_dp_link_training.c | 4 +- > > >> >> > 5 files changed, 38 insertions(+), 14 deletions(-) > > >> >> > > > >> >> > diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c > > >> >> > b/drivers/gpu/drm/i915/display/g4x_dp.c > > >> >> > index 5ff1cdf4581a5..c20a97e21419b 100644 > > >> >> > --- a/drivers/gpu/drm/i915/display/g4x_dp.c > > >> >> > +++ b/drivers/gpu/drm/i915/display/g4x_dp.c > > >> >> > @@ -1265,7 +1265,7 @@ static void intel_dp_encoder_reset(struct > > >> >> > drm_encoder *encoder) > > >> >> > > > >> >> > intel_dp->DP = intel_de_read(display, intel_dp->output_reg); > > >> >> > > > >> >> > - intel_dp->reset_link_params = true; > > >> >> > + intel_dp_reset_link_params_defer(intel_dp); > > >> >> > intel_dp_invalidate_source_oui(intel_dp); > > >> >> > > > >> >> > if (display->platform.valleyview || > > >> >> > display->platform.cherryview) > > >> >> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > >> >> > b/drivers/gpu/drm/i915/display/intel_ddi.c > > >> >> > index 86520848892e0..77819aaeccb76 100644 > > >> >> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > >> >> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > >> >> > @@ -4664,7 +4664,7 @@ static void intel_ddi_encoder_reset(struct > > >> >> > drm_encoder *encoder) > > >> >> > struct intel_dp *intel_dp = > > >> >> > enc_to_intel_dp(to_intel_encoder(encoder)); > > >> >> > struct intel_digital_port *dig_port = > > >> >> > enc_to_dig_port(to_intel_encoder(encoder)); > > >> >> > > > >> >> > - intel_dp->reset_link_params = true; > > >> >> > + intel_dp_reset_link_params_defer(intel_dp); > > >> >> > intel_dp_invalidate_source_oui(intel_dp); > > >> >> > > > >> >> > intel_pps_encoder_reset(intel_dp); > > >> >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > >> >> > b/drivers/gpu/drm/i915/display/intel_dp.c > > >> >> > index 1920d2f026665..13163dd085e91 100644 > > >> >> > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > >> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > >> >> > @@ -3710,7 +3710,11 @@ void intel_dp_set_link_params(struct > > >> >> > intel_dp *intel_dp, > > >> >> > intel_dp->lane_count = lane_count; > > >> >> > } > > >> >> > > > >> >> > -void intel_dp_reset_link_params(struct intel_dp *intel_dp) > > >> >> > +/* > > >> >> > + * Reset link params now, preserving any deferred connector > > >> >> > + * detect-time reset request. > > >> >> > + */ > > >> >> > +void intel_dp_reset_link_params_force(struct intel_dp *intel_dp) > > >> >> > { > > >> >> > intel_dp->link.max_lane_count = > > >> >> > intel_dp_max_common_lane_count(intel_dp); > > >> >> > intel_dp->link.max_rate = intel_dp_max_common_rate(intel_dp); > > >> >> > @@ -3720,6 +3724,28 @@ void intel_dp_reset_link_params(struct > > >> >> > intel_dp *intel_dp) > > >> >> > intel_dp->link.seq_train_failures = 0; > > >> >> > } > > >> >> > > > >> >> > +/* > > >> >> > + * Reset link params during the next connector detect. > > >> >> > + * Return %true if a new reset was queued. > > >> >> > + */ > > >> >> > +bool intel_dp_reset_link_params_defer(struct intel_dp *intel_dp) > > >> >> > > >> >> I find the intel_dp_reset_link_params_defer() vs. > > >> >> intel_dp_reset_link_params_force() naming rather confusing. > > >> >> > > >> >> Can't immediately think of a really good name for > > >> >> intel_dp_reset_link_params_defer() so maybe it's better to not > > >> >> have a function for it at all (ie. just drop this patch)? > > >> > > > >> > The idea was to have an interface to reset the link params directly or > > >> > in a deferred way, instead of a direct access of the flag. > > >> > > > >> > The names are not great yes. I could use what Jouni suggested instead, > > >> > or if the above argument is not good enough I can also drop this patch. > > >> > > >> I suggest intel_dp_reset_link_params() keeps its name, or gets renamed > > >> to intel_dp_link_params_reset(). It just does the thing, no "force". > > >> > > >> Then the other two use something like: > > >> > > >> - submit/process > > >> - queue/process > > >> - stage/handle > > >> > > >> or some combination i.e. something like: > > >> > > >> intel_dp_link_params_reset() > > >> intel_dp_link_params_reset_submit() > > >> intel_dp_link_params_reset_process() > > > > > > All of it kinda leaves it a bit unclear how these things > > > are related, and then one probably has to read through > > > them to figure out what they're actually doing. > > > > > > Hence why I think just setting/checking the flag on the > > > spot might be the clearest approach. You can immediately > > > find where it's set vs. checked. > > > > Trouble is if you want to hide that flag in an opaque type in the > > future. > > Do we want that?
There's a few property of the (active) link like intel_dp::link_rate, lane_count, link.active, reset_link_params, which all could be tracked in a separate link specific state. It could also make sense then to move all the handlers of that state into a separate module, but not sure about the details for that. For now I think it's better to drop this patch and reconsider a suitable interface later. Will resend v2 with that. > > -- > Ville Syrjälä > Intel
