Re: [PATCH 7/9] 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] Mutating hardware state and global device tracking variables inside an atomic_check callback.
--

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

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

This commit adds support for the 10-bit YUV 4:2:2 output format
(MEDIA_BUS_FMT_UYVY10_1X20) to the Rockchip DW HDMI QP encoder. It also
forces 8 bpc when YUV 4:2:2 is in use to ensure the PHY PLL output remains
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

[ ... ]

> @@ -227,7 +228,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;
>  		s->output_type = DRM_MODE_CONNECTOR_HDMIA;
>  		s->output_bpc = conn_state->hdmi.output_bpc;
>  		s->bus_format = ingest_fmt;
>  	} else {

[Severity: High]
This is a pre-existing issue, but does
dw_hdmi_qp_rockchip_encoder_atomic_check() mutate hardware state and global
tracking variables during an atomic check?

In the DRM atomic framework, the atomic_check phase is expected to be
side-effect free. However, this code calls phy_configure(), which writes
directly to the PHY hardware registers, and then modifies the global
hdmi->tmds_char_rate variable.

Could this corrupt the active display configuration if userspace performs a
DRM_MODE_ATOMIC_TEST_ONLY commit or if an atomic commit fails a later
validation stage?

Furthermore, if the device is runtime-suspended, could accessing unclocked
hardware registers here trigger a synchronous abort and hang the system?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=7