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
Attachment:
signature.asc
Description: PGP signature
