thr3ads.net - Nouveau - [Nouveau] [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers [Sep 2022]

If this information is useful, please help other people find it:
Share via:

Thomas Zimmermann

2022-Sep-09 10:59 UTC

[Nouveau] [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups

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

Thomas Zimmermann

2022-Sep-09 10:59 UTC

head link

[Nouveau] [PATCH 1/4] drm/plane: Remove drm_plane_init()

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 at 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

Thomas Zimmermann

2022-Sep-09 10:59 UTC

head link

[Nouveau] [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()

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 at suse.de>
---
 drivers/gpu/drm/drm_modeset_helper.c    | 61 +++++++++++--------------
 drivers/gpu/drm/drm_plane.c             | 38 +++++++++++++++
 drivers/gpu/drm/nouveau/dispnv04/crtc.c | 41 ++++++-----------
 include/drm/drm_plane.h                 | 44 ++++++++++++++++++
 4 files changed, 121 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/drm_modeset_helper.c
b/drivers/gpu/drm/drm_modeset_helper.c
index 611dd01fb604..38040eebfa16 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -113,38 +113,6 @@ static const struct drm_plane_funcs primary_plane_funcs = {
 	.destroy = drm_plane_helper_destroy,
 };
 
-static struct drm_plane *create_primary_plane(struct drm_device *dev)
-{
-	struct drm_plane *primary;
-	int ret;
-
-	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
-	if (primary == NULL) {
-		DRM_DEBUG_KMS("Failed to allocate primary plane\n");
-		return NULL;
-	}
-
-	/*
-	 * Remove the format_default field from drm_plane when dropping
-	 * this helper.
-	 */
-	primary->format_default = true;
-
-	/* possible_crtc's will be filled in later by crtc_init */
-	ret = drm_universal_plane_init(dev, primary, 0,
-				       &primary_plane_funcs,
-				       safe_modeset_formats,
-				       ARRAY_SIZE(safe_modeset_formats),
-				       NULL,
-				       DRM_PLANE_TYPE_PRIMARY, NULL);
-	if (ret) {
-		kfree(primary);
-		primary = NULL;
-	}
-
-	return primary;
-}
-
 /**
  * drm_crtc_init - Legacy CRTC initialization function
  * @dev: DRM device
@@ -176,10 +144,33 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc
*crtc,
 		  const struct drm_crtc_funcs *funcs)
 {
 	struct drm_plane *primary;
+	int ret;
+
+	/* possible_crtc's will be filled in later by crtc_init */
+	primary = __drm_universal_plane_alloc(dev, sizeof(*primary), 0, 0,
+					      &primary_plane_funcs,
+					      safe_modeset_formats,
+					      ARRAY_SIZE(safe_modeset_formats),
+					      NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (IS_ERR(primary))
+		return PTR_ERR(primary);
 
-	primary = create_primary_plane(dev);
-	return drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs,
-					 NULL);
+	/*
+	 * Remove the format_default field from drm_plane when dropping
+	 * this helper.
+	 */
+	primary->format_default = true;
+
+	ret = drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs, NULL);
+	if (ret)
+		goto err_drm_plane_cleanup;
+
+	return 0;
+
+err_drm_plane_cleanup:
+	drm_plane_cleanup(primary);
+	kfree(primary);
+	return ret;
 }
 EXPORT_SYMBOL(drm_crtc_init);
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 0f14b4d3bb10..33357629a7f5 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -448,6 +448,44 @@ void *__drmm_universal_plane_alloc(struct drm_device *dev,
size_t size,
 }
 EXPORT_SYMBOL(__drmm_universal_plane_alloc);
 
+void *__drm_universal_plane_alloc(struct drm_device *dev, size_t size,
+				  size_t offset, uint32_t possible_crtcs,
+				  const struct drm_plane_funcs *funcs,
+				  const uint32_t *formats, unsigned int format_count,
+				  const uint64_t *format_modifiers,
+				  enum drm_plane_type type,
+				  const char *name, ...)
+{
+	void *container;
+	struct drm_plane *plane;
+	va_list ap;
+	int ret;
+
+	if (drm_WARN_ON(dev, !funcs))
+		return ERR_PTR(-EINVAL);
+
+	container = kzalloc(size, GFP_KERNEL);
+	if (!container)
+		return ERR_PTR(-ENOMEM);
+
+	plane = container + offset;
+
+	va_start(ap, name);
+	ret = __drm_universal_plane_init(dev, plane, possible_crtcs, funcs,
+					 formats, format_count, format_modifiers,
+					 type, name, ap);
+	va_end(ap);
+	if (ret)
+		goto err_kfree;
+
+	return container;
+
+err_kfree:
+	kfree(container);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(__drm_universal_plane_alloc);
+
 int drm_plane_register_all(struct drm_device *dev)
 {
 	unsigned int num_planes = 0;
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 660c4cbc0b3d..6b8a014b5e97 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -1281,32 +1281,6 @@ static const struct drm_plane_funcs
nv04_primary_plane_funcs = {
 	.destroy = drm_plane_helper_destroy,
 };
 
-static struct drm_plane *
-create_primary_plane(struct drm_device *dev)
-{
-        struct drm_plane *primary;
-        int ret;
-
-        primary = kzalloc(sizeof(*primary), GFP_KERNEL);
-        if (primary == NULL) {
-                DRM_DEBUG_KMS("Failed to allocate primary plane\n");
-                return NULL;
-        }
-
-        /* possible_crtc's will be filled in later by crtc_init */
-        ret = drm_universal_plane_init(dev, primary, 0,
-				       &nv04_primary_plane_funcs,
-                                       modeset_formats,
-                                       ARRAY_SIZE(modeset_formats), NULL,
-                                       DRM_PLANE_TYPE_PRIMARY, NULL);
-        if (ret) {
-                kfree(primary);
-                primary = NULL;
-        }
-
-        return primary;
-}
-
 static int nv04_crtc_vblank_handler(struct nvif_notify *notify)
 {
 	struct nouveau_crtc *nv_crtc @@ -1321,6 +1295,7 @@ nv04_crtc_create(struct
drm_device *dev, int crtc_num)
 {
 	struct nouveau_display *disp = nouveau_display(dev);
 	struct nouveau_crtc *nv_crtc;
+	struct drm_plane *primary;
 	int ret;
 
 	nv_crtc = kzalloc(sizeof(*nv_crtc), GFP_KERNEL);
@@ -1335,8 +1310,18 @@ nv04_crtc_create(struct drm_device *dev, int crtc_num)
 	nv_crtc->save = nv_crtc_save;
 	nv_crtc->restore = nv_crtc_restore;
 
-	drm_crtc_init_with_planes(dev, &nv_crtc->base,
-                                  create_primary_plane(dev), NULL,
+	primary = __drm_universal_plane_alloc(dev, sizeof(*primary), 0, 0,
+					      &nv04_primary_plane_funcs,
+					      modeset_formats,
+					      ARRAY_SIZE(modeset_formats), NULL,
+					      DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (IS_ERR(primary)) {
+		ret = PTR_ERR(primary);
+		kfree(nv_crtc);
+		return ret;
+	}
+
+	drm_crtc_init_with_planes(dev, &nv_crtc->base, primary, NULL,
                                   &nv04_crtc_funcs, NULL);
 	drm_crtc_helper_add(&nv_crtc->base, &nv04_crtc_helper_funcs);
 	drm_mode_crtc_set_gamma_size(&nv_crtc->base, 256);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 910cb941f3d5..21dfa7f97948 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -809,6 +809,50 @@ void *__drmm_universal_plane_alloc(struct drm_device *dev,
 					      format_count, format_modifiers, \
 					      plane_type, name, ##__VA_ARGS__))
 
+__printf(10, 11)
+void *__drm_universal_plane_alloc(struct drm_device *dev,
+				  size_t size, size_t offset,
+				  uint32_t possible_crtcs,
+				  const struct drm_plane_funcs *funcs,
+				  const uint32_t *formats,
+				  unsigned int format_count,
+				  const uint64_t *format_modifiers,
+				  enum drm_plane_type plane_type,
+				  const char *name, ...);
+
+/**
+ * drm_universal_plane_alloc - Allocate and initialize an universal plane
object
+ * @dev: DRM device
+ * @type: the type of the struct which contains struct &drm_plane
+ * @member: the name of the &drm_plane within @type
+ * @possible_crtcs: bitmask of possible CRTCs
+ * @funcs: callbacks for the new plane
+ * @formats: array of supported formats (DRM_FORMAT\_\*)
+ * @format_count: number of elements in @formats
+ * @format_modifiers: array of struct drm_format modifiers terminated by
+ *                    DRM_FORMAT_MOD_INVALID
+ * @plane_type: type of plane (overlay, primary, cursor)
+ * @name: printf style format string for the plane name, or NULL for default
name
+ *
+ * Allocates and initializes a plane object of type @type. The caller
+ * is responsible for releasing the allocated memory with kfree().
+ *
+ * Drivers are encouraged to use drmm_universal_plane_alloc() instead.
+ *
+ * Drivers that only support the DRM_FORMAT_MOD_LINEAR modifier support may set
+ * @format_modifiers to NULL. The plane will advertise the linear modifier.
+ *
+ * Returns:
+ * Pointer to new plane, or ERR_PTR on failure.
+ */
+#define drm_universal_plane_alloc(dev, type, member, possible_crtcs, funcs,
formats, \
+				   format_count, format_modifiers, plane_type, name, ...) \
+	((type *)__drm_universal_plane_alloc(dev, sizeof(type), \
+					     offsetof(type, member), \
+					     possible_crtcs, funcs, formats, \
+					     format_count, format_modifiers, \
+					     plane_type, name, ##__VA_ARGS__))
+
 /**
  * drm_plane_index - find the index of a registered plane
  * @plane: plane to find index for
-- 
2.37.2

Thomas Zimmermann

2022-Sep-09 10:59 UTC

head link

[Nouveau] [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers

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 at ffwll.ch>
Signed-off-by: Thomas Zimmermann <tzimmermann at 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

Thomas Zimmermann

2022-Sep-09 10:59 UTC

head link

[Nouveau] [PATCH 4/4] drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro

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 at 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

Lyude Paul

2022-Sep-09 18:39 UTC

head link

[Nouveau] [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups

For the nouveau bits on 1, 2 and 4:

Reviewed-by: Lyude Paul <lyude at 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

jyri.sarha at iki.fi

2022-Sep-13 09:06 UTC

head link

[Nouveau] [PATCH 1/4] drm/plane: Remove drm_plane_init()

September 9, 2022 at 1:59 PM, "Thomas Zimmermann" <tzimmermann at
suse.de mailto:tzimmermann at
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 at suse.de>
Acked-by: Jyri Sarha <jyri.sarha at 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
>

Nouveau - Sep 2022 - [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers

[Nouveau] [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups

[Nouveau] [PATCH 1/4] drm/plane: Remove drm_plane_init()

[Nouveau] [PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()

[Nouveau] [PATCH 3/4] drm/plane-helper: Warn if atomic drivers call non-atomic helpers

[Nouveau] [PATCH 4/4] drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro

[Nouveau] [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups

[Nouveau] [PATCH 1/4] drm/plane: Remove drm_plane_init()