Re: [PATCH v5 12/15] drm/rockchip: dw_hdmi_qp: Do not mask error code in atomic_check()

[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 corruption in atomic_check()
--

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

drm/rockchip: dw_hdmi_qp: Do not mask error code in atomic_check()

This commit changes dw_hdmi_qp_rockchip_encoder_atomic_check() to
propagate the error code returned by dw_hdmi_qp_rockchip_get_vop_format()
instead of masking it with -EINVAL. This ensures that -EDEADLK can be
propagated up to the DRM core to trigger the ww_mutex backoff mechanism.

Link: https://lore.kernel.org/all/20260902230603.AF5C61F000E9@smtp.kernel.org/

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index f2a1ea8dfefa5..db1933d75ced5 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -203,7 +203,7 @@ dw_hdmi_qp_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
>  
>  	ingest_fmt = dw_hdmi_qp_rockchip_get_vop_format(encoder, conn_state);
>  	if (ingest_fmt < 0)
> -		return -EINVAL;
> +		return ingest_fmt;
>  
>  	if (hdmi->tmds_char_rate == conn_state->hdmi.tmds_char_rate &&
>  	    s->output_bpc == conn_state->hdmi.output_bpc &&

[Severity: High]
Does this function improperly mutate hardware state during the
validation-only atomic_check phase?

The DRM atomic framework uses atomic_check for configuration validation,
which executes during TEST_ONLY commits that are never applied to hardware.

However, further down in dw_hdmi_qp_rockchip_encoder_atomic_check(), the
code calls phy_configure() and updates global driver state:

	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 {

If userspace issues a TEST_ONLY commit with a new mode, the hardware PHY
state and driver tracking (hdmi->tmds_char_rate) are modified to reflect the
test configuration. 

If the commit is then aborted, the hardware state and driver tracking become
out of sync.

Could this cause a subsequent valid commit to break display output by
incorrectly skipping the necessary configuration due to the fast-path
check shown in the diff above?

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