[PATCH v2 02/68] drm/crtc: Introduce drmm_crtc_init_with_planes

[Thomas Zimmermann]

Hi

Am 22.06.22 um 16:31 schrieb Maxime Ripard:
> The DRM-managed function to register a CRTC is
> drmm_crtc_alloc_with_planes(), which will allocate the underlying
> structure and initialisation the CRTC.
> 
> However, we might want to separate the structure creation and the CRTC
> initialisation, for example if the structure is shared across multiple
> DRM entities, for example an encoder and a connector.
> 
> Let's create an helper to only initialise a CRTC that would be passed as
> an argument.
> 
> Signed-off-by: Maxime Ripard <maxime at cerno.tech>
> ---
>   drivers/gpu/drm/drm_crtc.c | 75 ++++++++++++++++++++++++++++++++------
>   include/drm/drm_crtc.h     |  6 +++
>   2 files changed, 69 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 26a77a735905..6c1fb42ecf19 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -341,9 +341,10 @@ static int __drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *
>    * The @primary and @cursor planes are only relevant for legacy uAPI, see
>    * &drm_crtc.primary and &drm_crtc.cursor.
>    *
> - * Note: consider using drmm_crtc_alloc_with_planes() instead of
> - * drm_crtc_init_with_planes() to let the DRM managed resource infrastructure
> - * take care of cleanup and deallocation.
> + * Note: consider using drmm_crtc_alloc_with_planes() or
> + * drmm_crtc_init_with_planes() instead of drm_crtc_init_with_planes()
> + * to let the DRM managed resource infrastructure take care of cleanup
> + * and deallocation.
>    *
>    * Returns:
>    * Zero on success, error code on failure.
> @@ -368,14 +369,69 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
>   }
>   EXPORT_SYMBOL(drm_crtc_init_with_planes);
>   
> -static void drmm_crtc_alloc_with_planes_cleanup(struct drm_device *dev,
> -						void *ptr)
> +static void drmm_crtc_init_with_planes_cleanup(struct drm_device *dev,
> +					       void *ptr)
>   {
>   	struct drm_crtc *crtc = ptr;
>   
>   	drm_crtc_cleanup(crtc);
>   }
>   
> +/**
> + * drmm_crtc_init_with_planes - Initialise a new CRTC object with
> + *    specified primary and cursor planes.
> + * @dev: DRM device
> + * @crtc: CRTC object to init
> + * @primary: Primary plane for CRTC
> + * @cursor: Cursor plane for CRTC
> + * @funcs: callbacks for the new CRTC
> + * @name: printf style format string for the CRTC name, or NULL for default name
> + *
> + * Inits a new object created as base part of a driver crtc object. Drivers
> + * should use this function instead of drm_crtc_init(), which is only provided
> + * for backwards compatibility with drivers which do not yet support universal
> + * planes). For really simple hardware which has only 1 plane look at
> + * drm_simple_display_pipe_init() instead.
> + *
> + * Cleanup is automatically handled through registering
> + * drmm_crtc_cleanup() with drmm_add_action(). The crtc structure should
> + * be allocated with drmm_kzalloc().
> + *
> + * The @drm_crtc_funcs.destroy hook must be NULL.
> + *
> + * The @primary and @cursor planes are only relevant for legacy uAPI, see
> + * &drm_crtc.primary and &drm_crtc.cursor.
> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drmm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> +			       struct drm_plane *primary,
> +			       struct drm_plane *cursor,
> +			       const struct drm_crtc_funcs *funcs,
> +			       const char *name, ...)
> +{
> +	va_list ap;
> +	int ret;
> +
> +	WARN_ON(funcs && funcs->destroy);
> +
> +	va_start(ap, name);
> +	ret = __drm_crtc_init_with_planes(dev, crtc, primary, cursor, funcs,
> +					  name, ap);
> +	va_end(ap);
> +	if (ret)
> +		return ret;
> +
> +	ret = drmm_add_action_or_reset(dev, drmm_crtc_init_with_planes_cleanup,
> +				       crtc);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drmm_crtc_init_with_planes);
> +
>   void *__drmm_crtc_alloc_with_planes(struct drm_device *dev,
>   				    size_t size, size_t offset,
>   				    struct drm_plane *primary,
> @@ -398,17 +454,12 @@ void *__drmm_crtc_alloc_with_planes(struct drm_device *dev,
>   	crtc = container + offset;
>   
>   	va_start(ap, name);
> -	ret = __drm_crtc_init_with_planes(dev, crtc, primary, cursor, funcs,
> -					  name, ap);
> +	ret = drmm_crtc_init_with_planes(dev, crtc, primary, cursor, funcs,
> +					 name, ap);

You cannot pass ap as final argument here. There needs to be an internal 
helper, say __drmm_crtc_init_with_planes(), that specifically takes the 
va pointer.

The rest of the code looks good.

Best regards
Thomas

>   	va_end(ap);
>   	if (ret)
>   		return ERR_PTR(ret);
>   
> -	ret = drmm_add_action_or_reset(dev, drmm_crtc_alloc_with_planes_cleanup,
> -				       crtc);
> -	if (ret)
> -		return ERR_PTR(ret);
> -
>   	return container;
>   }
>   EXPORT_SYMBOL(__drmm_crtc_alloc_with_planes);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index a70baea0636c..2babd5cffbf3 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1229,6 +1229,12 @@ int drm_crtc_init_with_planes(struct drm_device *dev,
>   			      struct drm_plane *cursor,
>   			      const struct drm_crtc_funcs *funcs,
>   			      const char *name, ...);
> +int drmm_crtc_init_with_planes(struct drm_device *dev,
> +			       struct drm_crtc *crtc,
> +			       struct drm_plane *primary,
> +			       struct drm_plane *cursor,
> +			       const struct drm_crtc_funcs *funcs,
> +			       const char *name, ...);
>   void drm_crtc_cleanup(struct drm_crtc *crtc);
>   
>   __printf(7, 8)

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220628/3b370e0b/attachment.sig>