public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] drm/panel: Panel Refcounting infrastructure
@ 2025-03-27 14:55 Anusha Srivatsa
  2025-03-27 14:55 ` [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons Anusha Srivatsa
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Anusha Srivatsa @ 2025-03-27 14:55 UTC (permalink / raw)
  To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, linux-kernel, Luca Ceresoli, Anusha Srivatsa

This series adds the infrastructure needed for the refcounting
allocations for panels similar to Luca's efforts with bridges.
Underlying intention and idea is the same - avoid use-after-free
situations in panels. Get reference to panel when in use and put
the reference back (down) when not in use.
Once this gets approved, rest of the drivers will have to be
mass converted to use this API.  All the callers of of_drm_find_panel()
will have to be converted too.

Tried to split the patches as suggested in the RFC series[1].
Also fixed the connector used during panel_init to be the one
passed by driver.

Patch 4 was not suggested or part of my initial work. Added it
after looking at the comments Luca's v8 of the bridge series
received.[2]

[1] -> https://patchwork.freedesktop.org/series/146236/
[2] -> https://patchwork.freedesktop.org/series/146306/#rev2

Suggested-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
Changes in v2:
- Change drm_panel_put() to return void.
- Export drm_panel_get()/put()
- Code cleanups: add missing return documentation, improve documentation
  in commit logs. 
- Link to v1: https://lore.kernel.org/r/20250325-b4-panel-refcounting-v1-0-4e2bf5d19c5d@redhat.com

---
Anusha Srivatsa (4):
      drm/panel: Add new helpers for refcounted panel allocatons
      drm/panel: Add refcount support
      drm/panel: deprecate old-style panel allocation
      drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc()

 drivers/gpu/drm/drm_panel.c          | 92 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/panel/panel-simple.c |  9 ++--
 include/drm/drm_panel.h              | 37 +++++++++++++++
 3 files changed, 131 insertions(+), 7 deletions(-)
---
base-commit: c8ba07caaecc622a9922cda49f24790821af8a71
change-id: 20250324-b4-panel-refcounting-40ab56aa34f7

Best regards,
-- 
Anusha Srivatsa <asrivats@redhat.com>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons
  2025-03-27 14:55 [PATCH v2 0/4] drm/panel: Panel Refcounting infrastructure Anusha Srivatsa
@ 2025-03-27 14:55 ` Anusha Srivatsa
  2025-03-27 15:57   ` Maxime Ripard
                     ` (2 more replies)
  2025-03-27 14:55 ` [PATCH v2 2/4] drm/panel: Add refcount support Anusha Srivatsa
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 16+ messages in thread
From: Anusha Srivatsa @ 2025-03-27 14:55 UTC (permalink / raw)
  To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, linux-kernel, Luca Ceresoli, Anusha Srivatsa

Introduce reference counted allocations for panels to avoid
use-after-free. The patch adds the macro devm_drm_bridge_alloc()
to allocate a new refcounted panel. Followed the documentation for
drmm_encoder_alloc() and devm_drm_dev_alloc and other similar
implementations for this purpose.

v2: Better documentation for connector_type field - follow drm_panel_init
documentation. (Luca)
- Clarify the refcount initialisation in comments.(Maxime)
- Correct the documentation of the return type (Maxime)

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/drm_panel.c | 25 +++++++++++++++++++++++++
 include/drm/drm_panel.h     | 23 +++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index c627e42a7ce70459f50eb5095fffc806ca45dabf..bdeab5710ee324dc1742fbc77582250960556308 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -355,6 +355,31 @@ struct drm_panel *of_drm_find_panel(const struct device_node *np)
 }
 EXPORT_SYMBOL(of_drm_find_panel);
 
+void *__devm_drm_panel_alloc(struct device *dev, size_t size, size_t offset,
+			     const struct drm_panel_funcs *funcs,
+			     int connector_type)
+{
+	void *container;
+	struct drm_panel *panel;
+
+	if (!funcs) {
+		dev_warn(dev, "Missing funcs pointer\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	container = devm_kzalloc(dev, size, GFP_KERNEL);
+	if (!container)
+		return ERR_PTR(-ENOMEM);
+
+	panel = container + offset;
+	panel->funcs = funcs;
+
+	drm_panel_init(panel, dev, funcs, connector_type);
+
+	return container;
+}
+EXPORT_SYMBOL(__devm_drm_panel_alloc);
+
 /**
  * of_drm_get_panel_orientation - look up the orientation of the panel through
  * the "rotation" binding from a device tree node
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index a9c042c8dea1a82ef979c7a68204e0b55483fc28..53251c6b11d78149ede3dad41ffa6a88f3c3c58b 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -28,6 +28,7 @@
 #include <linux/errno.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
+#include <linux/kref.h>
 
 struct backlight_device;
 struct dentry;
@@ -268,6 +269,28 @@ struct drm_panel {
 	bool enabled;
 };
 
+void *__devm_drm_panel_alloc(struct device *dev, size_t size, size_t offset,
+			     const struct drm_panel_funcs *funcs,
+			     int connector_type);
+
+/**
+ * devm_drm_panel_alloc - Allocate and initialize an refcounted panel
+ * @dev: struct device of the panel device
+ * @type: the type of the struct which contains struct &drm_panel
+ * @member: the name of the &drm_panel within @type
+ * @funcs: callbacks for this panel
+ * @connector_type: the connector type (DRM_MODE_CONNECTOR_*) corresponding to
+ * the panel interface
+ * Returns:
+ * Pointer to container structure embedding the panel, ERR_PTR on failure.
+ * The reference count is initialised to 1 and is automatically  given back
+ * by devm action.
+ */
+#define devm_drm_panel_alloc(dev, type, member, funcs, connector_type) \
+	((type *)__devm_drm_panel_alloc(dev, sizeof(type), \
+					offsetof(type, member), funcs, \
+					connector_type))
+
 void drm_panel_init(struct drm_panel *panel, struct device *dev,
 		    const struct drm_panel_funcs *funcs,
 		    int connector_type);

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/4] drm/panel: Add refcount support
  2025-03-27 14:55 [PATCH v2 0/4] drm/panel: Panel Refcounting infrastructure Anusha Srivatsa
  2025-03-27 14:55 ` [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons Anusha Srivatsa
@ 2025-03-27 14:55 ` Anusha Srivatsa
  2025-03-27 15:58   ` Maxime Ripard
  2025-03-28  8:48   ` Luca Ceresoli
  2025-03-27 14:55 ` [PATCH v2 3/4] drm/panel: deprecate old-style panel allocation Anusha Srivatsa
  2025-03-27 14:55 ` [PATCH v2 4/4] drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc() Anusha Srivatsa
  3 siblings, 2 replies; 16+ messages in thread
From: Anusha Srivatsa @ 2025-03-27 14:55 UTC (permalink / raw)
  To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, linux-kernel, Luca Ceresoli, Anusha Srivatsa

Allocate panel via reference counting. Add _get() and _put() helper
functions to ensure panel allocations are refcounted. Avoid use after
free by ensuring panel pointer is valid and can be usable till the last
reference is put.

v2: Export drm_panel_put/get() (Maxime)
- Change commit log with better workding (Luca, Maxime)
- Change drm_panel_put() to return void (Luca)
- Code Cleanups - add return in documentation, replace bridge to
panel (Luca)

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/drm_panel.c | 64 ++++++++++++++++++++++++++++++++++++++++++++-
 include/drm/drm_panel.h     | 14 ++++++++++
 2 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index bdeab5710ee324dc1742fbc77582250960556308..7b17531d85a4dc3031709919564d2e4d8332f748 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -355,24 +355,86 @@ struct drm_panel *of_drm_find_panel(const struct device_node *np)
 }
 EXPORT_SYMBOL(of_drm_find_panel);
 
+static void __drm_panel_free(struct kref *kref)
+{
+	struct drm_panel *panel = container_of(kref, struct drm_panel, refcount);
+
+	kfree(panel->container);
+}
+
+/**
+ * drm_panel_get - Acquire a panel reference
+ * @panel: DRM panel
+ *
+ * This function increments the panel's refcount.
+ * Returns:
+ * Pointer to @panel
+ */
+struct drm_panel *drm_panel_get(struct drm_panel *panel)
+{
+	if (!panel)
+		return panel;
+
+	kref_get(&panel->refcount);
+
+	return panel;
+}
+EXPORT_SYMBOL(drm_panel_get);
+
+/**
+ * drm_panel_put - Release a panel reference
+ * @panel: DRM panel
+ *
+ * This function decrements the panel's reference count and frees the
+ * object if the reference count drops to zero.
+ */
+void drm_panel_put(struct drm_panel *panel)
+{
+	if (panel)
+		kref_put(&panel->refcount, __drm_panel_free);
+}
+EXPORT_SYMBOL(drm_panel_put);
+
+/**
+ * drm_panel_put_void - wrapper to drm_panel_put() taking a void pointer
+ *
+ * @data: pointer to @struct drm_panel, cast to a void pointer
+ *
+ * Wrapper of drm_panel_put() to be used when a function taking a void
+ * pointer is needed, for example as a devm action.
+ */
+static void drm_panel_put_void(void *data)
+{
+	struct drm_panel *panel = (struct drm_panel *)data;
+
+	drm_panel_put(panel);
+}
+
 void *__devm_drm_panel_alloc(struct device *dev, size_t size, size_t offset,
 			     const struct drm_panel_funcs *funcs,
 			     int connector_type)
 {
 	void *container;
 	struct drm_panel *panel;
+	int err;
 
 	if (!funcs) {
 		dev_warn(dev, "Missing funcs pointer\n");
 		return ERR_PTR(-EINVAL);
 	}
 
-	container = devm_kzalloc(dev, size, GFP_KERNEL);
+	container = kzalloc(size, GFP_KERNEL);
 	if (!container)
 		return ERR_PTR(-ENOMEM);
 
 	panel = container + offset;
+	panel->container = container;
 	panel->funcs = funcs;
+	kref_init(&panel->refcount);
+
+	err = devm_add_action_or_reset(dev, drm_panel_put_void, panel);
+	if (err)
+		return ERR_PTR(err);
 
 	drm_panel_init(panel, dev, funcs, connector_type);
 
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 53251c6b11d78149ede3dad41ffa6a88f3c3c58b..5f8ae1be0532b0f861c33bb4d522b349cd469e9a 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -267,6 +267,17 @@ struct drm_panel {
 	 * If true then the panel has been enabled.
 	 */
 	bool enabled;
+
+	/**
+	 * @container: Pointer to the private driver struct embedding this
+	 * @struct drm_panel.
+	 */
+	void *container;
+
+	/**
+	 * @refcount: reference count of users referencing this panel.
+	 */
+	struct kref refcount;
 };
 
 void *__devm_drm_panel_alloc(struct device *dev, size_t size, size_t offset,
@@ -295,6 +306,9 @@ void drm_panel_init(struct drm_panel *panel, struct device *dev,
 		    const struct drm_panel_funcs *funcs,
 		    int connector_type);
 
+struct drm_panel *drm_panel_get(struct drm_panel *panel);
+void drm_panel_put(struct drm_panel *panel);
+
 void drm_panel_add(struct drm_panel *panel);
 void drm_panel_remove(struct drm_panel *panel);
 

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/4] drm/panel: deprecate old-style panel allocation
  2025-03-27 14:55 [PATCH v2 0/4] drm/panel: Panel Refcounting infrastructure Anusha Srivatsa
  2025-03-27 14:55 ` [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons Anusha Srivatsa
  2025-03-27 14:55 ` [PATCH v2 2/4] drm/panel: Add refcount support Anusha Srivatsa
@ 2025-03-27 14:55 ` Anusha Srivatsa
  2025-03-27 15:58   ` Maxime Ripard
  2025-03-28  8:51   ` Luca Ceresoli
  2025-03-27 14:55 ` [PATCH v2 4/4] drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc() Anusha Srivatsa
  3 siblings, 2 replies; 16+ messages in thread
From: Anusha Srivatsa @ 2025-03-27 14:55 UTC (permalink / raw)
  To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, linux-kernel, Luca Ceresoli, Anusha Srivatsa

Start moving to the new refcounted allocations using
the new API devm_drm_panel_alloc(). Deprecate any other
allocation.

v2: make the documentation changes in v1 more precise (Maxime)

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/drm_panel.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 7b17531d85a4dc3031709919564d2e4d8332f748..870bf8d471ee9c5fe65d88acc13661cacd883b9b 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -74,8 +74,9 @@ EXPORT_SYMBOL(drm_panel_init);
  * drm_panel_add - add a panel to the global registry
  * @panel: panel to add
  *
- * Add a panel to the global registry so that it can be looked up by display
- * drivers.
+ * Add a panel to the global registry so that it can be looked
+ * up by display drivers. The panel to be added must have been
+ * allocated by devm_drm_panel_alloc().
  */
 void drm_panel_add(struct drm_panel *panel)
 {

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/4] drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc()
  2025-03-27 14:55 [PATCH v2 0/4] drm/panel: Panel Refcounting infrastructure Anusha Srivatsa
                   ` (2 preceding siblings ...)
  2025-03-27 14:55 ` [PATCH v2 3/4] drm/panel: deprecate old-style panel allocation Anusha Srivatsa
@ 2025-03-27 14:55 ` Anusha Srivatsa
  2025-03-27 15:59   ` Maxime Ripard
  2025-03-28  8:53   ` Luca Ceresoli
  3 siblings, 2 replies; 16+ messages in thread
From: Anusha Srivatsa @ 2025-03-27 14:55 UTC (permalink / raw)
  To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: dri-devel, linux-kernel, Luca Ceresoli, Anusha Srivatsa

Start using the new helper that does the refcounted
allocations.

v2: check error condition (Luca)

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 6ba600f97aa4c8daae577823fcf17ef31b0eb46f..df718c4a86cb7dc0cd126e807d33306e5a21d8a0 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -579,9 +579,10 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 	u32 bus_flags;
 	int err;
 
-	panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL);
-	if (!panel)
-		return -ENOMEM;
+	panel = devm_drm_panel_alloc(dev, struct panel_simple, base,
+				     &panel_simple_funcs, desc->connector_type);
+	if (IS_ERR(panel))
+		return PTR_ERR(panel);
 
 	panel->desc = desc;
 
@@ -694,8 +695,6 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 	pm_runtime_set_autosuspend_delay(dev, 1000);
 	pm_runtime_use_autosuspend(dev);
 
-	drm_panel_init(&panel->base, dev, &panel_simple_funcs, connector_type);
-
 	err = drm_panel_of_backlight(&panel->base);
 	if (err) {
 		dev_err_probe(dev, err, "Could not find backlight\n");

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons
  2025-03-27 14:55 ` [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons Anusha Srivatsa
@ 2025-03-27 15:57   ` Maxime Ripard
  2025-03-27 15:58   ` Maxime Ripard
  2025-03-28  8:34   ` Luca Ceresoli
  2 siblings, 0 replies; 16+ messages in thread
From: Maxime Ripard @ 2025-03-27 15:57 UTC (permalink / raw)
  To: Anusha Srivatsa
  Cc: dri-devel, linux-kernel, David Airlie, Jessica Zhang,
	Luca Ceresoli, Maarten Lankhorst, Maxime Ripard, Neil Armstrong,
	Simona Vetter, Thomas Zimmermann

On Thu, 27 Mar 2025 10:55:39 -0400, Anusha Srivatsa wrote:
> Introduce reference counted allocations for panels to avoid
> use-after-free. The patch adds the macro devm_drm_bridge_alloc()
> to allocate a new refcounted panel. Followed the documentation for
> drmm_encoder_alloc() and devm_drm_dev_alloc and other similar
> implementations for this purpose.
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons
  2025-03-27 14:55 ` [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons Anusha Srivatsa
  2025-03-27 15:57   ` Maxime Ripard
@ 2025-03-27 15:58   ` Maxime Ripard
       [not found]     ` <CAN9Xe3QCL=KwhS0KLfaOaDc_TthQg6Gt-pLf1oEEg=1EBLZE2w@mail.gmail.com>
  2025-03-28  8:34   ` Luca Ceresoli
  2 siblings, 1 reply; 16+ messages in thread
From: Maxime Ripard @ 2025-03-27 15:58 UTC (permalink / raw)
  To: Anusha Srivatsa
  Cc: Neil Armstrong, Jessica Zhang, Maarten Lankhorst,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel, Luca Ceresoli

[-- Attachment #1: Type: text/plain, Size: 3395 bytes --]

On Thu, Mar 27, 2025 at 10:55:39AM -0400, Anusha Srivatsa wrote:
> Introduce reference counted allocations for panels to avoid
> use-after-free. The patch adds the macro devm_drm_bridge_alloc()
> to allocate a new refcounted panel. Followed the documentation for
> drmm_encoder_alloc() and devm_drm_dev_alloc and other similar
> implementations for this purpose.
> 
> v2: Better documentation for connector_type field - follow drm_panel_init
> documentation. (Luca)
> - Clarify the refcount initialisation in comments.(Maxime)
> - Correct the documentation of the return type (Maxime)
> 
> Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
> ---
>  drivers/gpu/drm/drm_panel.c | 25 +++++++++++++++++++++++++
>  include/drm/drm_panel.h     | 23 +++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index c627e42a7ce70459f50eb5095fffc806ca45dabf..bdeab5710ee324dc1742fbc77582250960556308 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -355,6 +355,31 @@ struct drm_panel *of_drm_find_panel(const struct device_node *np)
>  }
>  EXPORT_SYMBOL(of_drm_find_panel);
>  
> +void *__devm_drm_panel_alloc(struct device *dev, size_t size, size_t offset,
> +			     const struct drm_panel_funcs *funcs,
> +			     int connector_type)
> +{
> +	void *container;
> +	struct drm_panel *panel;
> +
> +	if (!funcs) {
> +		dev_warn(dev, "Missing funcs pointer\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	container = devm_kzalloc(dev, size, GFP_KERNEL);
> +	if (!container)
> +		return ERR_PTR(-ENOMEM);
> +
> +	panel = container + offset;
> +	panel->funcs = funcs;
> +
> +	drm_panel_init(panel, dev, funcs, connector_type);
> +
> +	return container;
> +}
> +EXPORT_SYMBOL(__devm_drm_panel_alloc);
> +
>  /**
>   * of_drm_get_panel_orientation - look up the orientation of the panel through
>   * the "rotation" binding from a device tree node
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index a9c042c8dea1a82ef979c7a68204e0b55483fc28..53251c6b11d78149ede3dad41ffa6a88f3c3c58b 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -28,6 +28,7 @@
>  #include <linux/errno.h>
>  #include <linux/list.h>
>  #include <linux/mutex.h>
> +#include <linux/kref.h>
>  
>  struct backlight_device;
>  struct dentry;
> @@ -268,6 +269,28 @@ struct drm_panel {
>  	bool enabled;
>  };
>  
> +void *__devm_drm_panel_alloc(struct device *dev, size_t size, size_t offset,
> +			     const struct drm_panel_funcs *funcs,
> +			     int connector_type);
> +
> +/**
> + * devm_drm_panel_alloc - Allocate and initialize an refcounted panel
> + * @dev: struct device of the panel device
> + * @type: the type of the struct which contains struct &drm_panel
> + * @member: the name of the &drm_panel within @type
> + * @funcs: callbacks for this panel
> + * @connector_type: the connector type (DRM_MODE_CONNECTOR_*) corresponding to
> + * the panel interface
> + * Returns:
> + * Pointer to container structure embedding the panel, ERR_PTR on failure.
> + * The reference count is initialised to 1 and is automatically  given back
> + * by devm action.

Sorry, I noticed after the facts, but this can't be in the Returns
section, it needs to be in the main one.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/4] drm/panel: Add refcount support
  2025-03-27 14:55 ` [PATCH v2 2/4] drm/panel: Add refcount support Anusha Srivatsa
@ 2025-03-27 15:58   ` Maxime Ripard
  2025-03-28  8:48   ` Luca Ceresoli
  1 sibling, 0 replies; 16+ messages in thread
From: Maxime Ripard @ 2025-03-27 15:58 UTC (permalink / raw)
  To: Anusha Srivatsa
  Cc: dri-devel, linux-kernel, David Airlie, Jessica Zhang,
	Luca Ceresoli, Maarten Lankhorst, Maxime Ripard, Neil Armstrong,
	Simona Vetter, Thomas Zimmermann

On Thu, 27 Mar 2025 10:55:40 -0400, Anusha Srivatsa wrote:
> Allocate panel via reference counting. Add _get() and _put() helper
> functions to ensure panel allocations are refcounted. Avoid use after
> free by ensuring panel pointer is valid and can be usable till the last
> reference is put.
> 
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/4] drm/panel: deprecate old-style panel allocation
  2025-03-27 14:55 ` [PATCH v2 3/4] drm/panel: deprecate old-style panel allocation Anusha Srivatsa
@ 2025-03-27 15:58   ` Maxime Ripard
  2025-03-28  8:51   ` Luca Ceresoli
  1 sibling, 0 replies; 16+ messages in thread
From: Maxime Ripard @ 2025-03-27 15:58 UTC (permalink / raw)
  To: Anusha Srivatsa
  Cc: dri-devel, linux-kernel, David Airlie, Jessica Zhang,
	Luca Ceresoli, Maarten Lankhorst, Maxime Ripard, Neil Armstrong,
	Simona Vetter, Thomas Zimmermann

On Thu, 27 Mar 2025 10:55:41 -0400, Anusha Srivatsa wrote:
> Start moving to the new refcounted allocations using
> the new API devm_drm_panel_alloc(). Deprecate any other
> allocation.
> 
> v2: make the documentation changes in v1 more precise (Maxime)
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/4] drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc()
  2025-03-27 14:55 ` [PATCH v2 4/4] drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc() Anusha Srivatsa
@ 2025-03-27 15:59   ` Maxime Ripard
  2025-03-28  8:53   ` Luca Ceresoli
  1 sibling, 0 replies; 16+ messages in thread
From: Maxime Ripard @ 2025-03-27 15:59 UTC (permalink / raw)
  To: Anusha Srivatsa
  Cc: dri-devel, linux-kernel, David Airlie, Jessica Zhang,
	Luca Ceresoli, Maarten Lankhorst, Maxime Ripard, Neil Armstrong,
	Simona Vetter, Thomas Zimmermann

On Thu, 27 Mar 2025 10:55:42 -0400, Anusha Srivatsa wrote:
> Start using the new helper that does the refcounted
> allocations.
> 
> v2: check error condition (Luca)
> 
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons
  2025-03-27 14:55 ` [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons Anusha Srivatsa
  2025-03-27 15:57   ` Maxime Ripard
  2025-03-27 15:58   ` Maxime Ripard
@ 2025-03-28  8:34   ` Luca Ceresoli
  2 siblings, 0 replies; 16+ messages in thread
From: Luca Ceresoli @ 2025-03-28  8:34 UTC (permalink / raw)
  To: Anusha Srivatsa
  Cc: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel

Hello Anusha,

Thanks for your continued effort.

I have a few minor comments. Nothing big, but since Maxime requested a
change you'll have to send a new iteration, so find my comments below.

On Thu, 27 Mar 2025 10:55:39 -0400
Anusha Srivatsa <asrivats@redhat.com> wrote:

[...]

> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index a9c042c8dea1a82ef979c7a68204e0b55483fc28..53251c6b11d78149ede3dad41ffa6a88f3c3c58b 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -28,6 +28,7 @@
>  #include <linux/errno.h>
>  #include <linux/list.h>
>  #include <linux/mutex.h>
> +#include <linux/kref.h>

Minor nit: you don't need this include in patch 1. You should move it
to patch 2 where it is actually used.

> @@ -268,6 +269,28 @@ struct drm_panel {
>  	bool enabled;
>  };
>  
> +void *__devm_drm_panel_alloc(struct device *dev, size_t size, size_t offset,
> +			     const struct drm_panel_funcs *funcs,
> +			     int connector_type);
> +
> +/**
> + * devm_drm_panel_alloc - Allocate and initialize an refcounted panel
                                                     ^^
A typo here is certainly not a huge problem, but I think I had already
reported this should be "a refcounted panel".

> + * @dev: struct device of the panel device
> + * @type: the type of the struct which contains struct &drm_panel
> + * @member: the name of the &drm_panel within @type
> + * @funcs: callbacks for this panel
> + * @connector_type: the connector type (DRM_MODE_CONNECTOR_*) corresponding to
> + * the panel interface
> + * Returns:
> + * Pointer to container structure embedding the panel, ERR_PTR on failure.
> + * The reference count is initialised to 1 and is automatically  given back
> + * by devm action.
> + */

In addition to Maxime's comment: I think it's a common practice to have
an empty line after the last @argument and also before the "Returns:"
line, to improve readability

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/4] drm/panel: Add refcount support
  2025-03-27 14:55 ` [PATCH v2 2/4] drm/panel: Add refcount support Anusha Srivatsa
  2025-03-27 15:58   ` Maxime Ripard
@ 2025-03-28  8:48   ` Luca Ceresoli
  1 sibling, 0 replies; 16+ messages in thread
From: Luca Ceresoli @ 2025-03-28  8:48 UTC (permalink / raw)
  To: Anusha Srivatsa
  Cc: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel

On Thu, 27 Mar 2025 10:55:40 -0400
Anusha Srivatsa <asrivats@redhat.com> wrote:

> Allocate panel via reference counting. Add _get() and _put() helper
> functions to ensure panel allocations are refcounted. Avoid use after
> free by ensuring panel pointer is valid and can be usable till the last
> reference is put.
> 
> v2: Export drm_panel_put/get() (Maxime)
> - Change commit log with better workding (Luca, Maxime)
> - Change drm_panel_put() to return void (Luca)
> - Code Cleanups - add return in documentation, replace bridge to
> panel (Luca)
> 
> Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>

With the added #include as per my comment on patch 1, you can add:

 Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/4] drm/panel: deprecate old-style panel allocation
  2025-03-27 14:55 ` [PATCH v2 3/4] drm/panel: deprecate old-style panel allocation Anusha Srivatsa
  2025-03-27 15:58   ` Maxime Ripard
@ 2025-03-28  8:51   ` Luca Ceresoli
  1 sibling, 0 replies; 16+ messages in thread
From: Luca Ceresoli @ 2025-03-28  8:51 UTC (permalink / raw)
  To: Anusha Srivatsa
  Cc: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel

On Thu, 27 Mar 2025 10:55:41 -0400
Anusha Srivatsa <asrivats@redhat.com> wrote:

> Start moving to the new refcounted allocations using
> the new API devm_drm_panel_alloc(). Deprecate any other
> allocation.
> 
> v2: make the documentation changes in v1 more precise (Maxime)

Note that the changelog (list of changes since previous versions)
should be...

> 
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
> ---

...here, just after the '---' line. In such way it will not be applied
by 'git am' and similar tools and will not appear in the final commit
when your patch is applied. The patch history is not needed in the
final commit.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/4] drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc()
  2025-03-27 14:55 ` [PATCH v2 4/4] drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc() Anusha Srivatsa
  2025-03-27 15:59   ` Maxime Ripard
@ 2025-03-28  8:53   ` Luca Ceresoli
       [not found]     ` <CAN9Xe3SzU0AohuBnyJtE0UWFkrW0iMGKH1F8cuUZYLZ-vbfkpw@mail.gmail.com>
  1 sibling, 1 reply; 16+ messages in thread
From: Luca Ceresoli @ 2025-03-28  8:53 UTC (permalink / raw)
  To: Anusha Srivatsa
  Cc: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel

On Thu, 27 Mar 2025 10:55:42 -0400
Anusha Srivatsa <asrivats@redhat.com> wrote:

> Start using the new helper that does the refcounted
> allocations.
> 
> v2: check error condition (Luca)

Here as well, when you resend, move the changelog after the '---' line.

> Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons
       [not found]     ` <CAN9Xe3QCL=KwhS0KLfaOaDc_TthQg6Gt-pLf1oEEg=1EBLZE2w@mail.gmail.com>
@ 2025-03-28  9:05       ` Maxime Ripard
  0 siblings, 0 replies; 16+ messages in thread
From: Maxime Ripard @ 2025-03-28  9:05 UTC (permalink / raw)
  To: Anusha Srivatsa
  Cc: Neil Armstrong, Jessica Zhang, Maarten Lankhorst,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel, Luca Ceresoli

[-- Attachment #1: Type: text/plain, Size: 4385 bytes --]

On Thu, Mar 27, 2025 at 11:33:15AM -0400, Anusha Srivatsa wrote:
> On Thu, Mar 27, 2025 at 11:58 AM Maxime Ripard <mripard@kernel.org> wrote:
> 
> > On Thu, Mar 27, 2025 at 10:55:39AM -0400, Anusha Srivatsa wrote:
> > > Introduce reference counted allocations for panels to avoid
> > > use-after-free. The patch adds the macro devm_drm_bridge_alloc()
> > > to allocate a new refcounted panel. Followed the documentation for
> > > drmm_encoder_alloc() and devm_drm_dev_alloc and other similar
> > > implementations for this purpose.
> > >
> > > v2: Better documentation for connector_type field - follow drm_panel_init
> > > documentation. (Luca)
> > > - Clarify the refcount initialisation in comments.(Maxime)
> > > - Correct the documentation of the return type (Maxime)
> > >
> > > Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
> > > ---
> > >  drivers/gpu/drm/drm_panel.c | 25 +++++++++++++++++++++++++
> > >  include/drm/drm_panel.h     | 23 +++++++++++++++++++++++
> > >  2 files changed, 48 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> > > index
> > c627e42a7ce70459f50eb5095fffc806ca45dabf..bdeab5710ee324dc1742fbc77582250960556308
> > 100644
> > > --- a/drivers/gpu/drm/drm_panel.c
> > > +++ b/drivers/gpu/drm/drm_panel.c
> > > @@ -355,6 +355,31 @@ struct drm_panel *of_drm_find_panel(const struct
> > device_node *np)
> > >  }
> > >  EXPORT_SYMBOL(of_drm_find_panel);
> > >
> > > +void *__devm_drm_panel_alloc(struct device *dev, size_t size, size_t
> > offset,
> > > +                          const struct drm_panel_funcs *funcs,
> > > +                          int connector_type)
> > > +{
> > > +     void *container;
> > > +     struct drm_panel *panel;
> > > +
> > > +     if (!funcs) {
> > > +             dev_warn(dev, "Missing funcs pointer\n");
> > > +             return ERR_PTR(-EINVAL);
> > > +     }
> > > +
> > > +     container = devm_kzalloc(dev, size, GFP_KERNEL);
> > > +     if (!container)
> > > +             return ERR_PTR(-ENOMEM);
> > > +
> > > +     panel = container + offset;
> > > +     panel->funcs = funcs;
> > > +
> > > +     drm_panel_init(panel, dev, funcs, connector_type);
> > > +
> > > +     return container;
> > > +}
> > > +EXPORT_SYMBOL(__devm_drm_panel_alloc);
> > > +
> > >  /**
> > >   * of_drm_get_panel_orientation - look up the orientation of the panel
> > through
> > >   * the "rotation" binding from a device tree node
> > > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> > > index
> > a9c042c8dea1a82ef979c7a68204e0b55483fc28..53251c6b11d78149ede3dad41ffa6a88f3c3c58b
> > 100644
> > > --- a/include/drm/drm_panel.h
> > > +++ b/include/drm/drm_panel.h
> > > @@ -28,6 +28,7 @@
> > >  #include <linux/errno.h>
> > >  #include <linux/list.h>
> > >  #include <linux/mutex.h>
> > > +#include <linux/kref.h>
> > >
> > >  struct backlight_device;
> > >  struct dentry;
> > > @@ -268,6 +269,28 @@ struct drm_panel {
> > >       bool enabled;
> > >  };
> > >
> > > +void *__devm_drm_panel_alloc(struct device *dev, size_t size, size_t
> > offset,
> > > +                          const struct drm_panel_funcs *funcs,
> > > +                          int connector_type);
> > > +
> > > +/**
> > > + * devm_drm_panel_alloc - Allocate and initialize an refcounted panel
> > > + * @dev: struct device of the panel device
> > > + * @type: the type of the struct which contains struct &drm_panel
> > > + * @member: the name of the &drm_panel within @type
> > > + * @funcs: callbacks for this panel
> > > + * @connector_type: the connector type (DRM_MODE_CONNECTOR_*)
> > corresponding to
> > > + * the panel interface
> > > + * Returns:
> > > + * Pointer to container structure embedding the panel, ERR_PTR on
> > failure.
> > > + * The reference count is initialised to 1 and is automatically  given
> > back
> > > + * by devm action.
> >
> > Sorry, I noticed after the facts, but this can't be in the Returns
> > section, it needs to be in the main one.
>
> Maxime, Not really following you. Are you suggesting this explanation
> needs to be in the helper documentation instead of in returns?

This is a general documentation thing, so it needs to be in the main
documentation section, thus between the argumnts and returned values
one.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/4] drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc()
       [not found]     ` <CAN9Xe3SzU0AohuBnyJtE0UWFkrW0iMGKH1F8cuUZYLZ-vbfkpw@mail.gmail.com>
@ 2025-03-31 15:56       ` Luca Ceresoli
  0 siblings, 0 replies; 16+ messages in thread
From: Luca Ceresoli @ 2025-03-31 15:56 UTC (permalink / raw)
  To: Anusha Srivatsa
  Cc: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel

On Fri, 28 Mar 2025 12:09:08 -0400
Anusha Srivatsa <asrivats@redhat.com> wrote:

> On Fri, Mar 28, 2025 at 4:54 AM Luca Ceresoli <luca.ceresoli@bootlin.com>
> wrote:
> 
> > On Thu, 27 Mar 2025 10:55:42 -0400
> > Anusha Srivatsa <asrivats@redhat.com> wrote:
> >  
> > > Start using the new helper that does the refcounted
> > > allocations.
> > >
> > > v2: check error condition (Luca)  
> >
> > Here as well, when you resend, move the changelog after the '---' line.
> >
> >  
> Hadn't noticed this. Saw some other series that do follow this method. I
> will make this change.

That's the general rule [0], even though not all maintainers are strict
about this, so they sometimes get through.

[0] https://docs.kernel.org/process/submitting-patches.html#commentary

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-03-31 15:56 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-27 14:55 [PATCH v2 0/4] drm/panel: Panel Refcounting infrastructure Anusha Srivatsa
2025-03-27 14:55 ` [PATCH v2 1/4] drm/panel: Add new helpers for refcounted panel allocatons Anusha Srivatsa
2025-03-27 15:57   ` Maxime Ripard
2025-03-27 15:58   ` Maxime Ripard
     [not found]     ` <CAN9Xe3QCL=KwhS0KLfaOaDc_TthQg6Gt-pLf1oEEg=1EBLZE2w@mail.gmail.com>
2025-03-28  9:05       ` Maxime Ripard
2025-03-28  8:34   ` Luca Ceresoli
2025-03-27 14:55 ` [PATCH v2 2/4] drm/panel: Add refcount support Anusha Srivatsa
2025-03-27 15:58   ` Maxime Ripard
2025-03-28  8:48   ` Luca Ceresoli
2025-03-27 14:55 ` [PATCH v2 3/4] drm/panel: deprecate old-style panel allocation Anusha Srivatsa
2025-03-27 15:58   ` Maxime Ripard
2025-03-28  8:51   ` Luca Ceresoli
2025-03-27 14:55 ` [PATCH v2 4/4] drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc() Anusha Srivatsa
2025-03-27 15:59   ` Maxime Ripard
2025-03-28  8:53   ` Luca Ceresoli
     [not found]     ` <CAN9Xe3SzU0AohuBnyJtE0UWFkrW0iMGKH1F8cuUZYLZ-vbfkpw@mail.gmail.com>
2025-03-31 15:56       ` Luca Ceresoli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox