Re: [PATCH 5/5] drm/i915/dp: Detect changes in common link parameters

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, May 18, 2026 at 02:24:26PM +0300, Imre Deak wrote:
> Detect DPRX capability changes without a long HPD or RX_CAP_CHANGED
> signal and queue a corresponding link params reset.
> 
> Besides detecting the above unexpected capability changes, this also
> avoids races between queuing and handling a deferred link params reset.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 50 +++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 6c4dadfc35806..dd968c2d9fa64 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -362,19 +362,25 @@ int intel_dp_max_source_lane_count(struct intel_digital_port *dig_port)
>  	return max_lanes;
>  }
>  
> -/* Theoretical max between source and sink */
> -static void intel_dp_set_max_common_lane_count(struct intel_dp *intel_dp)
> +/*
> + * Theoretical max between source and sink.
> + * Return %true if the max common lane count changed.
> + */
> +static bool intel_dp_set_max_common_lane_count(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	int source_max = intel_dp_max_source_lane_count(dig_port);
>  	int sink_max = intel_dp->max_sink_lane_count;
>  	int lane_max = intel_tc_port_max_lane_count(dig_port);
>  	int lttpr_max = drm_dp_lttpr_max_lane_count(intel_dp->lttpr_common_caps);
> +	int old_max_common_lane_count = intel_dp->max_common_lane_count;
>  
>  	if (lttpr_max)
>  		sink_max = min(sink_max, lttpr_max);
>  
>  	intel_dp->max_common_lane_count = min3(source_max, sink_max, lane_max);
> +
> +	return intel_dp->max_common_lane_count != old_max_common_lane_count;
>  }
>  
>  int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
> @@ -792,13 +798,20 @@ int intel_dp_link_config_index(struct intel_dp *intel_dp, int link_rate, int lan
>  	return -1;
>  }
>  
> -static void intel_dp_set_common_rates(struct intel_dp *intel_dp)
> +/* Return %true if the common rates changed. */
> +static bool intel_dp_set_common_rates(struct intel_dp *intel_dp)
>  {
>  	struct intel_display *display = to_intel_display(intel_dp);
> +	int num_old_common_rates = intel_dp->num_common_rates;
> +	int old_common_rates[DP_MAX_SUPPORTED_RATES];
>  
>  	drm_WARN_ON(display->drm,
>  		    !intel_dp->num_source_rates || !intel_dp->num_sink_rates);
>  
> +	static_assert(sizeof(old_common_rates) == sizeof(intel_dp->common_rates));

Could also assert the element size/type match. Maybe (as a followup
later) introduce a proper type for this rates[]+num construct and then
we could just copy the darn thing with a normal assignment and not have
to worry about this kind of stuff at all...

> +	memcpy(old_common_rates, intel_dp->common_rates,
> +	       num_old_common_rates * sizeof(old_common_rates[0]));
> +
>  	intel_dp->num_common_rates = intersect_rates(intel_dp->source_rates,
>  						     intel_dp->num_source_rates,
>  						     intel_dp->sink_rates,
> @@ -810,13 +823,26 @@ static void intel_dp_set_common_rates(struct intel_dp *intel_dp)
>  		intel_dp->common_rates[0] = 162000;
>  		intel_dp->num_common_rates = 1;
>  	}
> +
> +	return num_old_common_rates != intel_dp->num_common_rates ||
> +	       memcmp(old_common_rates, intel_dp->common_rates,
> +		      num_old_common_rates * sizeof(old_common_rates[0]));
>  }
>  
> -static void intel_dp_set_common_link_params(struct intel_dp *intel_dp)
> +/* Return %true if any common link param changed. */
> +static bool intel_dp_set_common_link_params(struct intel_dp *intel_dp)
>  {
> -	intel_dp_set_common_rates(intel_dp);
> -	intel_dp_set_max_common_lane_count(intel_dp);
> +	bool params_changed = false;
> +
> +	if (intel_dp_set_common_rates(intel_dp))
> +		params_changed = true;
> +
> +	if (intel_dp_set_max_common_lane_count(intel_dp))
> +		params_changed = true;
> +
>  	intel_dp_link_config_init(intel_dp);
> +
> +	return params_changed;
>  }
>  
>  bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int link_rate,
> @@ -4911,9 +4937,19 @@ intel_dp_has_sink_count(struct intel_dp *intel_dp)
>  
>  void intel_dp_update_sink_caps(struct intel_dp *intel_dp)
>  {
> +	struct intel_display *display = to_intel_display(intel_dp);
> +
>  	intel_dp_set_sink_rates(intel_dp);
>  	intel_dp_set_max_sink_lane_count(intel_dp);
> -	intel_dp_set_common_link_params(intel_dp);
> +	/*
> +	 * Handle unexpected sink cap changes, or a race between setting
> +	 * the deferred link params flag in the HPD IRQ handler and
> +	 * clearing the flag during connector detect.
> +	 */
> +	if (intel_dp_set_common_link_params(intel_dp) &&
> +	    intel_dp_reset_link_params_defer(intel_dp))
> +		drm_dbg_kms(display->drm,
> +			    "DPRX capabilities changed before long HPD or RX_CAP_CHANGED signal\n");
>  }
>  
>  static bool
> -- 
> 2.49.1

-- 
Ville Syrjälä
Intel