Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] drm/tests: incorrect atomic backoff jump causes loss of CRTC test 
state
--

commit 45b02bdee8c1daea9d271ea9c59930edf078ec24
Author: Nicolas Frattaroli <[email protected]>

drm/tests: hdmi: Add Quick Media Switching tests

This commit adds KUnit tests to validate the HDMI Quick Media Switching
(QMS) functionality in the HDMI state helpers. The tests verify that the
QMS TFR min and max flags are respected.

> diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c 
> b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> index e2d98280ea237..961a4b48afd86 100644
> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
[ ... ]
> +retry_crtc_state:
> +     crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +     if (PTR_ERR(crtc_state) == -EDEADLK) {
> +             drm_atomic_commit_clear(state);
> +             ret = drm_modeset_backoff(&ctx);
> +             if (!ret)
> +                     goto retry_crtc_state;
> +     }
> +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_state);
> +
> +     crtc_state->vrr_enabled = true;
> +     crtc_state->vrr_state.vrr_min_n = 48000;
> +     crtc_state->vrr_state.vrr_min_d = 1001;
> +     /* Equivalent fraction, as an added hurdle */
> +     crtc_state->vrr_state.vrr_max_n = 48000 * 2;
> +     crtc_state->vrr_state.vrr_max_d = 1001 * 2;
> +
> +retry_conn_state:
> +     conn_state = drm_atomic_get_connector_state(state, conn);
> +     if (PTR_ERR(conn_state) == -EDEADLK) {
> +             drm_atomic_commit_clear(state);
> +             ret = drm_modeset_backoff(&ctx);
> +             if (!ret)
> +                     goto retry_conn_state;

[Severity: Medium]
If drm_atomic_get_connector_state() returns -EDEADLK,
drm_atomic_commit_clear() is called. This clears the previously acquired
crtc_state from the atomic transaction.

Because the code jumps back to retry_conn_state instead of retry_crtc_state,
the test does not re-acquire the CRTC state and loses the vrr_enabled and
vrr_state changes.

Could this cause the test to commit an incomplete atomic state? Should this
instead goto retry_crtc_state?

This same pattern appears in the other tests introduced in this patch:
- drm_test_check_hdmi_vrr_qms_fail()
- drm_test_check_hdmi_vrr_qms_nonfixed_fail()
- drm_test_check_hdmi_vrr_qms_tfr_min()
- drm_test_check_hdmi_vrr_qms_no_tfr_max_fail()
- drm_test_check_hdmi_vrr_qms_tfr_max()

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=24

Reply via email to