Re: [PATCH v3 13/14] drm/rockchip: dw_hdmi_qp: Enable YUV420 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] Modifying hardware or global driver state within atomic_check callback.
--

--- Patch [13]: [PATCH v3 13/14] drm/rockchip: dw_hdmi_qp: Enable YUV420 output format ---
commit 886f3880152ce2fc09e568484e921ca99dad89d4
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: dw_hdmi_qp: Enable YUV420 output format

Both RK3576 and RK3588 SoCs are capable of driving the YUV420 output
color format, and the required bus-format handling and VOP2 support are
already in place.  Advertise it via the platform supported formats so
the HDMI core can select it.

YUV420 halves the TMDS bandwidth compared to RGB/YUV444, which enables
high-resolution modes such as 4K@60Hz on links that cannot otherwise
carry the full-bandwidth signal.

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 78473f9a0fce3..1af29066a0a31 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but I noticed something concerning
in dw_hdmi_qp_rockchip_encoder_atomic_check() in this same file:

drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rockchip_encoder_atomic_check() {
    ...
	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 {
    ...
}

Does this code accidentally configure the hardware PHY and modify global
driver state (hdmi->tmds_char_rate) during the atomic_check phase?

When userspace performs an atomic modeset with the DRM_MODE_ATOMIC_TEST_ONLY
flag, dw_hdmi_qp_rockchip_encoder_atomic_check() is invoked to validate the
state. Modifying hardware or global state here breaks the guarantee that
atomic test-only commits are free of side effects. This could cause the
hardware state to desynchronize from the DRM core's expectations.

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