[Nouveau] [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()

[Javier Martinez Canillas]

On 9/16/22 13:41, Thomas Zimmermann wrote:

[...]

>>
>>> + * @dev: DRM device
>>> + * @type: the type of the struct which contains struct &drm_plane
>>> + * @member: the name of the &drm_plane within @type
>>> + * @possible_crtcs: bitmask of possible CRTCs
>>> + * @funcs: callbacks for the new plane
>>> + * @formats: array of supported formats (DRM_FORMAT\_\*)
>>> + * @format_count: number of elements in @formats
>>> + * @format_modifiers: array of struct drm_format modifiers terminated by
>>> + *                    DRM_FORMAT_MOD_INVALID
>>> + * @plane_type: type of plane (overlay, primary, cursor)
>>> + * @name: printf style format string for the plane name, or NULL for default name
>>> + *
>>> + * Allocates and initializes a plane object of type @type. The caller
>>> + * is responsible for releasing the allocated memory with kfree().
>>> + *
>>
>> I thought that all the drmm_*_alloc() managed interfaces should use the
>> drmm_k{z,m}alloc() helpers, so that the memory is automatically freed
>> on the last drm_dev_put() call ?
> 
> For new drivers, managed cleanup is preferred. But this is an existing 
> unmanaged cleanup.
> 
> I don't know if drmm_ changes the semantics in unexpected ways (well, 
> probably not). With managed memory releases, I was worried that plane 
> memory might stay around longer than expected. And we'd have to fix the 
> plane-destroy function in each user as well.
> 
> Adding the existing code as a new function is the trivial solution and 
> allows to address each driver individually. Also see my reply to 
> Laurent's question on that topic.
>

Ah, never mind. I misread the function name definition and thought that
you had a drmm_ suffix but, now on second read I see that is only drm_
so just ignore my comment about using managed memory alloc / release.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat