Hi Daniel,
On Thu, 2020-07-23 at 00:22 +0200, daniel@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