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 <[email protected]> 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/[email protected]?part=10
