* [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups
@ 2022-09-09 10:59 Thomas Zimmermann
2022-09-09 10:59 ` [PATCH 1/4] drm/plane: Remove drm_plane_init() Thomas Zimmermann
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-09-09 10:59 UTC (permalink / raw)
To: maarten.lankhorst, mripard, airlied, daniel, bskeggs, kherbst,
lyude, laurent.pinchart, kieran.bingham+renesas, jyri.sarha,
tomba, sam
Cc: dri-devel, nouveau, linux-renesas-soc, Thomas Zimmermann
This patchset does cleanups to the plane code, most noteably it removes
drm_plane_init(). The function is a small wrapper, which can easily be
inlined into the few callers. Patch #1 fixes this.
The other clean-up patches #2 to #4 affect plane creation. Modesetting
helpers and nouveau share some plane-allocation code that can be shared as
helper function. While the function is already outdated, it's now at least
well documented. As suggested by Daniel, patch #3 adds a warning to
non-atomic plane helpers when they are being called from atomic drivers.
Patch #4 adds an initializer macro for non-atomic plane functions. It
should not be used in new drivers, but at least documents the current
practice.
Tested with nouveau on Nvidia G72 hardware.
A possible next step would be the inlining of drm_crtc_init() and the
removal of drm_plane.format_default.
Thomas Zimmermann (4):
drm/plane: Remove drm_plane_init()
drm/plane: Allocate planes with drm_universal_plane_alloc()
drm/plane-helper: Warn if atomic drivers call non-atomic helpers
drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro
drivers/gpu/drm/drm_modeset_helper.c | 68 +++++++++------------
drivers/gpu/drm/drm_plane.c | 70 ++++++++++++----------
drivers/gpu/drm/drm_plane_helper.c | 10 ++++
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 45 +++++---------
drivers/gpu/drm/nouveau/dispnv04/overlay.c | 13 ++--
drivers/gpu/drm/shmobile/shmob_drm_plane.c | 7 ++-
drivers/gpu/drm/tilcdc/tilcdc_plane.c | 9 ++-
include/drm/drm_plane.h | 52 +++++++++++++---
include/drm/drm_plane_helper.h | 12 ++++
9 files changed, 162 insertions(+), 124 deletions(-)
base-commit: f2c3a05d33693ad51996fa7d12d3b2d4b0f372eb
prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
prerequisite-patch-id: 3f204510fcbf9530d6540bd8e6128cce598988b6
--
2.37.2
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/4] drm/plane: Remove drm_plane_init()
2022-09-09 10:59 [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups Thomas Zimmermann
@ 2022-09-09 10:59 ` Thomas Zimmermann
2022-09-09 18:24 ` Lyude Paul
` (2 more replies)
2022-09-09 10:59 ` [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc() Thomas Zimmermann
` (4 subsequent siblings)
5 siblings, 3 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-09-09 10:59 UTC (permalink / raw)
To: maarten.lankhorst, mripard, airlied, daniel, bskeggs, kherbst,
lyude, laurent.pinchart, kieran.bingham+renesas, jyri.sarha,
tomba, sam
Cc: dri-devel, nouveau, linux-renesas-soc, Thomas Zimmermann
Open-code drm_plane_init() and remove the function from DRM. The
implementation of drm_plane_init() is a simple wrapper around a call
to drm_universal_plane_init(), so drivers can just use that instead.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_modeset_helper.c | 3 +-
drivers/gpu/drm/drm_plane.c | 32 ----------------------
drivers/gpu/drm/nouveau/dispnv04/overlay.c | 13 +++++----
drivers/gpu/drm/shmobile/shmob_drm_plane.c | 7 +++--
drivers/gpu/drm/tilcdc/tilcdc_plane.c | 9 +++---
include/drm/drm_plane.h | 8 +-----
6 files changed, 17 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
index bd609a978848..611dd01fb604 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -100,8 +100,7 @@ EXPORT_SYMBOL(drm_helper_mode_fill_fb_struct);
* This is the minimal list of formats that seem to be safe for modeset use
* with all current DRM drivers. Most hardware can actually support more
* formats than this and drivers may specify a more accurate list when
- * creating the primary plane. However drivers that still call
- * drm_plane_init() will use this minimal format list as the default.
+ * creating the primary plane.
*/
static const uint32_t safe_modeset_formats[] = {
DRM_FORMAT_XRGB8888,
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 726f2f163c26..0f14b4d3bb10 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -482,38 +482,6 @@ void drm_plane_unregister_all(struct drm_device *dev)
}
}
-/**
- * drm_plane_init - Initialize a legacy plane
- * @dev: DRM device
- * @plane: plane object to init
- * @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
- * @is_primary: plane type (primary vs overlay)
- *
- * Legacy API to initialize a DRM plane.
- *
- * New drivers should call drm_universal_plane_init() instead.
- *
- * Returns:
- * Zero on success, error code on failure.
- */
-int drm_plane_init(struct drm_device *dev, struct drm_plane *plane,
- uint32_t possible_crtcs,
- const struct drm_plane_funcs *funcs,
- const uint32_t *formats, unsigned int format_count,
- bool is_primary)
-{
- enum drm_plane_type type;
-
- type = is_primary ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
- return drm_universal_plane_init(dev, plane, possible_crtcs, funcs,
- formats, format_count,
- NULL, type, NULL);
-}
-EXPORT_SYMBOL(drm_plane_init);
-
/**
* drm_plane_cleanup - Clean up the core plane usage
* @plane: plane to cleanup
diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
index 37e63e98cd08..33f29736024a 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
@@ -296,9 +296,10 @@ nv10_overlay_init(struct drm_device *device)
break;
}
- ret = drm_plane_init(device, &plane->base, 3 /* both crtc's */,
- &nv10_plane_funcs,
- formats, num_formats, false);
+ ret = drm_universal_plane_init(device, &plane->base, 3 /* both crtc's */,
+ &nv10_plane_funcs,
+ formats, num_formats, NULL,
+ DRM_PLANE_TYPE_OVERLAY, NULL);
if (ret)
goto err;
@@ -475,9 +476,9 @@ nv04_overlay_init(struct drm_device *device)
if (!plane)
return;
- ret = drm_plane_init(device, &plane->base, 1 /* single crtc */,
- &nv04_plane_funcs,
- formats, 2, false);
+ ret = drm_universal_plane_init(device, &plane->base, 1 /* single crtc */,
+ &nv04_plane_funcs, formats, 2, NULL,
+ DRM_PLANE_TYPE_OVERLAY, NULL);
if (ret)
goto err;
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
index 54228424793a..6c5f0cbe7d95 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_plane.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
@@ -252,9 +252,10 @@ int shmob_drm_plane_create(struct shmob_drm_device *sdev, unsigned int index)
splane->index = index;
splane->alpha = 255;
- ret = drm_plane_init(sdev->ddev, &splane->plane, 1,
- &shmob_drm_plane_funcs, formats,
- ARRAY_SIZE(formats), false);
+ ret = drm_universal_plane_init(sdev->ddev, &splane->plane, 1,
+ &shmob_drm_plane_funcs,
+ formats, ARRAY_SIZE(formats), NULL,
+ DRM_PLANE_TYPE_OVERLAY, NULL);
return ret;
}
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_plane.c b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
index 0ccf791301cb..cf77a8ce7398 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_plane.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
@@ -105,11 +105,10 @@ int tilcdc_plane_init(struct drm_device *dev,
struct tilcdc_drm_private *priv = dev->dev_private;
int ret;
- ret = drm_plane_init(dev, plane, 1,
- &tilcdc_plane_funcs,
- priv->pixelformats,
- priv->num_pixelformats,
- true);
+ ret = drm_universal_plane_init(dev, plane, 1, &tilcdc_plane_funcs,
+ priv->pixelformats,
+ priv->num_pixelformats,
+ NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
if (ret) {
dev_err(dev->dev, "Failed to initialize plane: %d\n", ret);
return ret;
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 89ea54652e87..910cb941f3d5 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -631,7 +631,7 @@ struct drm_plane {
unsigned int format_count;
/**
* @format_default: driver hasn't supplied supported formats for the
- * plane. Used by the drm_plane_init compatibility wrapper only.
+ * plane. Used by the non-atomic driver compatibility wrapper only.
*/
bool format_default;
@@ -762,12 +762,6 @@ int drm_universal_plane_init(struct drm_device *dev,
const uint64_t *format_modifiers,
enum drm_plane_type type,
const char *name, ...);
-int drm_plane_init(struct drm_device *dev,
- struct drm_plane *plane,
- uint32_t possible_crtcs,
- const struct drm_plane_funcs *funcs,
- const uint32_t *formats, unsigned int format_count,
- bool is_primary);
void drm_plane_cleanup(struct drm_plane *plane);
__printf(10, 11)
--
2.37.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()
2022-09-09 10:59 [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups Thomas Zimmermann
2022-09-09 10:59 ` [PATCH 1/4] drm/plane: Remove drm_plane_init() Thomas Zimmermann
@ 2022-09-09 10:59 ` Thomas Zimmermann
2022-09-16 11:06 ` Laurent Pinchart
2022-09-16 11:22 ` Javier Martinez Canillas
2022-09-09 10:59 ` [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers Thomas Zimmermann
` (3 subsequent siblings)
5 siblings, 2 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-09-09 10:59 UTC (permalink / raw)
To: maarten.lankhorst, mripard, airlied, daniel, bskeggs, kherbst,
lyude, laurent.pinchart, kieran.bingham+renesas, jyri.sarha,
tomba, sam
Cc: dri-devel, nouveau, linux-renesas-soc, Thomas Zimmermann
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(-)
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers
2022-09-09 10:59 [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups Thomas Zimmermann
2022-09-09 10:59 ` [PATCH 1/4] drm/plane: Remove drm_plane_init() Thomas Zimmermann
2022-09-09 10:59 ` [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc() Thomas Zimmermann
@ 2022-09-09 10:59 ` Thomas Zimmermann
2022-09-16 11:20 ` Laurent Pinchart
2022-09-16 11:23 ` Javier Martinez Canillas
2022-09-09 10:59 ` [PATCH 4/4] drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro Thomas Zimmermann
` (2 subsequent siblings)
5 siblings, 2 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-09-09 10:59 UTC (permalink / raw)
To: maarten.lankhorst, mripard, airlied, daniel, bskeggs, kherbst,
lyude, laurent.pinchart, kieran.bingham+renesas, jyri.sarha,
tomba, sam
Cc: dri-devel, nouveau, linux-renesas-soc, Thomas Zimmermann
The plane update and disable helpers are only useful for non-atomic
drivers. Print a warning if an atomic driver calls them.
Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_plane_helper.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index c7785967f5bf..1c904fc26a58 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -30,8 +30,10 @@
#include <drm/drm_atomic_uapi.h>
#include <drm/drm_crtc_helper.h>
#include <drm/drm_device.h>
+#include <drm/drm_drv.h>
#include <drm/drm_encoder.h>
#include <drm/drm_plane_helper.h>
+#include <drm/drm_print.h>
#include <drm/drm_rect.h>
#define SUBPIXEL_MASK 0xffff
@@ -195,10 +197,14 @@ int drm_plane_helper_update_primary(struct drm_plane *plane, struct drm_crtc *cr
.x2 = crtc_x + crtc_w,
.y2 = crtc_y + crtc_h,
};
+ struct drm_device *dev = plane->dev;
struct drm_connector **connector_list;
int num_connectors, ret;
bool visible;
+ if (drm_WARN_ON_ONCE(dev, drm_drv_uses_atomic_modeset(dev)))
+ return -EINVAL;
+
ret = drm_plane_helper_check_update(plane, crtc, fb,
&src, &dest,
DRM_MODE_ROTATE_0,
@@ -260,6 +266,10 @@ EXPORT_SYMBOL(drm_plane_helper_update_primary);
int drm_plane_helper_disable_primary(struct drm_plane *plane,
struct drm_modeset_acquire_ctx *ctx)
{
+ struct drm_device *dev = plane->dev;
+
+ drm_WARN_ON_ONCE(dev, drm_drv_uses_atomic_modeset(dev));
+
return -EINVAL;
}
EXPORT_SYMBOL(drm_plane_helper_disable_primary);
--
2.37.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/4] drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro
2022-09-09 10:59 [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups Thomas Zimmermann
` (2 preceding siblings ...)
2022-09-09 10:59 ` [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers Thomas Zimmermann
@ 2022-09-09 10:59 ` Thomas Zimmermann
2022-09-16 11:22 ` Laurent Pinchart
2022-09-16 11:24 ` Javier Martinez Canillas
2022-09-09 18:39 ` [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups Lyude Paul
2022-09-13 9:06 ` [PATCH 1/4] drm/plane: Remove drm_plane_init() jyri.sarha
5 siblings, 2 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-09-09 10:59 UTC (permalink / raw)
To: maarten.lankhorst, mripard, airlied, daniel, bskeggs, kherbst,
lyude, laurent.pinchart, kieran.bingham+renesas, jyri.sarha,
tomba, sam
Cc: dri-devel, nouveau, linux-renesas-soc, Thomas Zimmermann
Provide DRM_PLANE_NON_ATOMIC_FUNCS, which initializes plane functions
of non-atomic drivers to default values. The macro is not supposed to
be used in new code, but helps with documenting and finding existing
users.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_modeset_helper.c | 4 +---
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 +---
include/drm/drm_plane_helper.h | 12 ++++++++++++
3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
index 38040eebfa16..f858dfedf2cf 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -108,9 +108,7 @@ static const uint32_t safe_modeset_formats[] = {
};
static const struct drm_plane_funcs primary_plane_funcs = {
- .update_plane = drm_plane_helper_update_primary,
- .disable_plane = drm_plane_helper_disable_primary,
- .destroy = drm_plane_helper_destroy,
+ DRM_PLANE_NON_ATOMIC_FUNCS,
};
/**
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 6b8a014b5e97..ee92d576d277 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -1276,9 +1276,7 @@ static const uint32_t modeset_formats[] = {
};
static const struct drm_plane_funcs nv04_primary_plane_funcs = {
- .update_plane = drm_plane_helper_update_primary,
- .disable_plane = drm_plane_helper_disable_primary,
- .destroy = drm_plane_helper_destroy,
+ DRM_PLANE_NON_ATOMIC_FUNCS,
};
static int nv04_crtc_vblank_handler(struct nvif_notify *notify)
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 1781fab24dd6..75f9c4830564 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -42,4 +42,16 @@ int drm_plane_helper_disable_primary(struct drm_plane *plane,
struct drm_modeset_acquire_ctx *ctx);
void drm_plane_helper_destroy(struct drm_plane *plane);
+/**
+ * DRM_PLANE_NON_ATOMIC_FUNCS - Default plane functions for non-atomic drivers
+ *
+ * This macro initializes plane functions for non-atomic drivers to default
+ * values. Non-atomic interfaces are deprecated and should not be used in new
+ * drivers.
+ */
+#define DRM_PLANE_NON_ATOMIC_FUNCS \
+ .update_plane = drm_plane_helper_update_primary, \
+ .disable_plane = drm_plane_helper_disable_primary, \
+ .destroy = drm_plane_helper_destroy
+
#endif
--
2.37.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] drm/plane: Remove drm_plane_init()
2022-09-09 10:59 ` [PATCH 1/4] drm/plane: Remove drm_plane_init() Thomas Zimmermann
@ 2022-09-09 18:24 ` Lyude Paul
2022-09-16 11:00 ` Javier Martinez Canillas
2022-09-16 11:05 ` Laurent Pinchart
2 siblings, 0 replies; 20+ messages in thread
From: Lyude Paul @ 2022-09-09 18:24 UTC (permalink / raw)
To: Thomas Zimmermann, maarten.lankhorst, mripard, airlied, daniel,
bskeggs, kherbst, laurent.pinchart, kieran.bingham+renesas,
jyri.sarha, tomba, sam
Cc: dri-devel, nouveau, linux-renesas-soc
Reviewed-by: Lyude Paul <lyude@redhat.com>
for common and nouveau bits
On Fri, 2022-09-09 at 12:59 +0200, Thomas Zimmermann wrote:
> Open-code drm_plane_init() and remove the function from DRM. The
> implementation of drm_plane_init() is a simple wrapper around a call
> to drm_universal_plane_init(), so drivers can just use that instead.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/drm_modeset_helper.c | 3 +-
> drivers/gpu/drm/drm_plane.c | 32 ----------------------
> drivers/gpu/drm/nouveau/dispnv04/overlay.c | 13 +++++----
> drivers/gpu/drm/shmobile/shmob_drm_plane.c | 7 +++--
> drivers/gpu/drm/tilcdc/tilcdc_plane.c | 9 +++---
> include/drm/drm_plane.h | 8 +-----
> 6 files changed, 17 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index bd609a978848..611dd01fb604 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -100,8 +100,7 @@ EXPORT_SYMBOL(drm_helper_mode_fill_fb_struct);
> * This is the minimal list of formats that seem to be safe for modeset use
> * with all current DRM drivers. Most hardware can actually support more
> * formats than this and drivers may specify a more accurate list when
> - * creating the primary plane. However drivers that still call
> - * drm_plane_init() will use this minimal format list as the default.
> + * creating the primary plane.
> */
> static const uint32_t safe_modeset_formats[] = {
> DRM_FORMAT_XRGB8888,
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 726f2f163c26..0f14b4d3bb10 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -482,38 +482,6 @@ void drm_plane_unregister_all(struct drm_device *dev)
> }
> }
>
> -/**
> - * drm_plane_init - Initialize a legacy plane
> - * @dev: DRM device
> - * @plane: plane object to init
> - * @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
> - * @is_primary: plane type (primary vs overlay)
> - *
> - * Legacy API to initialize a DRM plane.
> - *
> - * New drivers should call drm_universal_plane_init() instead.
> - *
> - * Returns:
> - * Zero on success, error code on failure.
> - */
> -int drm_plane_init(struct drm_device *dev, struct drm_plane *plane,
> - uint32_t possible_crtcs,
> - const struct drm_plane_funcs *funcs,
> - const uint32_t *formats, unsigned int format_count,
> - bool is_primary)
> -{
> - enum drm_plane_type type;
> -
> - type = is_primary ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
> - return drm_universal_plane_init(dev, plane, possible_crtcs, funcs,
> - formats, format_count,
> - NULL, type, NULL);
> -}
> -EXPORT_SYMBOL(drm_plane_init);
> -
> /**
> * drm_plane_cleanup - Clean up the core plane usage
> * @plane: plane to cleanup
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> index 37e63e98cd08..33f29736024a 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> @@ -296,9 +296,10 @@ nv10_overlay_init(struct drm_device *device)
> break;
> }
>
> - ret = drm_plane_init(device, &plane->base, 3 /* both crtc's */,
> - &nv10_plane_funcs,
> - formats, num_formats, false);
> + ret = drm_universal_plane_init(device, &plane->base, 3 /* both crtc's */,
> + &nv10_plane_funcs,
> + formats, num_formats, NULL,
> + DRM_PLANE_TYPE_OVERLAY, NULL);
> if (ret)
> goto err;
>
> @@ -475,9 +476,9 @@ nv04_overlay_init(struct drm_device *device)
> if (!plane)
> return;
>
> - ret = drm_plane_init(device, &plane->base, 1 /* single crtc */,
> - &nv04_plane_funcs,
> - formats, 2, false);
> + ret = drm_universal_plane_init(device, &plane->base, 1 /* single crtc */,
> + &nv04_plane_funcs, formats, 2, NULL,
> + DRM_PLANE_TYPE_OVERLAY, NULL);
> if (ret)
> goto err;
>
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> index 54228424793a..6c5f0cbe7d95 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> @@ -252,9 +252,10 @@ int shmob_drm_plane_create(struct shmob_drm_device *sdev, unsigned int index)
> splane->index = index;
> splane->alpha = 255;
>
> - ret = drm_plane_init(sdev->ddev, &splane->plane, 1,
> - &shmob_drm_plane_funcs, formats,
> - ARRAY_SIZE(formats), false);
> + ret = drm_universal_plane_init(sdev->ddev, &splane->plane, 1,
> + &shmob_drm_plane_funcs,
> + formats, ARRAY_SIZE(formats), NULL,
> + DRM_PLANE_TYPE_OVERLAY, NULL);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_plane.c b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> index 0ccf791301cb..cf77a8ce7398 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> @@ -105,11 +105,10 @@ int tilcdc_plane_init(struct drm_device *dev,
> struct tilcdc_drm_private *priv = dev->dev_private;
> int ret;
>
> - ret = drm_plane_init(dev, plane, 1,
> - &tilcdc_plane_funcs,
> - priv->pixelformats,
> - priv->num_pixelformats,
> - true);
> + ret = drm_universal_plane_init(dev, plane, 1, &tilcdc_plane_funcs,
> + priv->pixelformats,
> + priv->num_pixelformats,
> + NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> dev_err(dev->dev, "Failed to initialize plane: %d\n", ret);
> return ret;
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 89ea54652e87..910cb941f3d5 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -631,7 +631,7 @@ struct drm_plane {
> unsigned int format_count;
> /**
> * @format_default: driver hasn't supplied supported formats for the
> - * plane. Used by the drm_plane_init compatibility wrapper only.
> + * plane. Used by the non-atomic driver compatibility wrapper only.
> */
> bool format_default;
>
> @@ -762,12 +762,6 @@ int drm_universal_plane_init(struct drm_device *dev,
> const uint64_t *format_modifiers,
> enum drm_plane_type type,
> const char *name, ...);
> -int drm_plane_init(struct drm_device *dev,
> - struct drm_plane *plane,
> - uint32_t possible_crtcs,
> - const struct drm_plane_funcs *funcs,
> - const uint32_t *formats, unsigned int format_count,
> - bool is_primary);
> void drm_plane_cleanup(struct drm_plane *plane);
>
> __printf(10, 11)
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups
2022-09-09 10:59 [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups Thomas Zimmermann
` (3 preceding siblings ...)
2022-09-09 10:59 ` [PATCH 4/4] drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro Thomas Zimmermann
@ 2022-09-09 18:39 ` Lyude Paul
2022-09-13 9:06 ` [PATCH 1/4] drm/plane: Remove drm_plane_init() jyri.sarha
5 siblings, 0 replies; 20+ messages in thread
From: Lyude Paul @ 2022-09-09 18:39 UTC (permalink / raw)
To: Thomas Zimmermann, maarten.lankhorst, mripard, airlied, daniel,
bskeggs, kherbst, laurent.pinchart, kieran.bingham+renesas,
jyri.sarha, tomba, sam
Cc: dri-devel, nouveau, linux-renesas-soc
For the nouveau bits on 1, 2 and 4:
Reviewed-by: Lyude Paul <lyude@redhat.com>
On Fri, 2022-09-09 at 12:59 +0200, Thomas Zimmermann wrote:
> This patchset does cleanups to the plane code, most noteably it removes
> drm_plane_init(). The function is a small wrapper, which can easily be
> inlined into the few callers. Patch #1 fixes this.
>
> The other clean-up patches #2 to #4 affect plane creation. Modesetting
> helpers and nouveau share some plane-allocation code that can be shared as
> helper function. While the function is already outdated, it's now at least
> well documented. As suggested by Daniel, patch #3 adds a warning to
> non-atomic plane helpers when they are being called from atomic drivers.
> Patch #4 adds an initializer macro for non-atomic plane functions. It
> should not be used in new drivers, but at least documents the current
> practice.
>
> Tested with nouveau on Nvidia G72 hardware.
>
> A possible next step would be the inlining of drm_crtc_init() and the
> removal of drm_plane.format_default.
>
> Thomas Zimmermann (4):
> drm/plane: Remove drm_plane_init()
> drm/plane: Allocate planes with drm_universal_plane_alloc()
> drm/plane-helper: Warn if atomic drivers call non-atomic helpers
> drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro
>
> drivers/gpu/drm/drm_modeset_helper.c | 68 +++++++++------------
> drivers/gpu/drm/drm_plane.c | 70 ++++++++++++----------
> drivers/gpu/drm/drm_plane_helper.c | 10 ++++
> drivers/gpu/drm/nouveau/dispnv04/crtc.c | 45 +++++---------
> drivers/gpu/drm/nouveau/dispnv04/overlay.c | 13 ++--
> drivers/gpu/drm/shmobile/shmob_drm_plane.c | 7 ++-
> drivers/gpu/drm/tilcdc/tilcdc_plane.c | 9 ++-
> include/drm/drm_plane.h | 52 +++++++++++++---
> include/drm/drm_plane_helper.h | 12 ++++
> 9 files changed, 162 insertions(+), 124 deletions(-)
>
>
> base-commit: f2c3a05d33693ad51996fa7d12d3b2d4b0f372eb
> prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
> prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
> prerequisite-patch-id: 3f204510fcbf9530d6540bd8e6128cce598988b6
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] drm/plane: Remove drm_plane_init()
2022-09-09 10:59 [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups Thomas Zimmermann
` (4 preceding siblings ...)
2022-09-09 18:39 ` [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups Lyude Paul
@ 2022-09-13 9:06 ` jyri.sarha
5 siblings, 0 replies; 20+ messages in thread
From: jyri.sarha @ 2022-09-13 9:06 UTC (permalink / raw)
To: Thomas Zimmermann, maarten.lankhorst, mripard, airlied, daniel,
bskeggs, kherbst, lyude, laurent.pinchart,
kieran.bingham+renesas, jyri.sarha, tomba, sam
Cc: dri-devel, nouveau, linux-renesas-soc
September 9, 2022 at 1:59 PM, "Thomas Zimmermann" <tzimmermann@suse.de mailto:tzimmermann@suse.de?to=%22Thomas%20Zimmermann%22%20%3Ctzimmermann%40suse.de%3E > wrote:
>
> Open-code drm_plane_init() and remove the function from DRM. The
> implementation of drm_plane_init() is a simple wrapper around a call
> to drm_universal_plane_init(), so drivers can just use that instead.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Jyri Sarha <jyri.sarha@iki.fi>
> ---
> drivers/gpu/drm/drm_modeset_helper.c | 3 +-
> drivers/gpu/drm/drm_plane.c | 32 ----------------------
> drivers/gpu/drm/nouveau/dispnv04/overlay.c | 13 +++++----
> drivers/gpu/drm/shmobile/shmob_drm_plane.c | 7 +++--
> drivers/gpu/drm/tilcdc/tilcdc_plane.c | 9 +++---
> include/drm/drm_plane.h | 8 +-----
> 6 files changed, 17 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index bd609a978848..611dd01fb604 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -100,8 +100,7 @@ EXPORT_SYMBOL(drm_helper_mode_fill_fb_struct);
> * This is the minimal list of formats that seem to be safe for modeset use
> * with all current DRM drivers. Most hardware can actually support more
> * formats than this and drivers may specify a more accurate list when
> - * creating the primary plane. However drivers that still call
> - * drm_plane_init() will use this minimal format list as the default.
> + * creating the primary plane.
> */
> static const uint32_t safe_modeset_formats[] = {
> DRM_FORMAT_XRGB8888,
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 726f2f163c26..0f14b4d3bb10 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -482,38 +482,6 @@ void drm_plane_unregister_all(struct drm_device *dev)
> }
> }
>
> -/**
> - * drm_plane_init - Initialize a legacy plane
> - * @dev: DRM device
> - * @plane: plane object to init
> - * @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
> - * @is_primary: plane type (primary vs overlay)
> - *
> - * Legacy API to initialize a DRM plane.
> - *
> - * New drivers should call drm_universal_plane_init() instead.
> - *
> - * Returns:
> - * Zero on success, error code on failure.
> - */
> -int drm_plane_init(struct drm_device *dev, struct drm_plane *plane,
> - uint32_t possible_crtcs,
> - const struct drm_plane_funcs *funcs,
> - const uint32_t *formats, unsigned int format_count,
> - bool is_primary)
> -{
> - enum drm_plane_type type;
> -
> - type = is_primary ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
> - return drm_universal_plane_init(dev, plane, possible_crtcs, funcs,
> - formats, format_count,
> - NULL, type, NULL);
> -}
> -EXPORT_SYMBOL(drm_plane_init);
> -
> /**
> * drm_plane_cleanup - Clean up the core plane usage
> * @plane: plane to cleanup
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> index 37e63e98cd08..33f29736024a 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> @@ -296,9 +296,10 @@ nv10_overlay_init(struct drm_device *device)
> break;
> }
>
> - ret = drm_plane_init(device, &plane->base, 3 /* both crtc's */,
> - &nv10_plane_funcs,
> - formats, num_formats, false);
> + ret = drm_universal_plane_init(device, &plane->base, 3 /* both crtc's */,
> + &nv10_plane_funcs,
> + formats, num_formats, NULL,
> + DRM_PLANE_TYPE_OVERLAY, NULL);
> if (ret)
> goto err;
>
> @@ -475,9 +476,9 @@ nv04_overlay_init(struct drm_device *device)
> if (!plane)
> return;
>
> - ret = drm_plane_init(device, &plane->base, 1 /* single crtc */,
> - &nv04_plane_funcs,
> - formats, 2, false);
> + ret = drm_universal_plane_init(device, &plane->base, 1 /* single crtc */,
> + &nv04_plane_funcs, formats, 2, NULL,
> + DRM_PLANE_TYPE_OVERLAY, NULL);
> if (ret)
> goto err;
>
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> index 54228424793a..6c5f0cbe7d95 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> @@ -252,9 +252,10 @@ int shmob_drm_plane_create(struct shmob_drm_device *sdev, unsigned int index)
> splane->index = index;
> splane->alpha = 255;
>
> - ret = drm_plane_init(sdev->ddev, &splane->plane, 1,
> - &shmob_drm_plane_funcs, formats,
> - ARRAY_SIZE(formats), false);
> + ret = drm_universal_plane_init(sdev->ddev, &splane->plane, 1,
> + &shmob_drm_plane_funcs,
> + formats, ARRAY_SIZE(formats), NULL,
> + DRM_PLANE_TYPE_OVERLAY, NULL);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_plane.c b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> index 0ccf791301cb..cf77a8ce7398 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> @@ -105,11 +105,10 @@ int tilcdc_plane_init(struct drm_device *dev,
> struct tilcdc_drm_private *priv = dev->dev_private;
> int ret;
>
> - ret = drm_plane_init(dev, plane, 1,
> - &tilcdc_plane_funcs,
> - priv->pixelformats,
> - priv->num_pixelformats,
> - true);
> + ret = drm_universal_plane_init(dev, plane, 1, &tilcdc_plane_funcs,
> + priv->pixelformats,
> + priv->num_pixelformats,
> + NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> dev_err(dev->dev, "Failed to initialize plane: %d\n", ret);
> return ret;
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 89ea54652e87..910cb941f3d5 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -631,7 +631,7 @@ struct drm_plane {
> unsigned int format_count;
> /**
> * @format_default: driver hasn't supplied supported formats for the
> - * plane. Used by the drm_plane_init compatibility wrapper only.
> + * plane. Used by the non-atomic driver compatibility wrapper only.
> */
> bool format_default;
>
> @@ -762,12 +762,6 @@ int drm_universal_plane_init(struct drm_device *dev,
> const uint64_t *format_modifiers,
> enum drm_plane_type type,
> const char *name, ...);
> -int drm_plane_init(struct drm_device *dev,
> - struct drm_plane *plane,
> - uint32_t possible_crtcs,
> - const struct drm_plane_funcs *funcs,
> - const uint32_t *formats, unsigned int format_count,
> - bool is_primary);
> void drm_plane_cleanup(struct drm_plane *plane);
>
> __printf(10, 11)
> --
> 2.37.2
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] drm/plane: Remove drm_plane_init()
2022-09-09 10:59 ` [PATCH 1/4] drm/plane: Remove drm_plane_init() Thomas Zimmermann
2022-09-09 18:24 ` Lyude Paul
@ 2022-09-16 11:00 ` Javier Martinez Canillas
2022-09-16 11:05 ` Laurent Pinchart
2 siblings, 0 replies; 20+ messages in thread
From: Javier Martinez Canillas @ 2022-09-16 11:00 UTC (permalink / raw)
To: Thomas Zimmermann, maarten.lankhorst, mripard, airlied, daniel,
bskeggs, kherbst, lyude, laurent.pinchart,
kieran.bingham+renesas, jyri.sarha, tomba, sam
Cc: linux-renesas-soc, nouveau, dri-devel
Hello Thomas,
On 9/9/22 12:59, Thomas Zimmermann wrote:
> Open-code drm_plane_init() and remove the function from DRM. The
> implementation of drm_plane_init() is a simple wrapper around a call
> to drm_universal_plane_init(), so drivers can just use that instead.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
[...]
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> index 37e63e98cd08..33f29736024a 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> @@ -296,9 +296,10 @@ nv10_overlay_init(struct drm_device *device)
> break;
> }
>
> - ret = drm_plane_init(device, &plane->base, 3 /* both crtc's */,
> - &nv10_plane_funcs,
> - formats, num_formats, false);
> + ret = drm_universal_plane_init(device, &plane->base, 3 /* both crtc's */,
> + &nv10_plane_funcs,
> + formats, num_formats, NULL,
> + DRM_PLANE_TYPE_OVERLAY, NULL);
Not only drm_plane_init() doesn't add much value but makes the code
harder to read. Since by calling drm_universal_plane_init() instead,
it's explicit whether the initialized plane is primary or an overlay.
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] drm/plane: Remove drm_plane_init()
2022-09-09 10:59 ` [PATCH 1/4] drm/plane: Remove drm_plane_init() Thomas Zimmermann
2022-09-09 18:24 ` Lyude Paul
2022-09-16 11:00 ` Javier Martinez Canillas
@ 2022-09-16 11:05 ` Laurent Pinchart
2 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2022-09-16 11:05 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: maarten.lankhorst, mripard, airlied, daniel, bskeggs, kherbst,
lyude, kieran.bingham+renesas, jyri.sarha, tomba, sam, dri-devel,
nouveau, linux-renesas-soc
Hi Thomas,
Thank you for the patch.
On Fri, Sep 09, 2022 at 12:59:44PM +0200, Thomas Zimmermann wrote:
> Open-code drm_plane_init() and remove the function from DRM. The
> implementation of drm_plane_init() is a simple wrapper around a call
> to drm_universal_plane_init(), so drivers can just use that instead.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/drm_modeset_helper.c | 3 +-
> drivers/gpu/drm/drm_plane.c | 32 ----------------------
> drivers/gpu/drm/nouveau/dispnv04/overlay.c | 13 +++++----
> drivers/gpu/drm/shmobile/shmob_drm_plane.c | 7 +++--
> drivers/gpu/drm/tilcdc/tilcdc_plane.c | 9 +++---
> include/drm/drm_plane.h | 8 +-----
> 6 files changed, 17 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index bd609a978848..611dd01fb604 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -100,8 +100,7 @@ EXPORT_SYMBOL(drm_helper_mode_fill_fb_struct);
> * This is the minimal list of formats that seem to be safe for modeset use
> * with all current DRM drivers. Most hardware can actually support more
> * formats than this and drivers may specify a more accurate list when
> - * creating the primary plane. However drivers that still call
> - * drm_plane_init() will use this minimal format list as the default.
> + * creating the primary plane.
> */
> static const uint32_t safe_modeset_formats[] = {
> DRM_FORMAT_XRGB8888,
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 726f2f163c26..0f14b4d3bb10 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -482,38 +482,6 @@ void drm_plane_unregister_all(struct drm_device *dev)
> }
> }
>
> -/**
> - * drm_plane_init - Initialize a legacy plane
> - * @dev: DRM device
> - * @plane: plane object to init
> - * @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
> - * @is_primary: plane type (primary vs overlay)
> - *
> - * Legacy API to initialize a DRM plane.
> - *
> - * New drivers should call drm_universal_plane_init() instead.
> - *
> - * Returns:
> - * Zero on success, error code on failure.
> - */
> -int drm_plane_init(struct drm_device *dev, struct drm_plane *plane,
> - uint32_t possible_crtcs,
> - const struct drm_plane_funcs *funcs,
> - const uint32_t *formats, unsigned int format_count,
> - bool is_primary)
> -{
> - enum drm_plane_type type;
> -
> - type = is_primary ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
> - return drm_universal_plane_init(dev, plane, possible_crtcs, funcs,
> - formats, format_count,
> - NULL, type, NULL);
> -}
> -EXPORT_SYMBOL(drm_plane_init);
> -
> /**
> * drm_plane_cleanup - Clean up the core plane usage
> * @plane: plane to cleanup
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> index 37e63e98cd08..33f29736024a 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> @@ -296,9 +296,10 @@ nv10_overlay_init(struct drm_device *device)
> break;
> }
>
> - ret = drm_plane_init(device, &plane->base, 3 /* both crtc's */,
> - &nv10_plane_funcs,
> - formats, num_formats, false);
> + ret = drm_universal_plane_init(device, &plane->base, 3 /* both crtc's */,
> + &nv10_plane_funcs,
> + formats, num_formats, NULL,
> + DRM_PLANE_TYPE_OVERLAY, NULL);
> if (ret)
> goto err;
>
> @@ -475,9 +476,9 @@ nv04_overlay_init(struct drm_device *device)
> if (!plane)
> return;
>
> - ret = drm_plane_init(device, &plane->base, 1 /* single crtc */,
> - &nv04_plane_funcs,
> - formats, 2, false);
> + ret = drm_universal_plane_init(device, &plane->base, 1 /* single crtc */,
> + &nv04_plane_funcs, formats, 2, NULL,
> + DRM_PLANE_TYPE_OVERLAY, NULL);
> if (ret)
> goto err;
>
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> index 54228424793a..6c5f0cbe7d95 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> @@ -252,9 +252,10 @@ int shmob_drm_plane_create(struct shmob_drm_device *sdev, unsigned int index)
> splane->index = index;
> splane->alpha = 255;
>
> - ret = drm_plane_init(sdev->ddev, &splane->plane, 1,
> - &shmob_drm_plane_funcs, formats,
> - ARRAY_SIZE(formats), false);
> + ret = drm_universal_plane_init(sdev->ddev, &splane->plane, 1,
> + &shmob_drm_plane_funcs,
> + formats, ARRAY_SIZE(formats), NULL,
> + DRM_PLANE_TYPE_OVERLAY, NULL);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_plane.c b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> index 0ccf791301cb..cf77a8ce7398 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> @@ -105,11 +105,10 @@ int tilcdc_plane_init(struct drm_device *dev,
> struct tilcdc_drm_private *priv = dev->dev_private;
> int ret;
>
> - ret = drm_plane_init(dev, plane, 1,
> - &tilcdc_plane_funcs,
> - priv->pixelformats,
> - priv->num_pixelformats,
> - true);
> + ret = drm_universal_plane_init(dev, plane, 1, &tilcdc_plane_funcs,
> + priv->pixelformats,
> + priv->num_pixelformats,
> + NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> dev_err(dev->dev, "Failed to initialize plane: %d\n", ret);
> return ret;
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 89ea54652e87..910cb941f3d5 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -631,7 +631,7 @@ struct drm_plane {
> unsigned int format_count;
> /**
> * @format_default: driver hasn't supplied supported formats for the
> - * plane. Used by the drm_plane_init compatibility wrapper only.
> + * plane. Used by the non-atomic driver compatibility wrapper only.
> */
> bool format_default;
>
> @@ -762,12 +762,6 @@ int drm_universal_plane_init(struct drm_device *dev,
> const uint64_t *format_modifiers,
> enum drm_plane_type type,
> const char *name, ...);
> -int drm_plane_init(struct drm_device *dev,
> - struct drm_plane *plane,
> - uint32_t possible_crtcs,
> - const struct drm_plane_funcs *funcs,
> - const uint32_t *formats, unsigned int format_count,
> - bool is_primary);
> void drm_plane_cleanup(struct drm_plane *plane);
>
> __printf(10, 11)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()
2022-09-09 10:59 ` [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc() Thomas Zimmermann
@ 2022-09-16 11:06 ` Laurent Pinchart
2022-09-16 11:31 ` Thomas Zimmermann
2022-09-16 11:22 ` Javier Martinez Canillas
1 sibling, 1 reply; 20+ messages in thread
From: Laurent Pinchart @ 2022-09-16 11:06 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: maarten.lankhorst, mripard, airlied, daniel, bskeggs, kherbst,
lyude, kieran.bingham+renesas, jyri.sarha, tomba, sam, dri-devel,
nouveau, linux-renesas-soc
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
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers
2022-09-09 10:59 ` [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers Thomas Zimmermann
@ 2022-09-16 11:20 ` Laurent Pinchart
2022-09-16 11:23 ` Javier Martinez Canillas
1 sibling, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2022-09-16 11:20 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: maarten.lankhorst, mripard, airlied, daniel, bskeggs, kherbst,
lyude, kieran.bingham+renesas, jyri.sarha, tomba, sam, dri-devel,
nouveau, linux-renesas-soc
Hi Thomas,
Thank you for the patch.
On Fri, Sep 09, 2022 at 12:59:46PM +0200, Thomas Zimmermann wrote:
> The plane update and disable helpers are only useful for non-atomic
> drivers. Print a warning if an atomic driver calls them.
>
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/drm_plane_helper.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index c7785967f5bf..1c904fc26a58 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -30,8 +30,10 @@
> #include <drm/drm_atomic_uapi.h>
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_device.h>
> +#include <drm/drm_drv.h>
> #include <drm/drm_encoder.h>
> #include <drm/drm_plane_helper.h>
> +#include <drm/drm_print.h>
> #include <drm/drm_rect.h>
>
> #define SUBPIXEL_MASK 0xffff
> @@ -195,10 +197,14 @@ int drm_plane_helper_update_primary(struct drm_plane *plane, struct drm_crtc *cr
> .x2 = crtc_x + crtc_w,
> .y2 = crtc_y + crtc_h,
> };
> + struct drm_device *dev = plane->dev;
> struct drm_connector **connector_list;
> int num_connectors, ret;
> bool visible;
>
> + if (drm_WARN_ON_ONCE(dev, drm_drv_uses_atomic_modeset(dev)))
> + return -EINVAL;
> +
> ret = drm_plane_helper_check_update(plane, crtc, fb,
> &src, &dest,
> DRM_MODE_ROTATE_0,
> @@ -260,6 +266,10 @@ EXPORT_SYMBOL(drm_plane_helper_update_primary);
> int drm_plane_helper_disable_primary(struct drm_plane *plane,
> struct drm_modeset_acquire_ctx *ctx)
> {
> + struct drm_device *dev = plane->dev;
> +
> + drm_WARN_ON_ONCE(dev, drm_drv_uses_atomic_modeset(dev));
> +
> return -EINVAL;
> }
> EXPORT_SYMBOL(drm_plane_helper_disable_primary);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()
2022-09-09 10:59 ` [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc() Thomas Zimmermann
2022-09-16 11:06 ` Laurent Pinchart
@ 2022-09-16 11:22 ` Javier Martinez Canillas
2022-09-16 11:41 ` Thomas Zimmermann
1 sibling, 1 reply; 20+ messages in thread
From: Javier Martinez Canillas @ 2022-09-16 11:22 UTC (permalink / raw)
To: Thomas Zimmermann, maarten.lankhorst, mripard, airlied, daniel,
bskeggs, kherbst, lyude, laurent.pinchart,
kieran.bingham+renesas, jyri.sarha, tomba, sam
Cc: linux-renesas-soc, nouveau, dri-devel
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>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro
2022-09-09 10:59 ` [PATCH 4/4] drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro Thomas Zimmermann
@ 2022-09-16 11:22 ` Laurent Pinchart
2022-09-16 11:24 ` Javier Martinez Canillas
1 sibling, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2022-09-16 11:22 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: maarten.lankhorst, mripard, airlied, daniel, bskeggs, kherbst,
lyude, kieran.bingham+renesas, jyri.sarha, tomba, sam, dri-devel,
nouveau, linux-renesas-soc
Hi Thomas,
Thank you for the patch.
On Fri, Sep 09, 2022 at 12:59:47PM +0200, Thomas Zimmermann wrote:
> Provide DRM_PLANE_NON_ATOMIC_FUNCS, which initializes plane functions
> of non-atomic drivers to default values. The macro is not supposed to
> be used in new code, but helps with documenting and finding existing
> users.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/drm_modeset_helper.c | 4 +---
> drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 +---
> include/drm/drm_plane_helper.h | 12 ++++++++++++
> 3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index 38040eebfa16..f858dfedf2cf 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -108,9 +108,7 @@ static const uint32_t safe_modeset_formats[] = {
> };
>
> static const struct drm_plane_funcs primary_plane_funcs = {
> - .update_plane = drm_plane_helper_update_primary,
> - .disable_plane = drm_plane_helper_disable_primary,
> - .destroy = drm_plane_helper_destroy,
> + DRM_PLANE_NON_ATOMIC_FUNCS,
> };
>
> /**
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index 6b8a014b5e97..ee92d576d277 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -1276,9 +1276,7 @@ static const uint32_t modeset_formats[] = {
> };
>
> static const struct drm_plane_funcs nv04_primary_plane_funcs = {
> - .update_plane = drm_plane_helper_update_primary,
> - .disable_plane = drm_plane_helper_disable_primary,
> - .destroy = drm_plane_helper_destroy,
> + DRM_PLANE_NON_ATOMIC_FUNCS,
> };
>
> static int nv04_crtc_vblank_handler(struct nvif_notify *notify)
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 1781fab24dd6..75f9c4830564 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -42,4 +42,16 @@ int drm_plane_helper_disable_primary(struct drm_plane *plane,
> struct drm_modeset_acquire_ctx *ctx);
> void drm_plane_helper_destroy(struct drm_plane *plane);
>
> +/**
> + * DRM_PLANE_NON_ATOMIC_FUNCS - Default plane functions for non-atomic drivers
> + *
> + * This macro initializes plane functions for non-atomic drivers to default
> + * values. Non-atomic interfaces are deprecated and should not be used in new
> + * drivers.
I wonder if we could teach checkpath.pl to catch new users.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + */
> +#define DRM_PLANE_NON_ATOMIC_FUNCS \
> + .update_plane = drm_plane_helper_update_primary, \
> + .disable_plane = drm_plane_helper_disable_primary, \
> + .destroy = drm_plane_helper_destroy
> +
> #endif
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers
2022-09-09 10:59 ` [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers Thomas Zimmermann
2022-09-16 11:20 ` Laurent Pinchart
@ 2022-09-16 11:23 ` Javier Martinez Canillas
1 sibling, 0 replies; 20+ messages in thread
From: Javier Martinez Canillas @ 2022-09-16 11:23 UTC (permalink / raw)
To: Thomas Zimmermann, maarten.lankhorst, mripard, airlied, daniel,
bskeggs, kherbst, lyude, laurent.pinchart,
kieran.bingham+renesas, jyri.sarha, tomba, sam
Cc: linux-renesas-soc, nouveau, dri-devel
On 9/9/22 12:59, Thomas Zimmermann wrote:
> The plane update and disable helpers are only useful for non-atomic
> drivers. Print a warning if an atomic driver calls them.
>
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro
2022-09-09 10:59 ` [PATCH 4/4] drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro Thomas Zimmermann
2022-09-16 11:22 ` Laurent Pinchart
@ 2022-09-16 11:24 ` Javier Martinez Canillas
1 sibling, 0 replies; 20+ messages in thread
From: Javier Martinez Canillas @ 2022-09-16 11:24 UTC (permalink / raw)
To: Thomas Zimmermann, maarten.lankhorst, mripard, airlied, daniel,
bskeggs, kherbst, lyude, laurent.pinchart,
kieran.bingham+renesas, jyri.sarha, tomba, sam
Cc: linux-renesas-soc, nouveau, dri-devel
On 9/9/22 12:59, Thomas Zimmermann wrote:
> Provide DRM_PLANE_NON_ATOMIC_FUNCS, which initializes plane functions
> of non-atomic drivers to default values. The macro is not supposed to
> be used in new code, but helps with documenting and finding existing
> users.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()
2022-09-16 11:06 ` Laurent Pinchart
@ 2022-09-16 11:31 ` Thomas Zimmermann
2022-09-19 14:45 ` Laurent Pinchart
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Zimmermann @ 2022-09-16 11:31 UTC (permalink / raw)
To: Laurent Pinchart
Cc: tomba, kherbst, airlied, sam, dri-devel, linux-renesas-soc,
kieran.bingham+renesas, bskeggs, nouveau, jyri.sarha
[-- Attachment #1.1: Type: text/plain, Size: 11303 bytes --]
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
>>
>
--
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()
2022-09-16 11:22 ` Javier Martinez Canillas
@ 2022-09-16 11:41 ` Thomas Zimmermann
2022-09-16 11:51 ` Javier Martinez Canillas
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Zimmermann @ 2022-09-16 11:41 UTC (permalink / raw)
To: Javier Martinez Canillas, maarten.lankhorst, mripard, airlied,
daniel, bskeggs, kherbst, lyude, laurent.pinchart,
kieran.bingham+renesas, jyri.sarha, tomba, sam
Cc: linux-renesas-soc, nouveau, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 3336 bytes --]
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>
>
--
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()
2022-09-16 11:41 ` Thomas Zimmermann
@ 2022-09-16 11:51 ` Javier Martinez Canillas
0 siblings, 0 replies; 20+ messages in thread
From: Javier Martinez Canillas @ 2022-09-16 11:51 UTC (permalink / raw)
To: Thomas Zimmermann, maarten.lankhorst, mripard, airlied, daniel,
bskeggs, kherbst, lyude, laurent.pinchart,
kieran.bingham+renesas, jyri.sarha, tomba, sam
Cc: linux-renesas-soc, nouveau, dri-devel
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()
2022-09-16 11:31 ` Thomas Zimmermann
@ 2022-09-19 14:45 ` Laurent Pinchart
0 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2022-09-19 14:45 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: tomba, kherbst, airlied, sam, dri-devel, linux-renesas-soc,
kieran.bingham+renesas, bskeggs, nouveau, jyri.sarha
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
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-09-19 14:45 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-09 10:59 [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups Thomas Zimmermann
2022-09-09 10:59 ` [PATCH 1/4] drm/plane: Remove drm_plane_init() Thomas Zimmermann
2022-09-09 18:24 ` Lyude Paul
2022-09-16 11:00 ` Javier Martinez Canillas
2022-09-16 11:05 ` Laurent Pinchart
2022-09-09 10:59 ` [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc() Thomas Zimmermann
2022-09-16 11:06 ` Laurent Pinchart
2022-09-16 11:31 ` Thomas Zimmermann
2022-09-19 14:45 ` Laurent Pinchart
2022-09-16 11:22 ` Javier Martinez Canillas
2022-09-16 11:41 ` Thomas Zimmermann
2022-09-16 11:51 ` Javier Martinez Canillas
2022-09-09 10:59 ` [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers Thomas Zimmermann
2022-09-16 11:20 ` Laurent Pinchart
2022-09-16 11:23 ` Javier Martinez Canillas
2022-09-09 10:59 ` [PATCH 4/4] drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro Thomas Zimmermann
2022-09-16 11:22 ` Laurent Pinchart
2022-09-16 11:24 ` Javier Martinez Canillas
2022-09-09 18:39 ` [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups Lyude Paul
2022-09-13 9:06 ` [PATCH 1/4] drm/plane: Remove drm_plane_init() jyri.sarha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).