Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Modifying persistent driver state in atomic_check callback. -- --- Patch [49]: [PATCH v2 49/78] drm/mediatek: dp: Switch to atomic_create_state --- commit b93fba0abd0877816145c393b463fd7e217d61b5 Author: Maxime Ripard <[email protected]> drm/mediatek: dp: Switch to atomic_create_state The drm_bridge_funcs.atomic_reset callback and its drm_atomic_helper_bridge_reset() helper are deprecated. Switch to the atomic_create_state callback and its counterpart. > diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c > b/drivers/gpu/drm/mediatek/mtk_dp.c > index eefbc7e0f9c8d..f656b85b84212 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dp.c > +++ b/drivers/gpu/drm/mediatek/mtk_dp.c > @@ -2578,7 +2578,7 @@ static const struct drm_bridge_funcs > mtk_dp_bridge_funcs = { > .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > .atomic_get_output_bus_fmts = mtk_dp_bridge_atomic_get_output_bus_fmts, > .atomic_get_input_bus_fmts = mtk_dp_bridge_atomic_get_input_bus_fmts, > - .atomic_reset = drm_atomic_helper_bridge_reset, > + .atomic_create_state = drm_atomic_helper_bridge_create_state, [Severity: High] This isn't a bug introduced by this patch, but reviewing changes to the bridge functions struct prompted a check of the other callbacks. Is it safe for mtk_dp_bridge_atomic_check() to modify the persistent mtk_dp->info state directly? mtk_dp_bridge_atomic_check() { ... mtk_dp->info.format = DP_PIXELFORMAT_YUV422; ... drm_display_mode_to_videomode(&crtc_state->adjusted_mode, &mtk_dp->info.vm); ... } If a userspace compositor issues a test-only commit with a different display mode, atomic_check runs and mutates this persistent state. If the system then suspends and resumes, drm_atomic_helper_resume directly commits the saved pre-suspend state without re-running atomic_check. Could this cause the bridge's atomic_enable callback to program the hardware using the poisoned mtk_dp->info parameters from the aborted test commit instead of the actual restored state? > .attach = mtk_dp_bridge_attach, > .detach = mtk_dp_bridge_detach, > .atomic_enable = mtk_dp_bridge_atomic_enable, -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=49
