Re: [PATCH v7 04/13] drm/i915/dp_mst: Limit M/N ratio for MST

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/i915/dp_mst: M/N mode_valid check mismatches MST compute_config limits
--

commit 67d8f2b2b1195befcc18a05f5eb7b442e2188782
Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

drm/i915/dp_mst: Limit M/N ratio for MST

This commit adds a Link M/Link N ratio check to mst_connector_mode_valid_ctx()
using intel_dp_mode_valid_m_n() to prune display modes that exceed the
hardware M/N ratio limit on MST connectors.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 7023619aaaba9..f00edfb66a10a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1565,6 +1565,11 @@ mst_connector_mode_valid_ctx(struct drm_connector *_connector,
>  					   min_link_bpp_x16,
>  					   bw_overhead_flags);
>  
> +	if (!intel_dp_mode_valid_m_n(intel_dp, mode->clock)) {
> +		*status = MODE_CLOCK_HIGH;
> +		return 0;
> +	}
> +
>  	/*
>  	 * TODO:
>  	 * - Also check if compression would allow for the mode

[Severity: Medium]
Does calling intel_dp_mode_valid_m_n() here match the link configuration
constraints enforced during MST modeset in intel_dp_compute_config_limits()?

In drivers/gpu/drm/i915/display/intel_dp_mst.c, mst_connector_mode_valid_ctx()
evaluates bandwidth and DSC against max_bw_config, whereas
intel_dp_mode_valid_m_n() in drivers/gpu/drm/i915/display/intel_dp.c iterates
over all configurations in INTEL_DP_LINK_CAPS_FILTER_ALL:

drivers/gpu/drm/i915/display/intel_dp.c:intel_dp_mode_valid_m_n() {
	intel_dp_link_caps_iter_start(&iter, intel_dp->link.caps, order,
				      INTEL_DP_LINK_CAPS_FILTER_ALL);
	for_each_dp_link_config(&iter, &link_config) {
		if (intel_dp_can_support_m_n(display, m_n_pixel_clock,
					     link_config.rate)) {
			found = true;

			break;
		}
	}
}

On a UHBR-capable MST topology (for example UHBR10 with a 312.5 MHz symbol
clock, or UHBR13.5 with a 421.875 MHz symbol clock), intel_dp->link.caps also
includes 8b/10b rates such as HBR3 (810 MHz symbol clock). Because HBR3 has a
higher link symbol clock than UHBR10 or UHBR13.5, intel_dp_mode_valid_m_n()
will see that HBR3 satisfies intel_dp_can_support_m_n() and return true even
when max_bw_config violates the M/N ratio limit.

However, during modeset via
mst_stream_compute_config() -> mst_stream_compute_link_for_joined_pipes() ->
mst_stream_compute_config_limits() -> intel_dp_compute_config_limits(),
the MST path narrows limits->link_config_filter to the single highest-BW
max_config before intel_dp_compute_m_n_ratio_limits() is called:

drivers/gpu/drm/i915/display/intel_dp.c:intel_dp_compute_config_limits() {
	if (is_mst || intel_dp->use_max_params) {
		struct intel_dp_link_caps_filter new_filter = INTEL_DP_LINK_CAPS_FILTER_NONE;
		struct intel_dp_link_config max_config;
		...
		if (!intel_dp_get_connector_max_link_config(connector, limits, &max_config))
			return false;

		if (!intel_dp_link_caps_filter_add(link_caps, &new_filter, &max_config))
			return false;

		limits->link_config_filter = new_filter;
	}

	if (!intel_dp_compute_m_n_ratio_limits(intel_dp, conn_state, crtc_state, limits))
		return false;
}

As a result, intel_dp_compute_m_n_ratio_limits() only checks max_config for
MST and rejects the modeset if max_config violates the M/N ratio limit.

Should mst_connector_mode_valid_ctx() check intel_dp_can_support_m_n() against
max_link_clock (matching the rest of mst_connector_mode_valid_ctx() and the
current MST max_config selection), or should intel_dp_compute_m_n_ratio_limits()
filter limits->link_config_filter before intel_dp_get_connector_max_link_config()
picks max_config?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022032247.1378809-1-suraj.kandpal@intel.com?part=4