Re: [PATCH v4 13/14] drm/rockchip: dw_hdmi_qp: Enable YUV420 output format
From: sashiko-bot@kernel.org
Date: 2026-09-02 23:08:12
Also in:
dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Hardware PHY is programmed and driver software state is mutated during the DRM atomic_check phase. -- commit 37fcce32843c9e542a409c0b36a4aca043a38c1a Author: Cristian Ciocaltea [off-list ref] 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. Tested-by: Igor Paunovic <redacted> Signed-off-by: Cristian Ciocaltea <redacted>
quoted hunk ↗ jump to hunk
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@@ -611,7 +611,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master, plat_data.supported_formats = BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) | BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) | - BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422); + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) | + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420);
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in how the
atomic state machine is handled.
In 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;
Does this actively program the hardware PHY and mutate global driver state
(hdmi->tmds_char_rate) during the DRM atomic_check phase?
If userspace issues a DRM atomic test-only commit, it appears this will
overwrite hdmi->tmds_char_rate. Later, when the actual atomic commit is
performed with the same configuration, the early-return condition at the
beginning of the function will evaluate to true:
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;
Will this cause the driver to incorrectly skip configuring the PHY during the
actual commit phase?
Also, could calling phy_configure() outside of the enable paths bypass
expected runtime PM references, risking bus faults if the block is suspended?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-dw-hdmi-qp-yuv-v4-0-fb45bf4147eb@collabora.com?part=13