On Tue, Oct 14, 2025 at 11:31:47AM +0200, Maxime Ripard wrote: > The drm_private_obj initialization was inconsistent with the rest of the > KMS objects. Indeed, it required to pass a preallocated state in > drm_private_obj_init(), while all the others objects would have a reset > callback that would be called later on to create the state. > > However, reset really is meant to reset the hardware and software state. > That it creates an initial state is a side-effect that has been used in > all objects but drm_private_obj. This is made more complex since some > drm_private_obj, the DisplayPort ones in particular, need to be > persistent across and suspend/resume cycle, and such a cycle would call > drm_mode_config_reset().
Doesn't that mean that we need to save private objects's state in drm_atomic_helper_duplicate_state() and restore it in drm_atomic_helper_commit_duplicated_state()? Private objects don't have .atomic_commit() callbacks, but they can be used by other objects during drm_atomic_commit(). > Thus, we need to add a new callback to allocate a pristine state for a > given private object. > > This discussion has also came up during the atomic state readout > discussion, so it might be introduced into the other objects later on. > > Until all drivers are converted to that new allocation pattern, we will > only call it if the passed state is NULL. This will be removed > eventually. > > Signed-off-by: Maxime Ripard <[email protected]> > --- > drivers/gpu/drm/drm_atomic.c | 20 ++++++++++++++++++-- > include/drm/drm_atomic.h | 13 +++++++++++++ > 2 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index > a5c5617266ae1dfe6038baeee6dfa3828c626683..36b56c71cb4e1ddc57577df724efe7d89b4fb6a9 > 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -793,15 +793,31 @@ int drm_atomic_private_obj_init(struct drm_device *dev, > memset(obj, 0, sizeof(*obj)); > > drm_modeset_lock_init(&obj->lock); > > obj->dev = dev; > - obj->state = state; > obj->funcs = funcs; > list_add_tail(&obj->head, &dev->mode_config.privobj_list); > > - state->obj = obj; > + /* > + * Not all users of drm_atomic_private_obj_init have been > + * converted to using &drm_private_obj_funcs.reset yet. For the > + * time being, let's only call reset if the passed state is > + * NULL. Otherwise, we will fallback to the previous behaviour. This comment does no longer reflect the code. > + */ > + if (!state) { > + if (obj->funcs->atomic_create_state) { > + state = obj->funcs->atomic_create_state(obj); > + if (IS_ERR(state)) > + return PTR_ERR(state); > + > + obj->state = state; > + } > + } else { > + obj->state = state; > + state->obj = obj; > + } > > return 0; > } > EXPORT_SYMBOL(drm_atomic_private_obj_init); > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index > 9b3fb98b1e88c38877abdcb9df4d1c9540768833..10a71c4b6afc316f07023756be4cd3ed1d1d2974 > 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -205,10 +205,23 @@ struct drm_private_state; > * added to the atomic states is expected to have an implementation of these > * hooks and pass a pointer to its drm_private_state_funcs struct to > * drm_atomic_get_private_obj_state(). > */ > struct drm_private_state_funcs { > + /** > + * @atomic_create_state: > + * > + * Allocates a pristine, initialized, state for the private > + * object and returns it. > + * > + * RETURNS: > + * > + * A new, pristine, private state instance or an error pointer > + * on failure. > + */ > + struct drm_private_state *(*atomic_create_state)(struct drm_private_obj > *obj); > + > /** > * @atomic_duplicate_state: > * > * Duplicate the current state of the private object and return it. It > * is an error to call this before obj->state has been initialized. > > -- > 2.51.0 > -- With best wishes Dmitry
