Hi Maxime,
Thank you for the patch.
On Tue, Sep 08, 2026 at 04:47:06PM +0200, Maxime Ripard wrote:
> 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;
> }
>
> Signed-off-by: Maxime Ripard <[email protected]>
An impressive semantic patch.
Reviewed-by: Laurent Pinchart <[email protected]>
> ---
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c | 15 ++++++---------
> drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 15 ++++++---------
> 2 files changed, 12 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
> b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
> index 8870766b9e54..da2dff9bb317 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
> @@ -710,28 +710,25 @@ static void rcar_du_plane_atomic_destroy_state(struct
> drm_plane *plane,
> {
> __drm_atomic_helper_plane_destroy_state(state);
> kfree(to_rcar_plane_state(state));
> }
>
> -static void rcar_du_plane_reset(struct drm_plane *plane)
> +static struct drm_plane_state *rcar_du_plane_create_state(struct drm_plane
> *plane)
> {
> struct rcar_du_plane_state *state;
>
> - if (plane->state) {
> - rcar_du_plane_atomic_destroy_state(plane, plane->state);
> - plane->state = NULL;
> - }
> -
> state = kzalloc_obj(*state);
> if (state == NULL)
> - return;
> + return ERR_PTR(-ENOMEM);
>
> - __drm_atomic_helper_plane_reset(plane, &state->state);
> + __drm_atomic_helper_plane_state_init(&state->state, plane);
>
> state->hwindex = -1;
> state->source = RCAR_DU_PLANE_MEMORY;
> state->colorkey = RCAR_DU_COLORKEY_NONE;
> +
> + return &state->state;
> }
>
> static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
> struct drm_plane_state *state,
> struct drm_property *property,
> @@ -765,11 +762,11 @@ static int rcar_du_plane_atomic_get_property(struct
> drm_plane *plane,
> }
>
> static const struct drm_plane_funcs rcar_du_plane_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> - .reset = rcar_du_plane_reset,
> + .atomic_create_state = rcar_du_plane_create_state,
> .destroy = drm_plane_cleanup,
> .atomic_duplicate_state = rcar_du_plane_atomic_duplicate_state,
> .atomic_destroy_state = rcar_du_plane_atomic_destroy_state,
> .atomic_set_property = rcar_du_plane_atomic_set_property,
> .atomic_get_property = rcar_du_plane_atomic_get_property,
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> index ae9f381b03c8..4293e792afdb 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> @@ -419,30 +419,27 @@ static void
> rcar_du_vsp_plane_atomic_destroy_state(struct drm_plane *plane,
> {
> __drm_atomic_helper_plane_destroy_state(state);
> kfree(to_rcar_vsp_plane_state(state));
> }
>
> -static void rcar_du_vsp_plane_reset(struct drm_plane *plane)
> +static struct drm_plane_state *rcar_du_vsp_plane_create_state(struct
> drm_plane *plane)
> {
> struct rcar_du_vsp_plane_state *state;
>
> - if (plane->state) {
> - rcar_du_vsp_plane_atomic_destroy_state(plane, plane->state);
> - plane->state = NULL;
> - }
> -
> state = kzalloc_obj(*state);
> if (state == NULL)
> - return;
> + return ERR_PTR(-ENOMEM);
>
> - __drm_atomic_helper_plane_reset(plane, &state->state);
> + __drm_atomic_helper_plane_state_init(&state->state, plane);
> +
> + return &state->state;
> }
>
> static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> - .reset = rcar_du_vsp_plane_reset,
> + .atomic_create_state = rcar_du_vsp_plane_create_state,
> .destroy = drm_plane_cleanup,
> .atomic_duplicate_state = rcar_du_vsp_plane_atomic_duplicate_state,
> .atomic_destroy_state = rcar_du_vsp_plane_atomic_destroy_state,
> };
>
--
Regards,
Laurent Pinchart
