From 2bcec01cf10551084cb0de5f8f9ae5d51933331f Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Tue, 8 Sep 2026 16:46:57 +0200 Subject: [PATCH] drm/amdgpu_dm: Convert to atomic_create_state The plane reset implementation creates a custom state subclass, but only initializes a pristine state without resetting any hardware. This is equivalent to what atomic_create_state expects. Convert to it. The conversion was done using the following Coccinelle semantic patch: @@ identifier funcs; symbol drm_atomic_helper_plane_reset; symbol drm_atomic_helper_plane_create_state; @@ struct drm_plane_funcs funcs = { ..., - .reset = drm_atomic_helper_plane_reset, + .atomic_create_state = drm_atomic_helper_plane_create_state, ..., }; @match_struct_reset@ identifier funcs, reset_func; @@ struct drm_plane_funcs funcs = { ..., .reset = reset_func, ..., }; @reset_uses_helpers depends on match_struct_reset@ identifier match_struct_reset.reset_func; @@ void reset_func(...) { <+... ( __drm_atomic_helper_plane_reset(...); | __drm_gem_reset_shadow_plane(...); ) ...+> } @match_struct_destroy@ identifier funcs, destroy_func; @@ struct drm_plane_funcs funcs = { ..., .atomic_destroy_state = destroy_func, ..., }; @script:python renamed_func@ old_name << match_struct_reset.reset_func; new_name; @@ if old_name.endswith("_reset"): coccinelle.new_name = old_name.replace("_reset", "_create_state") else: coccinelle.new_name = old_name @update_struct depends on match_struct_reset && reset_uses_helpers@ identifier match_struct_reset.funcs, match_struct_reset.reset_func; identifier renamed_func.new_name; @@ struct drm_plane_funcs funcs = { ..., - .reset = reset_func, + .atomic_create_state = new_name, ..., }; @drop_destroy depends on update_struct && match_struct_destroy@ identifier match_struct_reset.reset_func; identifier match_struct_destroy.destroy_func; identifier container_func; identifier P; symbol drm_atomic_helper_plane_destroy_state; symbol __drm_atomic_helper_plane_destroy_state; @@ void reset_func(struct drm_plane *P) { ... ( - if (P->state) { - <+... ( - drm_atomic_helper_plane_destroy_state(P, P->state); | - __drm_atomic_helper_plane_destroy_state(P->state); | - P->funcs->atomic_destroy_state(P, P->state); | - destroy_func(P, P->state); ) - ...+> - } | - drm_WARN_ON_ONCE(P->dev, P->state); | - WARN_ON(P->state); ) ... ( - kfree(P->state); | - kfree(container_func(P->state)); | // kfree is optional ) ( - P->state = NULL; | // plane->state clearing is optional ) ... } @drop_destroy_mtk depends on update_struct@ identifier P; symbol __drm_atomic_helper_plane_destroy_state; symbol to_mtk_plane_state; @@ void mtk_plane_reset(struct drm_plane *P) { ... - if (P->state) { - __drm_atomic_helper_plane_destroy_state(P->state); - ... - } else { ... - } ... } @transform_nv50_wndw depends on update_struct@ identifier S; @@ void nv50_wndw_reset(...) { ... - if (WARN_ON(!(S = kzalloc_obj(*S)))) + S = kzalloc_obj(*S); + if (WARN_ON(!S)) return; ... } @transform_kzalloc depends on update_struct@ identifier match_struct_reset.reset_func; identifier P, S; statement ST; statement list STL; @@ void reset_func(struct drm_plane *P) { <... S = kzalloc_obj(*S); ( - if (S) - { - STL - } + if (!S) return; + + STL | - if (S) ST + if (!S) return; + + ST ) ...> } @transform_body depends on update_struct@ identifier match_struct_reset.reset_func; identifier renamed_func.new_name; identifier S, P; expression PS; @@ - void reset_func(struct drm_plane *P) + struct drm_plane_state *new_name(struct drm_plane *P) { ... S = kzalloc_obj(*S); ... ( if (!S) { ... - return; + return ERR_PTR(-ENOMEM); } | if (WARN_ON(!S)) { ... - return; + return ERR_PTR(-ENOMEM); } | if (S == NULL) { ... - return; + return ERR_PTR(-ENOMEM); } ) ... ( - __drm_atomic_helper_plane_reset(P, PS); + __drm_atomic_helper_plane_state_init(PS, P); | - __drm_gem_reset_shadow_plane(P, PS); + __drm_gem_shadow_plane_state_init(P, PS); ) ... } @update_early_return depends on update_struct@ identifier match_struct_reset.reset_func; identifier renamed_func.new_name; identifier P; expression PS; @@ struct drm_plane_state *new_name(struct drm_plane *P) { <+... - return; + return ERR_PTR(-EINVAL); ...+> } @update_return_plane depends on update_struct@ identifier match_struct_reset.reset_func; identifier renamed_func.new_name; identifier P; expression PS; @@ struct drm_plane_state *new_name(struct drm_plane *P) { ... __drm_atomic_helper_plane_state_init(PS, P); ... + + return PS; } @update_return_shadow depends on update_struct@ identifier renamed_func.new_name; identifier P; expression PS; @@ struct drm_plane_state *new_name(struct drm_plane *P) { ... __drm_gem_shadow_plane_state_init(P, PS); ... + + return &PS->base; } Reviewed-by: Leo Li Reviewed-by: Thomas Zimmermann Link: https://patch.msgid.link/20260908-drm-no-more-plane-reset-v4-26-a31b3fcfc989@kernel.org Signed-off-by: Maxime Ripard --- .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 28 +++++++++++-------- .../amd/display/amdgpu_dm/amdgpu_dm_plane.h | 2 +- .../amdgpu_dm/tests/amdgpu_dm_plane_test.c | 28 +++++++------------ 3 files changed, 28 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c index e13b96358208..adfbffae2983 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c @@ -1788,24 +1788,23 @@ static const struct drm_plane_helper_funcs dm_primary_plane_helper_funcs = { .panic_flush = amdgpu_dm_plane_panic_flush, }; -STATIC_IFN_KUNIT void amdgpu_dm_plane_drm_plane_reset(struct drm_plane *plane) +STATIC_IFN_KUNIT struct drm_plane_state *amdgpu_dm_plane_drm_plane_create_state(struct drm_plane *plane) { struct dm_plane_state *amdgpu_state; amdgpu_state = kzalloc_obj(*amdgpu_state); if (!amdgpu_state) - return; + return ERR_PTR(-ENOMEM); - if (plane->state) - plane->funcs->atomic_destroy_state(plane, plane->state); - - __drm_atomic_helper_plane_reset(plane, &amdgpu_state->base); + __drm_atomic_helper_plane_state_init(&amdgpu_state->base, plane); amdgpu_state->degamma_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT; amdgpu_state->hdr_mult = AMDGPU_HDR_MULT_DEFAULT; amdgpu_state->shaper_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT; amdgpu_state->blend_tf = AMDGPU_TRANSFER_FUNCTION_DEFAULT; + + return &amdgpu_state->base; } -EXPORT_IF_KUNIT(amdgpu_dm_plane_drm_plane_reset); +EXPORT_IF_KUNIT(amdgpu_dm_plane_drm_plane_create_state); STATIC_IFN_KUNIT struct drm_plane_state * amdgpu_dm_plane_drm_plane_duplicate_state(struct drm_plane *plane) @@ -2168,7 +2167,7 @@ static const struct drm_plane_funcs dm_plane_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, .destroy = drm_plane_helper_destroy, - .reset = amdgpu_dm_plane_drm_plane_reset, + .atomic_create_state = amdgpu_dm_plane_drm_plane_create_state, .atomic_duplicate_state = amdgpu_dm_plane_drm_plane_duplicate_state, .atomic_destroy_state = amdgpu_dm_plane_drm_plane_destroy_state, .format_mod_supported = amdgpu_dm_plane_format_mod_supported, @@ -2303,9 +2302,16 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm, return res; #endif - /* Create (reset) the plane state */ - if (plane->funcs->reset) - plane->funcs->reset(plane); + /* Create the plane state */ + if (plane->funcs->atomic_create_state) { + struct drm_plane_state *plane_state; + + plane_state = plane->funcs->atomic_create_state(plane); + if (IS_ERR(plane_state)) + return PTR_ERR(plane_state); + + plane->state = plane_state; + } return 0; } diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h index 092ade738ce9..6aaea290206d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h @@ -117,7 +117,7 @@ int amdgpu_dm_plane_atomic_async_check(struct drm_plane *plane, int amdgpu_dm_plane_atomic_check(struct drm_plane *plane, struct drm_atomic_commit *state); void amdgpu_dm_plane_panic_flush(struct drm_plane *plane); -void amdgpu_dm_plane_drm_plane_reset(struct drm_plane *plane); +struct drm_plane_state *amdgpu_dm_plane_drm_plane_create_state(struct drm_plane *plane); struct drm_plane_state *amdgpu_dm_plane_drm_plane_duplicate_state(struct drm_plane *plane); void amdgpu_dm_plane_drm_plane_destroy_state(struct drm_plane *plane, struct drm_plane_state *state); diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c index 23ac8ae41c68..c75a2ef87479 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_plane_test.c @@ -2924,14 +2924,14 @@ static const struct drm_plane_funcs dm_test_plane_reset_funcs = { }; /** - * dm_test_plane_reset_initializes_state() - Verify reset installs default state. + * dm_test_plane_create_state_initializes_state() - Verify create_state allocates default state. * @test: KUnit test context. * - * Verify amdgpu_dm_plane_drm_plane_reset() destroys the existing plane state, - * allocates a fresh dm_plane_state, and initializes the AMD-specific transfer - * function and HDR multiplier defaults. + * Verify amdgpu_dm_plane_drm_plane_create_state() allocates a fresh + * dm_plane_state, and initializes the AMD-specific transfer function and HDR + * multiplier defaults. */ -static void dm_test_plane_reset_initializes_state(struct kunit *test) +static void dm_test_plane_create_state_initializes_state(struct kunit *test) { struct dm_plane_state *old_state; struct dm_plane_state *new_state; @@ -2940,20 +2940,12 @@ static void dm_test_plane_reset_initializes_state(struct kunit *test) plane = kunit_kzalloc(test, sizeof(*plane), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, plane); - /* - * Provide an existing state plus a funcs table so reset exercises the - * destroy-existing-state path. The destroy hook frees this state, so it - * must be a plain (non-KUnit-managed) allocation. - */ - old_state = kzalloc_obj(*old_state); - KUNIT_ASSERT_NOT_NULL(test, old_state); plane->funcs = &dm_test_plane_reset_funcs; - plane->state = &old_state->base; - amdgpu_dm_plane_drm_plane_reset(plane); + plane_state = amdgpu_dm_plane_drm_plane_create_state(plane); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_state); - KUNIT_ASSERT_NOT_NULL(test, plane->state); - new_state = to_dm_plane_state(plane->state); + new_state = to_dm_plane_state(plane_state); KUNIT_EXPECT_EQ(test, new_state->degamma_tf, AMDGPU_TRANSFER_FUNCTION_DEFAULT); KUNIT_EXPECT_EQ(test, new_state->hdr_mult, AMDGPU_HDR_MULT_DEFAULT); KUNIT_EXPECT_EQ(test, new_state->shaper_tf, AMDGPU_TRANSFER_FUNCTION_DEFAULT); @@ -3098,8 +3090,8 @@ static struct kunit_case amdgpu_dm_plane_test_cases[] = { KUNIT_CASE(dm_test_atomic_check_scaling_failure), /* amdgpu_dm_plane_panic_flush() */ KUNIT_CASE(dm_test_panic_flush_no_dc_state), - /* amdgpu_dm_plane_drm_plane_reset() */ - KUNIT_CASE(dm_test_plane_reset_initializes_state), + /* amdgpu_dm_plane_drm_plane_create_state() */ + KUNIT_CASE(dm_test_plane_create_state_initializes_state), /* amdgpu_dm_plane_drm_plane_duplicate_state() */ KUNIT_CASE(dm_test_plane_duplicate_state_copies_fields), /* amdgpu_dm_plane_drm_plane_destroy_state() */