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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 16.09.22 um 13:06 schrieb Laurent Pinchart:
Hi Thomas,

Thank you for the patch.

On Fri, Sep 09, 2022 at 12:59:45PM +0200, Thomas Zimmermann wrote:
Provide drm_univeral_plane_alloc(), which allocated an initializes a
plane. Code for non-atomic drivers uses this pattern. Convert it to
the new function. The modeset helpers contain a quirk for handling their
color formats differently. Set the flag outside plane allocation.

The new function is already deprecated to some extend. Drivers should
rather use drmm_univeral_plane_alloc() or drm_universal_plane_init().

If this is already deprecated and used by a single driver, what is the
point ?

It's used by nouveau and drm_modeset_helper.c. Since the code is duplicated, it seems generally better to have it located and documented in a central place.

Although it may look somewhat pointless now, the helper will get useful in the future. The affected code in drm_modeset_helper is in drm_crtc_init(), which is also a deprecated interface; only used by non-atomic drivers. The function is a good candidate to be inlined into calling drivers. Getting drm_crtc_init() removed will allow us to correct these drivers' color-format handling. Once that happened, several more drivers will call drm_univeral_plane_alloc().

Best regards
Thomas


Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
---
  drivers/gpu/drm/drm_modeset_helper.c    | 61 +++++++++++--------------
  drivers/gpu/drm/drm_plane.c             | 38 +++++++++++++++
  drivers/gpu/drm/nouveau/dispnv04/crtc.c | 41 ++++++-----------
  include/drm/drm_plane.h                 | 44 ++++++++++++++++++
  4 files changed, 121 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
index 611dd01fb604..38040eebfa16 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -113,38 +113,6 @@ static const struct drm_plane_funcs primary_plane_funcs = {
  	.destroy = drm_plane_helper_destroy,
  };
-static struct drm_plane *create_primary_plane(struct drm_device *dev)
-{
-	struct drm_plane *primary;
-	int ret;
-
-	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
-	if (primary == NULL) {
-		DRM_DEBUG_KMS("Failed to allocate primary plane\n");
-		return NULL;
-	}
-
-	/*
-	 * Remove the format_default field from drm_plane when dropping
-	 * this helper.
-	 */
-	primary->format_default = true;
-
-	/* possible_crtc's will be filled in later by crtc_init */
-	ret = drm_universal_plane_init(dev, primary, 0,
-				       &primary_plane_funcs,
-				       safe_modeset_formats,
-				       ARRAY_SIZE(safe_modeset_formats),
-				       NULL,
-				       DRM_PLANE_TYPE_PRIMARY, NULL);
-	if (ret) {
-		kfree(primary);
-		primary = NULL;
-	}
-
-	return primary;
-}
-
  /**
   * drm_crtc_init - Legacy CRTC initialization function
   * @dev: DRM device
@@ -176,10 +144,33 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
  		  const struct drm_crtc_funcs *funcs)
  {
  	struct drm_plane *primary;
+	int ret;
+
+	/* possible_crtc's will be filled in later by crtc_init */
+	primary = __drm_universal_plane_alloc(dev, sizeof(*primary), 0, 0,
+					      &primary_plane_funcs,
+					      safe_modeset_formats,
+					      ARRAY_SIZE(safe_modeset_formats),
+					      NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (IS_ERR(primary))
+		return PTR_ERR(primary);
- primary = create_primary_plane(dev);
-	return drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs,
-					 NULL);
+	/*
+	 * Remove the format_default field from drm_plane when dropping
+	 * this helper.
+	 */
+	primary->format_default = true;
+
+	ret = drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs, NULL);
+	if (ret)
+		goto err_drm_plane_cleanup;
+
+	return 0;
+
+err_drm_plane_cleanup:
+	drm_plane_cleanup(primary);
+	kfree(primary);
+	return ret;
  }
  EXPORT_SYMBOL(drm_crtc_init);
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.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: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux