[PATCH 09/64] drm/simple: Introduce drmm_simple_encoder_init

[Thomas Zimmermann]

Hi

Am 10.06.22 um 11:28 schrieb Maxime Ripard:
> The DRM-managed function to register an encoder is
> drmm_simple_encoder_alloc() and its variants, which will allocate the
> underlying structure and initialisation the encoder.
> 
> However, we might want to separate the structure creation and the encoder
> 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 an encoder that would be passed
> as an argument.
> 

There's nothing wrong with this patch, but I have to admit that adding 
drm_simple_encoder_init() et al was a mistake. It would have been better 
to add an initializer macro like

#define DRM_STATIC_ENCODER_DEFAULT_FUNCS \
   .destroy = drm_encoder_cleanup

It's way more flexible and similarly easy to use. Anyway...

> Signed-off-by: Maxime Ripard <maxime at cerno.tech>

Acked-by: Thomas Zimmermann <tzimmermann at suse.de>

Best regards
Thomas

> ---
>   drivers/gpu/drm/drm_simple_kms_helper.c | 46 +++++++++++++++++++++++--
>   include/drm/drm_simple_kms_helper.h     |  3 ++
>   2 files changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index 72989ed1baba..876870dd98e5 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -58,9 +58,10 @@ static const struct drm_encoder_funcs drm_simple_encoder_funcs_cleanup = {
>    * stored in the device structure. Free the encoder's memory as part of
>    * the device release function.
>    *
> - * Note: consider using drmm_simple_encoder_alloc() instead of
> - * drm_simple_encoder_init() to let the DRM managed resource infrastructure
> - * take care of cleanup and deallocation.
> + * Note: consider using drmm_simple_encoder_alloc() or
> + * drmm_simple_encoder_init() instead of drm_simple_encoder_init() to
> + * let the DRM managed resource infrastructure take care of cleanup and
> + * deallocation.
>    *
>    * Returns:
>    * Zero on success, error code on failure.
> @@ -75,6 +76,45 @@ int drm_simple_encoder_init(struct drm_device *dev,
>   }
>   EXPORT_SYMBOL(drm_simple_encoder_init);
>   
> +static void drmm_simple_encoder_cleanup(struct drm_device *dev, void *ptr)
> +{
> +	struct drm_encoder *encoder = ptr;
> +
> +	drm_encoder_cleanup(encoder);
> +}
> +
> +/**
> + * drmm_simple_encoder_init - Initialize a preallocated encoder with
> + *                            basic functionality.
> + * @dev: drm device
> + * @encoder: the encoder to initialize
> + * @encoder_type: user visible type of the encoder
> + *
> + * Initialises a preallocated encoder that has no further functionality.
> + * Settings for possible CRTC and clones are left to their initial
> + * values. The encoder will be cleaned up automatically using a
> + * DRM-managed action.
> + *
> + * The structure containing the encoder's memory should be allocated
> + * using drmm_kzalloc().
> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drmm_simple_encoder_init(struct drm_device *dev,
> +			     struct drm_encoder *encoder,
> +			     int encoder_type)
> +{
> +	int ret;
> +
> +	ret = drm_encoder_init(dev, encoder, NULL, encoder_type, NULL);
> +	if (ret)
> +		return ret;
> +
> +	return drmm_add_action_or_reset(dev, drmm_simple_encoder_cleanup, encoder);
> +}
> +EXPORT_SYMBOL(drmm_simple_encoder_init);
> +
>   void *__drmm_simple_encoder_alloc(struct drm_device *dev, size_t size,
>   				  size_t offset, int encoder_type)
>   {
> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> index 0b3647e614dd..20456f4712f0 100644
> --- a/include/drm/drm_simple_kms_helper.h
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -241,6 +241,9 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
>   int drm_simple_encoder_init(struct drm_device *dev,
>   			    struct drm_encoder *encoder,
>   			    int encoder_type);
> +int drmm_simple_encoder_init(struct drm_device *dev,
> +			     struct drm_encoder *encoder,
> +			     int encoder_type);
>   
>   void *__drmm_simple_encoder_alloc(struct drm_device *dev, size_t size,
>   				  size_t offset, int encoder_type);

-- 
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/20220620/a6cc7dec/attachment.sig>