| Message ID | 20220909105947.6487-3-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm/plane: Remove drm_plane_init(), plus other cleanups | expand |
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 ? > Signed-off-by: Thomas Zimmermann <tzimmermann@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 >
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@suse.de> > --- [...] > > +__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 Should 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. > + * @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 ? Other than those two nits, the patch looks good to me. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
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@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 >> >
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@suse.de> >> --- > > [...] > >> >> +__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 > > Should 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@redhat.com> >
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.
Hi Thomas, On Fri, Sep 16, 2022 at 01:31:25PM +0200, Thomas Zimmermann wrote: > Am 16.09.22 um 13:06 schrieb Laurent Pinchart: > > 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(). OK, works for me. > >> Signed-off-by: Thomas Zimmermann <tzimmermann@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
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
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@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(-)