Hi Am 16.09.22 um 13:22 schrieb Javier Martinez Canillas:
On 9/9/22 12:59, Thomas Zimmermann wrote:Provide drm_univeral_plane_alloc(), which allocated an initializes a plane. Code for non-atomic drivers uses this pattern. Convert it to the new function. The modeset helpers contain a quirk for handling their color formats differently. Set the flag outside plane allocation. The new function is already deprecated to some extend. Drivers should rather use drmm_univeral_plane_alloc() or drm_universal_plane_init(). Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> ---[...]+__printf(10, 11)+void *__drm_universal_plane_alloc(struct drm_device *dev, + size_t size, size_t offset, + uint32_t possible_crtcs, + const struct drm_plane_funcs *funcs, + const uint32_t *formats, + unsigned int format_count, + const uint64_t *format_modifiers, + enum drm_plane_type plane_type, + const char *name, ...); + +/** + * drm_universal_plane_alloc - Allocate and initialize an universal plane objectShould functions kernel-doc definitions use parenthesis or not? I see in https://elixir.bootlin.com/linux/latest/source/Documentation/doc-guide/kernel-doc.rst#L59 that the example has it but notice that there is not consistency in DRM about this.
A wasn't aware of this convention.
+ * @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.
Best regards Thomas
Other than those two nits, the patch looks good to me. Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
