[PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()
Thomas Zimmermann
tzimmermann at suse.de
Fri Sep 16 11:31:25 UTC 2022
Hi
Am 16.09.22 um 13:06 schrieb Laurent Pinchart:
> Hi Thomas,
>
> Thank you for the patch.
>
> On Fri, Sep 09, 2022 at 12:59:45PM +0200, 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().
>
> If this is already deprecated and used by a single driver, what is the
> point ?
It's used by nouveau and drm_modeset_helper.c. Since the code is
duplicated, it seems generally better to have it located and documented
in a central place.
Although it may look somewhat pointless now, the helper will get useful
in the future. The affected code in drm_modeset_helper is in
drm_crtc_init(), which is also a deprecated interface; only used by
non-atomic drivers. The function is a good candidate to be inlined into
calling drivers. Getting drm_crtc_init() removed will allow us to
correct these drivers' color-format handling. Once that happened,
several more drivers will call drm_univeral_plane_alloc().
Best regards
Thomas
>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>> drivers/gpu/drm/drm_modeset_helper.c | 61 +++++++++++--------------
>> drivers/gpu/drm/drm_plane.c | 38 +++++++++++++++
>> drivers/gpu/drm/nouveau/dispnv04/crtc.c | 41 ++++++-----------
>> include/drm/drm_plane.h | 44 ++++++++++++++++++
>> 4 files changed, 121 insertions(+), 63 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
>> index 611dd01fb604..38040eebfa16 100644
>> --- a/drivers/gpu/drm/drm_modeset_helper.c
>> +++ b/drivers/gpu/drm/drm_modeset_helper.c
>> @@ -113,38 +113,6 @@ static const struct drm_plane_funcs primary_plane_funcs = {
>> .destroy = drm_plane_helper_destroy,
>> };
>>
>> -static struct drm_plane *create_primary_plane(struct drm_device *dev)
>> -{
>> - struct drm_plane *primary;
>> - int ret;
>> -
>> - primary = kzalloc(sizeof(*primary), GFP_KERNEL);
>> - if (primary == NULL) {
>> - DRM_DEBUG_KMS("Failed to allocate primary plane\n");
>> - return NULL;
>> - }
>> -
>> - /*
>> - * Remove the format_default field from drm_plane when dropping
>> - * this helper.
>> - */
>> - primary->format_default = true;
>> -
>> - /* possible_crtc's will be filled in later by crtc_init */
>> - ret = drm_universal_plane_init(dev, primary, 0,
>> - &primary_plane_funcs,
>> - safe_modeset_formats,
>> - ARRAY_SIZE(safe_modeset_formats),
>> - NULL,
>> - DRM_PLANE_TYPE_PRIMARY, NULL);
>> - if (ret) {
>> - kfree(primary);
>> - primary = NULL;
>> - }
>> -
>> - return primary;
>> -}
>> -
>> /**
>> * drm_crtc_init - Legacy CRTC initialization function
>> * @dev: DRM device
>> @@ -176,10 +144,33 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>> const struct drm_crtc_funcs *funcs)
>> {
>> struct drm_plane *primary;
>> + int ret;
>> +
>> + /* possible_crtc's will be filled in later by crtc_init */
>> + primary = __drm_universal_plane_alloc(dev, sizeof(*primary), 0, 0,
>> + &primary_plane_funcs,
>> + safe_modeset_formats,
>> + ARRAY_SIZE(safe_modeset_formats),
>> + NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
>> + if (IS_ERR(primary))
>> + return PTR_ERR(primary);
>>
>> - primary = create_primary_plane(dev);
>> - return drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs,
>> - NULL);
>> + /*
>> + * Remove the format_default field from drm_plane when dropping
>> + * this helper.
>> + */
>> + primary->format_default = true;
>> +
>> + ret = drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs, NULL);
>> + if (ret)
>> + goto err_drm_plane_cleanup;
>> +
>> + return 0;
>> +
>> +err_drm_plane_cleanup:
>> + drm_plane_cleanup(primary);
>> + kfree(primary);
>> + return ret;
>> }
>> EXPORT_SYMBOL(drm_crtc_init);
>>
>> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>> index 0f14b4d3bb10..33357629a7f5 100644
>> --- a/drivers/gpu/drm/drm_plane.c
>> +++ b/drivers/gpu/drm/drm_plane.c
>> @@ -448,6 +448,44 @@ void *__drmm_universal_plane_alloc(struct drm_device *dev, size_t size,
>> }
>> EXPORT_SYMBOL(__drmm_universal_plane_alloc);
>>
>> +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 type,
>> + const char *name, ...)
>> +{
>> + void *container;
>> + struct drm_plane *plane;
>> + va_list ap;
>> + int ret;
>> +
>> + if (drm_WARN_ON(dev, !funcs))
>> + return ERR_PTR(-EINVAL);
>> +
>> + container = kzalloc(size, GFP_KERNEL);
>> + if (!container)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + plane = container + offset;
>> +
>> + va_start(ap, name);
>> + ret = __drm_universal_plane_init(dev, plane, possible_crtcs, funcs,
>> + formats, format_count, format_modifiers,
>> + type, name, ap);
>> + va_end(ap);
>> + if (ret)
>> + goto err_kfree;
>> +
>> + return container;
>> +
>> +err_kfree:
>> + kfree(container);
>> + return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL(__drm_universal_plane_alloc);
>> +
>> int drm_plane_register_all(struct drm_device *dev)
>> {
>> unsigned int num_planes = 0;
>> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
>> index 660c4cbc0b3d..6b8a014b5e97 100644
>> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
>> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
>> @@ -1281,32 +1281,6 @@ static const struct drm_plane_funcs nv04_primary_plane_funcs = {
>> .destroy = drm_plane_helper_destroy,
>> };
>>
>> -static struct drm_plane *
>> -create_primary_plane(struct drm_device *dev)
>> -{
>> - struct drm_plane *primary;
>> - int ret;
>> -
>> - primary = kzalloc(sizeof(*primary), GFP_KERNEL);
>> - if (primary == NULL) {
>> - DRM_DEBUG_KMS("Failed to allocate primary plane\n");
>> - return NULL;
>> - }
>> -
>> - /* possible_crtc's will be filled in later by crtc_init */
>> - ret = drm_universal_plane_init(dev, primary, 0,
>> - &nv04_primary_plane_funcs,
>> - modeset_formats,
>> - ARRAY_SIZE(modeset_formats), NULL,
>> - DRM_PLANE_TYPE_PRIMARY, NULL);
>> - if (ret) {
>> - kfree(primary);
>> - primary = NULL;
>> - }
>> -
>> - return primary;
>> -}
>> -
>> static int nv04_crtc_vblank_handler(struct nvif_notify *notify)
>> {
>> struct nouveau_crtc *nv_crtc =
>> @@ -1321,6 +1295,7 @@ nv04_crtc_create(struct drm_device *dev, int crtc_num)
>> {
>> struct nouveau_display *disp = nouveau_display(dev);
>> struct nouveau_crtc *nv_crtc;
>> + struct drm_plane *primary;
>> int ret;
>>
>> nv_crtc = kzalloc(sizeof(*nv_crtc), GFP_KERNEL);
>> @@ -1335,8 +1310,18 @@ nv04_crtc_create(struct drm_device *dev, int crtc_num)
>> nv_crtc->save = nv_crtc_save;
>> nv_crtc->restore = nv_crtc_restore;
>>
>> - drm_crtc_init_with_planes(dev, &nv_crtc->base,
>> - create_primary_plane(dev), NULL,
>> + primary = __drm_universal_plane_alloc(dev, sizeof(*primary), 0, 0,
>> + &nv04_primary_plane_funcs,
>> + modeset_formats,
>> + ARRAY_SIZE(modeset_formats), NULL,
>> + DRM_PLANE_TYPE_PRIMARY, NULL);
>> + if (IS_ERR(primary)) {
>> + ret = PTR_ERR(primary);
>> + kfree(nv_crtc);
>> + return ret;
>> + }
>> +
>> + drm_crtc_init_with_planes(dev, &nv_crtc->base, primary, NULL,
>> &nv04_crtc_funcs, NULL);
>> drm_crtc_helper_add(&nv_crtc->base, &nv04_crtc_helper_funcs);
>> drm_mode_crtc_set_gamma_size(&nv_crtc->base, 256);
>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>> index 910cb941f3d5..21dfa7f97948 100644
>> --- a/include/drm/drm_plane.h
>> +++ b/include/drm/drm_plane.h
>> @@ -809,6 +809,50 @@ void *__drmm_universal_plane_alloc(struct drm_device *dev,
>> format_count, format_modifiers, \
>> plane_type, name, ##__VA_ARGS__))
>>
>> +__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 object
>> + * @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().
>> + *
>> + * Drivers are encouraged to use drmm_universal_plane_alloc() instead.
>> + *
>> + * Drivers that only support the DRM_FORMAT_MOD_LINEAR modifier support may set
>> + * @format_modifiers to NULL. The plane will advertise the linear modifier.
>> + *
>> + * Returns:
>> + * Pointer to new plane, or ERR_PTR on failure.
>> + */
>> +#define drm_universal_plane_alloc(dev, type, member, possible_crtcs, funcs, formats, \
>> + format_count, format_modifiers, plane_type, name, ...) \
>> + ((type *)__drm_universal_plane_alloc(dev, sizeof(type), \
>> + offsetof(type, member), \
>> + possible_crtcs, funcs, formats, \
>> + format_count, format_modifiers, \
>> + plane_type, name, ##__VA_ARGS__))
>> +
>> /**
>> * drm_plane_index - find the index of a registered plane
>> * @plane: plane to find index for
>> --
>> 2.37.2
>>
>
--
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220916/61f5bb53/attachment.sig>
More information about the dri-devel
mailing list