Re: [PATCH v5 13/15] drm/rockchip: dw_hdmi_qp: Support 10-bit YUV422 output format

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/rockchip: dw_hdmi_qp: state leak in atomic_check corrupts PHY bpc configuration
--

commit 197f61f0c784edd007d1f4b375c9dad379eab849
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: dw_hdmi_qp: Support 10-bit YUV422 output format

This patch adds support for 10-bit YUV 4:2:2 output to the Rockchip DW HDMI QP
encoder by using the MEDIA_BUS_FMT_UYVY10_1X20 format. It also forces 8 bpc
when YUV 4:2:2 is used to keep the PHY PLL output aligned with the TMDS
character rate.

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index db1933d75ced5..380dd9ca6870d 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
[ ... ]
> @@ -229,7 +230,14 @@ dw_hdmi_qp_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
>  	}
>  
>  	phy_cfg.hdmi.tmds_char_rate = conn_state->hdmi.tmds_char_rate;
> -	phy_cfg.hdmi.bpc = conn_state->hdmi.output_bpc;
> +	/*
> +	 * YUV422 always transmits two 12-bit components per clock cycle,
> +	 * regardless of the color depth, which from a rate perspective is
> +	 * equivalent to three 8-bit RGB components.  Force 8 bpc here to
> +	 * keep the PHY PLL output aligned with the TMDS character rate.
> +	 */
> +	phy_cfg.hdmi.bpc = (s->output_mode == ROCKCHIP_OUT_MODE_YUV422 ?
> +				8 : conn_state->hdmi.output_bpc);
>  
>  	ret = phy_configure(hdmi->phy, &phy_cfg);
>  	if (!ret) {

[Severity: High]
Does calling phy_configure() in this atomic check function leak test-only
state to the PHY driver?

Because dw_hdmi_qp_rockchip_encoder_atomic_check() executes during
DRM_MODE_ATOMIC_TEST_ONLY commits, calling phy_configure() here updates the
PHY driver's cached state (like hdmi_cfg.bpc) with test configurations. It
also mutates the global encoder state when it sets hdmi->tmds_char_rate on
success.

If a TEST_ONLY commit changes output_bpc while tmds_char_rate remains
unchanged, the new bpc is cached by the PHY.

A subsequent full modeset on the active state could then hit the early
return earlier in this function:

	if (hdmi->tmds_char_rate == conn_state->hdmi.tmds_char_rate &&
	    s->output_bpc == conn_state->hdmi.output_bpc &&
	    s->bus_format == ingest_fmt)
		return 0;

This would bypass phy_configure() and allow phy_power_on() to apply the
leaked TEST_ONLY bpc state, potentially desynchronizing the PHY PLLs and
corrupting display output.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-dw-hdmi-qp-yuv-v5-0-6fab0fe43a9b@collabora.com?part=13