Re: [PATCH v4 02/15] drm/atomic: Add new atomic_create_state callback to drm_private_obj

[Thomas Zimmermann <tzimmermann@suse.de>]

Am 28.01.26 um 13:43 schrieb Maxime Ripard:
> 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().
>
> 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 <mripard@kernel.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/drm_atomic.c | 18 ++++++++++++++++--
>   include/drm/drm_atomic.h     | 13 +++++++++++++
>   2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 4191a8333fc4ebdfc10f664c837a3f1693eff022..e3029c8f02e5a3698781117bcc80eff98407cf16 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -939,15 +939,29 @@ 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.atomic_create_state yet.
> +	 * For the time being, let's only call reset if the passed state is
> +	 * NULL. Otherwise, we will fallback to the previous behaviour.
> +	 */
> +	if (!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 712f5fb977bff8a15592a3949444d9ac306e6c54..0b1b32bcd2bda1b92299fd369ba7c23b1c2d3dfa 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -259,10 +259,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.
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)