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@xxxxxxx> --- 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.cindex 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
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
