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

[Thomas Zimmermann]

Hi

Am 20.06.22 um 15:48 schrieb Maxime Ripard:
> Hi,
> 
> On Mon, Jun 20, 2022 at 12:44:24PM +0200, Thomas Zimmermann wrote:
>> 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.
> 
> Why do you think it was a mistake?

The simple-encoder stuff is an interface that no one really needs. 
Compared to open-coding the function, it's barely an improvement in 
LOCs, but nothing else.

Back when I added drm_simple_encoder_init(), I wanted to simplify the 
many drivers that initialized the encoder with a cleanup callback and 
nothing else.  IIRC it was an improvement back then.  But now we already 
have a related drmm_ helper and a drmm_alloc_ helper. If I had only 
added the macro back then, we could use the regular encoder helpers.

> 
>> 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...
> 
> We can still have this. It would simplify this series so I could
> definitely squeeze that patch in and add a TODO item / deprecation
> notice for simple encoders if you think it's needed

Not necessary. It's not super important.

Best regards
Thomas

> 
> Maxime

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