On Mon, Oct 06, 2025 at 03:22:26PM +0200, Maxime Ripard wrote: > 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.
I would have perhaps just gone for '_state' to indicate to people that it's very private. But I guess we could have something a bit more verbose as well. 'freeable_state' doen't really give me the right vibes however. Maybe 'state_to_free' or 'state_to_destroy' or someting like that? > > Signed-off-by: Maxime Ripard <[email protected]> > --- > 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..b9036b59a671b2802fae28db623f020bbc535837 > 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].freeable_state); > > /* 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].freeable_state = 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..f3e6a3fc0fdd0a7ad77209dcae1be59759e2a7c1 > 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].freeable_state); > state->connectors[i].ptr = NULL; > - state->connectors[i].state = NULL; > + state->connectors[i].freeable_state = 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].freeable_state); > > state->crtcs[i].ptr = NULL; > - state->crtcs[i].state = NULL; > + state->crtcs[i].freeable_state = 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].freeable_state); > state->planes[i].ptr = NULL; > - state->planes[i].state = NULL; > + state->planes[i].freeable_state = 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].freeable_state); > state->private_objs[i].ptr = NULL; > - state->private_objs[i].state = NULL; > + state->private_objs[i].freeable_state = 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].freeable_state = 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].freeable_state = 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].freeable_state = 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].freeable_state = 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..7b9cee78f0969ee9bd66ac7f28cbe39c747f7ab2 > 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].freeable_state = 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].freeable_state = 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].freeable_state = 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].freeable_state = 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..cdb76ed0bcc08b82b728747f915f064f1ddbcbf7 > 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: > + * @freeable_state: > * > * 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 *freeable_state; > > struct drm_plane_state *old_state, *new_state; > }; > > struct __drm_crtcs_state { > struct drm_crtc *ptr; > > /** > - * @state: > + * @freeable_state: > * > * 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 *freeable_state; > > 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: > + * @freeable_state: > * > * 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 *freeable_state; > > 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: > + * @freeable_state: > * > * 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 *freeable_state; > > struct drm_private_state *old_state, *new_state; > }; > > /** > > --- > base-commit: 7a031e8d3528ba0860d282ffd3c88fbda4bf8c4c > change-id: 20251006-drm-rename-state-b2b0fed05f82 > > Best regards, > -- > Maxime Ripard <[email protected]> -- Ville Syrjälä Intel
