Re: [PATCH RFC 10/25] drm/rockchip: dw_hdmi_qp: Add VRR support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/rockchip: State mutation in atomic_check breaks DRM semantics
--

commit 0a43e1780a534b1f1de17fd89eca1b9b3bc5f434
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

drm/rockchip: dw_hdmi_qp: Add VRR support

This commit registers the vrr_capable property on the connector for RK3588
and sets it during the encoder's atomic_check callback.

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 9c4cf68d79d50..2b9432aa8c542 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c

[ ... ]

> @@ -238,6 +239,10 @@ dw_hdmi_qp_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
>  		dev_err(hdmi->dev, "Failed to configure phy: %d\n", ret);
>  	}
>  
> +	if (connector->vrr_capable_property)
> +		drm_connector_set_vrr_capable_property(connector,
> +			drm_display_info_is_vrr_capable(&connector->display_info));
> +

[Severity: Medium]
Does mutating global connector state here break DRM semantics?

The atomic_check callback is strictly required to be side-effect free, as
it is used for speculative TEST_ONLY commits. Calling
drm_connector_set_vrr_capable_property() synchronously mutates global,
visible connector state (the property array). This permanently leaks
proposed or temporary state into the visible global state.

Additionally, can this property update be entirely skipped in some paths?
Looking earlier in dw_hdmi_qp_rockchip_encoder_atomic_check():

    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;

If the format is unchanged, the function returns early. This means the
update will be skipped completely. Since userspace typically queries 
connector properties upon hotplug, deferring this property update to 
atomic_check means userspace may read a stale (false) capability during 
its initial probe and mistakenly conclude VRR is unsupported.

>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-vrr-limiter-uapi-v1-0-2fcd7d011646@collabora.com?part=10