lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b1601f73-3ce5-4f29-a20d-1920be935e0f@suse.de>
Date: Wed, 8 Oct 2025 16:52:53 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Maxime Ripard <mripard@...nel.org>,
 Harry Wentland <harry.wentland@....com>, Leo Li <sunpeng.li@....com>,
 Rodrigo Siqueira <siqueira@...lia.com>,
 Alex Deucher <alexander.deucher@....com>,
 Christian König <christian.koenig@....com>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
Cc: amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/atomic: Change state pointers to a more meaningful
 name



Am 08.10.25 um 15:11 schrieb Maxime Ripard:
> The state pointer found in the struct drm_atomic_state internals for
> most object is a bit ambiguous, and confusing when those internals also
> have old state and new state.
>
> After the recent cleanups, the state pointer only use is to point to the
> state we need to free when destroying the atomic state.
>
> We can thus rename it something less ambiguous, and hopefully more
> meaningful.
>
> Signed-off-by: Maxime Ripard <mripard@...nel.org>

Acked-by: Thomas Zimmermann <tzimmermann@...e.de>

> ---
> Changes in v2:
> - Switch from freeable_state to state_to_destroy
> - Link to v1: https://lore.kernel.org/r/20251006-drm-rename-state-v1-1-5b7c4154772b@kernel.org
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 ++--
>   drivers/gpu/drm/drm_atomic.c                      | 24 +++++++++++------------
>   drivers/gpu/drm/drm_atomic_helper.c               |  8 ++++----
>   include/drm/drm_atomic.h                          | 16 +++++++--------
>   4 files changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 62defeccbb5ca09c89523fc4112d2085bbdbb0a9..275e237c1058b76640c8dd36443b034c6c71f84f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -12335,22 +12335,22 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>   
>   			if (obj->funcs == adev->dm.atomic_obj.funcs) {
>   				int j = state->num_private_objs-1;
>   
>   				dm_atomic_destroy_state(obj,
> -						state->private_objs[i].state);
> +						state->private_objs[i].state_to_destroy);
>   
>   				/* If i is not at the end of the array then the
>   				 * last element needs to be moved to where i was
>   				 * before the array can safely be truncated.
>   				 */
>   				if (i != j)
>   					state->private_objs[i] =
>   						state->private_objs[j];
>   
>   				state->private_objs[j].ptr = NULL;
> -				state->private_objs[j].state = NULL;
> +				state->private_objs[j].state_to_destroy = NULL;
>   				state->private_objs[j].old_state = NULL;
>   				state->private_objs[j].new_state = NULL;
>   
>   				state->num_private_objs = j;
>   				break;
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 0fda567c390057b10bce691d9ddc11308088d92e..be2cb6e43cb07fbe553d1ab875911253be628d1a 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -205,13 +205,13 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
>   
>   		if (!connector)
>   			continue;
>   
>   		connector->funcs->atomic_destroy_state(connector,
> -						       state->connectors[i].state);
> +						       state->connectors[i].state_to_destroy);
>   		state->connectors[i].ptr = NULL;
> -		state->connectors[i].state = NULL;
> +		state->connectors[i].state_to_destroy = NULL;
>   		state->connectors[i].old_state = NULL;
>   		state->connectors[i].new_state = NULL;
>   		drm_connector_put(connector);
>   	}
>   
> @@ -220,14 +220,14 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
>   
>   		if (!crtc)
>   			continue;
>   
>   		crtc->funcs->atomic_destroy_state(crtc,
> -						  state->crtcs[i].state);
> +						  state->crtcs[i].state_to_destroy);
>   
>   		state->crtcs[i].ptr = NULL;
> -		state->crtcs[i].state = NULL;
> +		state->crtcs[i].state_to_destroy = NULL;
>   		state->crtcs[i].old_state = NULL;
>   		state->crtcs[i].new_state = NULL;
>   
>   		if (state->crtcs[i].commit) {
>   			drm_crtc_commit_put(state->crtcs[i].commit);
> @@ -240,24 +240,24 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
>   
>   		if (!plane)
>   			continue;
>   
>   		plane->funcs->atomic_destroy_state(plane,
> -						   state->planes[i].state);
> +						   state->planes[i].state_to_destroy);
>   		state->planes[i].ptr = NULL;
> -		state->planes[i].state = NULL;
> +		state->planes[i].state_to_destroy = NULL;
>   		state->planes[i].old_state = NULL;
>   		state->planes[i].new_state = NULL;
>   	}
>   
>   	for (i = 0; i < state->num_private_objs; i++) {
>   		struct drm_private_obj *obj = state->private_objs[i].ptr;
>   
>   		obj->funcs->atomic_destroy_state(obj,
> -						 state->private_objs[i].state);
> +						 state->private_objs[i].state_to_destroy);
>   		state->private_objs[i].ptr = NULL;
> -		state->private_objs[i].state = NULL;
> +		state->private_objs[i].state_to_destroy = NULL;
>   		state->private_objs[i].old_state = NULL;
>   		state->private_objs[i].new_state = NULL;
>   	}
>   	state->num_private_objs = 0;
>   
> @@ -359,11 +359,11 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
>   
>   	crtc_state = crtc->funcs->atomic_duplicate_state(crtc);
>   	if (!crtc_state)
>   		return ERR_PTR(-ENOMEM);
>   
> -	state->crtcs[index].state = crtc_state;
> +	state->crtcs[index].state_to_destroy = crtc_state;
>   	state->crtcs[index].old_state = crtc->state;
>   	state->crtcs[index].new_state = crtc_state;
>   	state->crtcs[index].ptr = crtc;
>   	crtc_state->state = state;
>   
> @@ -544,11 +544,11 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
>   
>   	plane_state = plane->funcs->atomic_duplicate_state(plane);
>   	if (!plane_state)
>   		return ERR_PTR(-ENOMEM);
>   
> -	state->planes[index].state = plane_state;
> +	state->planes[index].state_to_destroy = plane_state;
>   	state->planes[index].ptr = plane;
>   	state->planes[index].old_state = plane->state;
>   	state->planes[index].new_state = plane_state;
>   	plane_state->state = state;
>   
> @@ -856,11 +856,11 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
>   
>   	obj_state = obj->funcs->atomic_duplicate_state(obj);
>   	if (!obj_state)
>   		return ERR_PTR(-ENOMEM);
>   
> -	state->private_objs[index].state = obj_state;
> +	state->private_objs[index].state_to_destroy = obj_state;
>   	state->private_objs[index].old_state = obj->state;
>   	state->private_objs[index].new_state = obj_state;
>   	state->private_objs[index].ptr = obj;
>   	obj_state->state = state;
>   
> @@ -1159,11 +1159,11 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
>   	connector_state = connector->funcs->atomic_duplicate_state(connector);
>   	if (!connector_state)
>   		return ERR_PTR(-ENOMEM);
>   
>   	drm_connector_get(connector);
> -	state->connectors[index].state = connector_state;
> +	state->connectors[index].state_to_destroy = connector_state;
>   	state->connectors[index].old_state = connector->state;
>   	state->connectors[index].new_state = connector_state;
>   	state->connectors[index].ptr = connector;
>   	connector_state->state = state;
>   
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index d5ebe6ea0acbc5a08aef7fa41ecb9ed5d8fa8e80..5a473a274ff06d7ab83039e0a6328e1372b80a00 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3234,21 +3234,21 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
>   		WARN_ON(connector->state != old_conn_state);
>   
>   		old_conn_state->state = state;
>   		new_conn_state->state = NULL;
>   
> -		state->connectors[i].state = old_conn_state;
> +		state->connectors[i].state_to_destroy = old_conn_state;
>   		connector->state = new_conn_state;
>   	}
>   
>   	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
>   		WARN_ON(crtc->state != old_crtc_state);
>   
>   		old_crtc_state->state = state;
>   		new_crtc_state->state = NULL;
>   
> -		state->crtcs[i].state = old_crtc_state;
> +		state->crtcs[i].state_to_destroy = old_crtc_state;
>   		crtc->state = new_crtc_state;
>   
>   		if (new_crtc_state->commit) {
>   			spin_lock(&crtc->commit_lock);
>   			list_add(&new_crtc_state->commit->commit_entry,
> @@ -3264,22 +3264,22 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
>   		WARN_ON(plane->state != old_plane_state);
>   
>   		old_plane_state->state = state;
>   		new_plane_state->state = NULL;
>   
> -		state->planes[i].state = old_plane_state;
> +		state->planes[i].state_to_destroy = old_plane_state;
>   		plane->state = new_plane_state;
>   	}
>   	drm_panic_unlock(state->dev, flags);
>   
>   	for_each_oldnew_private_obj_in_state(state, obj, old_obj_state, new_obj_state, i) {
>   		WARN_ON(obj->state != old_obj_state);
>   
>   		old_obj_state->state = state;
>   		new_obj_state->state = NULL;
>   
> -		state->private_objs[i].state = old_obj_state;
> +		state->private_objs[i].state_to_destroy = old_obj_state;
>   		obj->state = new_obj_state;
>   	}
>   
>   	return 0;
>   }
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index c8ab2163bf658cd06b12a8dabada7c088a328654..155e82f87e4d47161475b57fc28762d7ba8fd206 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -159,11 +159,11 @@ struct drm_crtc_commit {
>   
>   struct __drm_planes_state {
>   	struct drm_plane *ptr;
>   
>   	/**
> -	 * @state:
> +	 * @state_to_destroy:
>   	 *
>   	 * Used to track the @drm_plane_state we will need to free when
>   	 * tearing down the associated &drm_atomic_state in
>   	 * $drm_mode_config_funcs.atomic_state_clear or
>   	 * drm_atomic_state_default_clear().
> @@ -171,20 +171,20 @@ struct __drm_planes_state {
>   	 * Before a commit, and the call to
>   	 * drm_atomic_helper_swap_state() in particular, it points to
>   	 * the same state than @new_state. After a commit, it points to
>   	 * the same state than @old_state.
>   	 */
> -	struct drm_plane_state *state;
> +	struct drm_plane_state *state_to_destroy;
>   
>   	struct drm_plane_state *old_state, *new_state;
>   };
>   
>   struct __drm_crtcs_state {
>   	struct drm_crtc *ptr;
>   
>   	/**
> -	 * @state:
> +	 * @state_to_destroy:
>   	 *
>   	 * Used to track the @drm_crtc_state we will need to free when
>   	 * tearing down the associated &drm_atomic_state in
>   	 * $drm_mode_config_funcs.atomic_state_clear or
>   	 * drm_atomic_state_default_clear().
> @@ -192,11 +192,11 @@ struct __drm_crtcs_state {
>   	 * Before a commit, and the call to
>   	 * drm_atomic_helper_swap_state() in particular, it points to
>   	 * the same state than @new_state. After a commit, it points to
>   	 * the same state than @old_state.
>   	 */
> -	struct drm_crtc_state *state;
> +	struct drm_crtc_state *state_to_destroy;
>   
>   	struct drm_crtc_state *old_state, *new_state;
>   
>   	/**
>   	 * @commit:
> @@ -214,11 +214,11 @@ struct __drm_crtcs_state {
>   
>   struct __drm_connnectors_state {
>   	struct drm_connector *ptr;
>   
>   	/**
> -	 * @state:
> +	 * @state_to_destroy:
>   	 *
>   	 * Used to track the @drm_connector_state we will need to free
>   	 * when tearing down the associated &drm_atomic_state in
>   	 * $drm_mode_config_funcs.atomic_state_clear or
>   	 * drm_atomic_state_default_clear().
> @@ -226,11 +226,11 @@ struct __drm_connnectors_state {
>   	 * Before a commit, and the call to
>   	 * drm_atomic_helper_swap_state() in particular, it points to
>   	 * the same state than @new_state. After a commit, it points to
>   	 * the same state than @old_state.
>   	 */
> -	struct drm_connector_state *state;
> +	struct drm_connector_state *state_to_destroy;
>   
>   	struct drm_connector_state *old_state, *new_state;
>   
>   	/**
>   	 * @out_fence_ptr:
> @@ -391,11 +391,11 @@ struct drm_private_state {
>   
>   struct __drm_private_objs_state {
>   	struct drm_private_obj *ptr;
>   
>   	/**
> -	 * @state:
> +	 * @state_to_destroy:
>   	 *
>   	 * Used to track the @drm_private_state we will need to free
>   	 * when tearing down the associated &drm_atomic_state in
>   	 * $drm_mode_config_funcs.atomic_state_clear or
>   	 * drm_atomic_state_default_clear().
> @@ -403,11 +403,11 @@ struct __drm_private_objs_state {
>   	 * Before a commit, and the call to
>   	 * drm_atomic_helper_swap_state() in particular, it points to
>   	 * the same state than @new_state. After a commit, it points to
>   	 * the same state than @old_state.
>   	 */
> -	struct drm_private_state *state;
> +	struct drm_private_state *state_to_destroy;
>   
>   	struct drm_private_state *old_state, *new_state;
>   };
>   
>   /**
>
> ---
> base-commit: 7a031e8d3528ba0860d282ffd3c88fbda4bf8c4c
> change-id: 20251006-drm-rename-state-b2b0fed05f82
>
> Best regards,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ