Re: [PATCH v2 49/78] drm/mediatek: dp: Switch to atomic_create_state

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