[PATCH] drm/simple_kms_helper: add drmm_simple_encoder_init()

Philipp Zabel p.zabel at pengutronix.de
Thu Jul 23 14:46:37 UTC 2020


Hi Daniel,

On Thu, 2020-07-23 at 00:22 +0200, daniel at ffwll.ch wrote:
[...]
> Yeah the drmm_ versions of these need to check that the ->cleanup hook is
> NULL.
>
> Also there's not actually a double-free, since drm_foo_cleanup removes it
> from the lists, which means drm_mode_config_cleanup won't even see it. But
> if the driver has some additional code in ->cleanup that won't ever run,
> so probably still a bug.
>
> I also think that the drmm_foo_ wrappers should also do the allocation
> (and upcasting) kinda like drmm_dev_alloc(). Otherwise we're still stuck
> with tons of boilerplate.

Ok, I'll try this:

drmm_encoder_init() variant can verify that the passed
drm_encoder_funcs::destroy hook is NULL.

drmm_simple_encoder_init() can just provide empty drm_encoder_funcs
internally.

> For now I think it's ok if drivers that switch to drmm_ just copypaste,
> until we're sure this is the right thing to do. And then maybe also roll
> these out for all objects that stay for the entire lifetime of drm_device
> (plane, crtc, encoder, plus variants). Just to make sure we're consistent
> across all of them.

Thank you for clarifying, I wasn't sure this was the goal. I've started
with this function mostly because this is the most used one in imx-drm
and the only one where I didn't have to deal with va_args boilerplate.

regards
Philipp


More information about the dri-devel mailing list