Re: [PATCH v3 12/14] 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:

Pre-existing issues:
- [High] The `atomic_check` callback unconditionally mutates global driver state and configures hardware PHY, breaking DRM atomic design principles and corrupting state during `TEST_ONLY` commits.
--

--- Patch [12]: [PATCH v3 12/14] drm/rockchip: dw_hdmi_qp: Support 10-bit YUV422 output format ---
commit 4acf186e6f48344c73efb358bbf5b17c6a065aa5
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

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

This commit adds support for 10-bit YUV422 output via the
MEDIA_BUS_FMT_UYVY10_1X20 format. It also forces 8 bpc for the HDMI PHY
configuration when YUV 4:2:2 is in use 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
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
[ ... ]
> @@ -226,7 +227,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) {
>  		hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;

[Severity: High]
This is a pre-existing issue, but does dw_hdmi_qp_rockchip_encoder_atomic_check
mutate active hardware state and global driver state during TEST_ONLY commits?

Because atomic_check callbacks are executed when userspace issues a
DRM_MODE_ATOMIC_TEST_ONLY ioctl to probe display configurations, calling
phy_configure() here could actively alter the PHY hardware under an active
display.

Additionally, modifying hdmi->tmds_char_rate mutates global driver state
even if the test commit is subsequently discarded by the kernel, potentially
corrupting the active configuration. Should these side effects be deferred to
an atomic_enable or atomic_update callback instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-dw-hdmi-qp-yuv-v3-0-a4a982a9f2e7@collabora.com?part=12