On Tue, Jun 14, 2022 at 10:29:20AM +0200, Thomas Zimmermann wrote:
Hi
Am 14.06.22 um 09:37 schrieb Maxime Ripard:
Hi Thomas,
On Mon, Jun 13, 2022 at 01:23:54PM +0200, Thomas Zimmermann wrote:
Am 10.06.22 um 11:28 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.
Before I review all of thes patches, one question. it's yet not clear to me why drm_crtc_init_with_planes() wouldn't work. (I know we discussed this on IRC.)
If you're calling drmm_mode_config_init(), it will clean up all the CRTC, encoder connector, etc. data structures for you. For CRTC instances in kmalloced memory, wouldn't it be simpler to put the corresponding kfree into vc4_crtc_destroy()?
My intent was to remove as much of the lifetime handling and memory-management from drivers as possible.
My feeling is that while the solution you suggest would definitely work, it relies on drivers authors to know about this, and make the effort to convert the drivers themselves. And then we would have to review that, which we will probably miss (collectively), because it's a bit obscure.
While with either the drmm_alloc_* functions, or the new functions I introduce there, we can just deprecate the old ones, create a TODO entry and a coccinelle script and trust that it works properly.
Thanks for explaining the motivation.
I would not want to deprecate any of the existing functions, as they work for many drivers. The drmm_ functions add additional overhead that not everyone is willing to pay.
I'm a bit confused. drm_crtc_init_with_planes() at the moment has:
* 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.
Just like drm_encoder_init(), drm_simple_encoder_init() and drm_universal_plane_init(), so all the functions we have a drmm_* helper for.
And we have a TODO-list item that heavily hints at using them: https://dri.freedesktop.org/docs/drm/gpu/todo.html#object-lifetime-fixes
So it looks like we're already well on the deprecation path?
Maxime