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()
BR,
Jani.
>
>> Then you at least see that it's just setting the flag. AFAICS you only
>> have a single place (in the last patch) that uses this return value
>> for anything, so could just do the check+set dance there on the spot.
>
>>
>> > +{
>> > + bool reset_was_pending = intel_dp->reset_link_params;
>> > +
>> > + intel_dp->reset_link_params = true;
>> > +
>> > + return !reset_was_pending;
>> > +}
>> > +
>> > +static void intel_dp_handle_deferred_link_params_reset(struct intel_dp
>> > *intel_dp)
>> > +{
>> > + if (!intel_dp->reset_link_params)
>> > + return;
>> > +
>> > + intel_dp->reset_link_params = false;
>> > + intel_dp_reset_link_params_force(intel_dp);
>> > +}
>> > +
>> > /* Enable backlight PWM and backlight PP control. */
>> > void intel_edp_backlight_on(const struct intel_crtc_state *crtc_state,
>> > const struct drm_connector_state *conn_state)
>> > @@ -4066,7 +4092,7 @@ void intel_dp_sync_state(struct intel_encoder
>> > *encoder,
>> > intel_dp_tunnel_resume(intel_dp, crtc_state, dpcd_updated);
>> >
>> > if (crtc_state) {
>> > - intel_dp_reset_link_params(intel_dp);
>> > + intel_dp_reset_link_params_force(intel_dp);
>> > intel_dp_set_link_params(intel_dp, crtc_state->port_clock,
>> > crtc_state->lane_count);
>> > intel_dp->link.active = true;
>> > }
>> > @@ -6487,10 +6513,7 @@ intel_dp_detect(struct drm_connector *_connector,
>> >
>> > intel_dp_detect_sdp_caps(intel_dp);
>> >
>> > - if (intel_dp->reset_link_params) {
>> > - intel_dp_reset_link_params(intel_dp);
>> > - intel_dp->reset_link_params = false;
>> > - }
>> > + intel_dp_handle_deferred_link_params_reset(intel_dp);
>> >
>> > intel_dp_mst_configure(intel_dp);
>> >
>> > @@ -6944,7 +6967,7 @@ intel_dp_hpd_pulse(struct intel_digital_port
>> > *dig_port, bool long_hpd)
>> >
>> > intel_dp_read_dprx_caps(intel_dp, dpcd);
>> >
>> > - intel_dp->reset_link_params = true;
>> > + intel_dp_reset_link_params_defer(intel_dp);
>> > intel_dp_invalidate_source_oui(intel_dp);
>> >
>> > return IRQ_NONE;
>> > @@ -7252,7 +7275,7 @@ intel_dp_init_connector(struct intel_digital_port
>> > *dig_port,
>> > encoder->base.name))
>> > return false;
>> >
>> > - intel_dp->reset_link_params = true;
>> > + intel_dp_reset_link_params_defer(intel_dp);
>> >
>> > /* Preserve the current hw state. */
>> > intel_dp->DP = intel_de_read(display, intel_dp->output_reg);
>> > @@ -7317,7 +7340,7 @@ intel_dp_init_connector(struct intel_digital_port
>> > *dig_port,
>> >
>> > intel_dp_set_source_rates(intel_dp);
>> > intel_dp_set_common_rates(intel_dp);
>> > - intel_dp_reset_link_params(intel_dp);
>> > + intel_dp_reset_link_params_force(intel_dp);
>> >
>> > /* init MST on ports that can support it */
>> > intel_dp_mst_encoder_init(dig_port, connector->base.base.id);
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
>> > b/drivers/gpu/drm/i915/display/intel_dp.h
>> > index f41480d247142..7c24d3dbb6983 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>> > @@ -114,7 +114,8 @@ int intel_dp_rate_index(const int *rates, int len, int
>> > rate);
>> > int intel_dp_link_config_index(struct intel_dp *intel_dp, int link_rate,
>> > int lane_count);
>> > void intel_dp_link_config_get(struct intel_dp *intel_dp, int idx, int
>> > *link_rate, int *lane_count);
>> > void intel_dp_update_sink_caps(struct intel_dp *intel_dp);
>> > -void intel_dp_reset_link_params(struct intel_dp *intel_dp);
>> > +void intel_dp_reset_link_params_force(struct intel_dp *intel_dp);
>> > +bool intel_dp_reset_link_params_defer(struct intel_dp *intel_dp);
>> >
>> > void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>> > u8 *link_bw, u8 *rate_select);
>> > 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 a26094223f780..b7075060e7bd3 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> > @@ -1935,7 +1935,7 @@ static ssize_t i915_dp_force_link_rate_write(struct
>> > file *file,
>> > if (err)
>> > return err;
>> >
>> > - intel_dp_reset_link_params(intel_dp);
>> > + intel_dp_reset_link_params_force(intel_dp);
>> > intel_dp->link.force_rate = rate;
>> >
>> > drm_modeset_unlock(&display->drm->mode_config.connection_mutex);
>> > @@ -2037,7 +2037,7 @@ static ssize_t i915_dp_force_lane_count_write(struct
>> > file *file,
>> > if (err)
>> > return err;
>> >
>> > - intel_dp_reset_link_params(intel_dp);
>> > + intel_dp_reset_link_params_force(intel_dp);
>> > intel_dp->link.force_lane_count = lane_count;
>> >
>> > drm_modeset_unlock(&display->drm->mode_config.connection_mutex);
>> > --
>> > 2.49.1
>>
>> --
>> Ville Syrjälä
>> Intel
--
Jani Nikula, Intel
