[PATCH] drm/i915/dp: Remove redundant link capabilities reset

[PATCH] drm/i915/dp: Remove redundant link capabilities reset

[George Karagounis]

intel_dp_link_caps_update(reset=true) is already called prior to intel_dp_reset_link_params(), making the explicit call to intel_dp_link_caps_reset() redundant. Remove it as requested by the existing TODO comment to clean up the link parameter reset flow.

Signed-off-by: George Karagounis <mail@taterr.org>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index d8c0d9381471..0e753824bc28 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3674,17 +3674,10 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
 
 void intel_dp_reset_link_params(struct intel_dp *intel_dp)
 {
-	/*
-	 * TODO: Remove the following reset of link capabilities, as
-	 * this isn't needed after intel_dp_link_caps_update(reset=true)
-	 * was called.
-	 */
-	intel_dp_link_caps_reset(intel_dp->link.caps);
-	intel_dp_tunnel_uhbr_lanes_wa_apply(intel_dp);
-
-	intel_dp->link.mst_probed_lane_count = 0;
-	intel_dp->link.mst_probed_rate = 0;
-	intel_dp_link_training_reset(intel_dp->link.training);
+       intel_dp_tunnel_uhbr_lanes_wa_apply(intel_dp);
+       intel_dp->link.mst_probed_lane_count = 0;
+       intel_dp->link.mst_probed_rate = 0;
+       intel_dp_link_training_reset(intel_dp->link.training);
 }
 
 /* Enable backlight PWM and backlight PP control. */
-- 
2.56.0

Re: [PATCH] drm/i915/dp: Remove redundant link capabilities reset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] drm/i915/dp: missing link capabilities reset in intel_dp_reset_link_params()
- [Low] drm/i915/dp: unused function intel_dp_link_caps_reset() left behind
- [Low] drm/i915/dp: space indentation used in intel_dp_reset_link_params()
--

commit a8d5bf6dc42f9bcd49f6371d597ac7f839e9b04f
Author: George Karagounis <taterraster@gmail.com>

drm/i915/dp: Remove redundant link capabilities reset

This commit removes the intel_dp_link_caps_reset() call from
intel_dp_reset_link_params(), assuming that intel_dp_link_caps_update() with
reset=true has already been invoked beforehand.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index d8c0d9381471d..0e753824bc281 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3674,17 +3674,10 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
>  
>  void intel_dp_reset_link_params(struct intel_dp *intel_dp)
>  {
> -	/*
> -	 * TODO: Remove the following reset of link capabilities, as
> -	 * this isn't needed after intel_dp_link_caps_update(reset=true)
> -	 * was called.
> -	 */
> -	intel_dp_link_caps_reset(intel_dp->link.caps);

[Severity: High]
Are all callers of intel_dp_reset_link_params() guaranteed to have called
intel_dp_link_caps_update(reset=true) beforehand?

Several paths call intel_dp_reset_link_params() without invoking
intel_dp_link_caps_update():

1. In drivers/gpu/drm/i915/display/intel_dp_link_caps.c, the debugfs handlers
i915_dp_force_link_rate_write() and i915_dp_force_lane_count_write() call
intel_dp_reset_link_params() directly before setting forced_params:

	intel_dp_flush_connector_commits(connector);

	intel_dp_reset_link_params(intel_dp);
	link_caps->forced_params.rate = rate;

2. In drivers/gpu/drm/i915/display/intel_dp.c, when intel_dp_detect() runs on
an eDP connector after intel_dp->reset_link_params is set (such as on
suspend/resume, a long HPD pulse in intel_dp_hpd_pulse(), or RX_CAP_CHANGED in
intel_dp_handle_link_service_irq()), it takes the edp_detect() branch rather
than intel_dp_detect_dpcd():

	if (intel_dp_is_edp(intel_dp))
		status = edp_detect(intel_dp);
	else if (intel_digital_port_connected(encoder))
		status = intel_dp_detect_dpcd(intel_dp);
	...
	if (intel_dp->reset_link_params) {
		intel_dp_reset_link_params(intel_dp);
		intel_dp->reset_link_params = false;
	}

Because intel_dp_get_dpcd() also skips intel_dp_update_sink_caps() for eDP,
intel_dp_link_caps_update() is never called on this path.

3. In drivers/gpu/drm/i915/display/intel_dp.c, intel_dp_sync_state() skips
intel_dp_get_dpcd() during system resume when intel_dp->dpcd[DP_DPCD_REV] != 0
(and on eDP) and calls intel_dp_reset_link_params() directly:

	if (crtc_state && intel_dp->dpcd[DP_DPCD_REV] == 0) {
		intel_dp_get_dpcd(intel_dp);
		dpcd_updated = true;
	}

	intel_dp_tunnel_resume(intel_dp, crtc_state, dpcd_updated);

	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.active = true;
	}

Without intel_dp_link_caps_reset() here, won't reduced link_caps->max_limits
and disabled configurations in link_caps->enabled_configs from earlier link
training fallbacks remain stuck on these paths?

[Severity: Low]
Also, since this was the only caller of intel_dp_link_caps_reset(), removing
this call leaves the function definition in
drivers/gpu/drm/i915/display/intel_dp_link_caps.c and its declaration in
drivers/gpu/drm/i915/display/intel_dp_link_caps.h unused:

void intel_dp_link_caps_reset(struct intel_dp_link_caps *link_caps)
{
	reset_max_link_limits_reenable_all(link_caps);
}

If intel_dp_link_caps_reset() were no longer needed here, should its
definition and declaration be removed as well?

> -	intel_dp_tunnel_uhbr_lanes_wa_apply(intel_dp);
> -
> -	intel_dp->link.mst_probed_lane_count = 0;
> -	intel_dp->link.mst_probed_rate = 0;
> -	intel_dp_link_training_reset(intel_dp->link.training);
> +       intel_dp_tunnel_uhbr_lanes_wa_apply(intel_dp);
> +       intel_dp->link.mst_probed_lane_count = 0;
> +       intel_dp->link.mst_probed_rate = 0;
> +       intel_dp_link_training_reset(intel_dp->link.training);

[Severity: Low]
This isn't a bug, but the replacement lines in intel_dp_reset_link_params()
replace the leading tab indentation with spaces. Could these lines keep
standard tab indentation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025113200.25097-1-mail@taterr.org?part=1

✗ LGCI.VerificationFailed: failure for drm/i915/dp: Remove redundant link capabilities reset

[Patchwork]

== Series Details ==

Series: drm/i915/dp: Remove redundant link capabilities reset
URL   : https://patchwork.freedesktop.org/series/176943/
State : failure

== Summary ==

Series author address 'taterraster@gmail.com' is not on the allowlist, which prevents CI from being automatically triggered.
If you want CI to run for this series, ask Patchwork project owners to click 'retest' on the series in Patchwork.
Exception occurred during validation, bailing out!
Build URL: http://gfx-ci.igk.intel.com:8080/job/CI_PW_kernel/191525/ (on built-in)

Re: [PATCH] drm/i915/dp: Remove redundant link capabilities reset

[Jani Nikula]

On Sun, 25 Oct 2026, George Karagounis <taterraster@gmail.com> wrote:
> intel_dp_link_caps_update(reset=true) is already called prior to intel_dp_reset_link_params(), making the explicit call to intel_dp_link_caps_reset() redundant. Remove it as requested by the existing TODO comment to clean up the link parameter reset flow.

Are you testing your changes or is this just using LLM to address TODO
comments in the code?

>
> Signed-off-by: George Karagounis <mail@taterr.org>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index d8c0d9381471..0e753824bc28 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3674,17 +3674,10 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
>  
>  void intel_dp_reset_link_params(struct intel_dp *intel_dp)
>  {
> -	/*
> -	 * TODO: Remove the following reset of link capabilities, as
> -	 * this isn't needed after intel_dp_link_caps_update(reset=true)
> -	 * was called.
> -	 */
> -	intel_dp_link_caps_reset(intel_dp->link.caps);
> -	intel_dp_tunnel_uhbr_lanes_wa_apply(intel_dp);
> -
> -	intel_dp->link.mst_probed_lane_count = 0;
> -	intel_dp->link.mst_probed_rate = 0;
> -	intel_dp_link_training_reset(intel_dp->link.training);
> +       intel_dp_tunnel_uhbr_lanes_wa_apply(intel_dp);
> +       intel_dp->link.mst_probed_lane_count = 0;
> +       intel_dp->link.mst_probed_rate = 0;
> +       intel_dp_link_training_reset(intel_dp->link.training);

Please use checkpatch.

>  }
>  
>  /* Enable backlight PWM and backlight PP control. */

-- 
Jani Nikula, Intel