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