Re: [PATCH 15/24] drm/msm/dp: do not reject wide-bus modes while a YUV420 mode is active

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The state-dependent mode validation bug fixed in `msm_dp_bridge_mode_valid` was missed in its eDP counterpart `msm_edp_bridge_mode_valid`.
- [High] Halving `mode_pclk_khz` for RGB wide-bus modes causes the DP link bandwidth requirement to be incorrectly evaluated as 50% of its true value, allowing invalid modes to pass validation.
--

--- Patch [15]: [PATCH 15/24] drm/msm/dp: do not reject wide-bus modes while a YUV420 mode is active ---
commit 6dbb162c934d82f2d73ac5d225ee02599200aeac
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Subject: drm/msm/dp: do not reject wide-bus modes while a YUV420 mode is active

This commit fixes a bug in msm_dp_bridge_mode_valid() where candidate modes
requiring wide bus to stay under DP_MAX_PIXEL_CLK_KHZ were wrongly rejected
if the currently active mode used YUV 420 output. It replaces the
state-dependent msm_dp_wide_bus_available() check with a direct check of
wide_bus_supported.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -722,7 +722,7 @@ enum drm_mode_status msm_dp_bridge_mode_valid(struct drm_bridge *bridge,
>  	if (is_yuv_420 && !msm_dp_display->panel->vsc_sdp_supported)
>  		return MODE_NO_420;
>  
> -	if (is_yuv_420 || msm_dp_wide_bus_available(dp))
> +	if (is_yuv_420 || msm_dp_display->wide_bus_supported)
>  		mode_pclk_khz /= 2;

[Severity: High]
This isn't a bug introduced by this patch, but should the same fix be
applied to the eDP counterpart?

Looking at msm_edp_bridge_mode_valid() in drivers/gpu/drm/msm/dp/dp_drm.c:

	if (msm_dp_wide_bus_available(dp))
		mode_pclk_khz /= 2;

Since eDP bridges also have ycbcr_420_allowed = true, could an eDP 
sink enter a YUV 420 mode and cause subsequent candidate eDP modes requiring 
wide bus to be incorrectly rejected?

[Severity: High]
This is a pre-existing issue, but does halving mode_pclk_khz here corrupt 
the link bandwidth validation further down in this function?

After mode_pclk_khz is halved, it is used to calculate mode_rate_khz:

	mode_bpp = msm_dp_panel_get_mode_bpp(msm_dp_display->panel,
			mode_bpp, mode_pclk_khz);

	mode_rate_khz = mode_pclk_khz * mode_bpp;
	supported_rate_khz = link_info->num_lanes * link_info->rate * 8;

	if (mode_rate_khz > supported_rate_khz)
		return MODE_BAD;

Since the physical DP serial link over the cable does not compress RGB data,
doesn't the physical link still need to transmit the full clock rate? Does 
calculating mode_rate_khz with the halved clock allow modes to pass that 
exceed the physical DP lanes' capacity?

>  
>  	if (mode_pclk_khz > DP_MAX_PIXEL_CLK_KHZ)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-drm-msm-display-interface-v1-0-368c10fe62fd@oss.qualcomm.com?part=15