diff mbox series

[v3,1/4] drm/panel: Add new helpers for refcounted panel allocatons

Message ID 20250330-b4-panel-refcounting-v3-1-0e0d4e4641eb@redhat.com (mailing list archive)
State New
Headers show
Series drm/panel: Panel Refcounting infrastructure | expand

Commit Message

Anusha Srivatsa March 31, 2025, 2:24 a.m. UTC 
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>
Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>

---
v3: Remove include. Fix typos. Code style corrections (Luca)
- Move the documentation to the main helper instead of in returns
  (Maxime)

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)
---
 drivers/gpu/drm/drm_panel.c | 25 +++++++++++++++++++++++++
 include/drm/drm_panel.h     | 24 ++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

Comments

Maxime Ripard March 31, 2025, 2:09 p.m. UTC | #1 
On Sun, Mar 30, 2025 at 10:24:12PM -0400, Anusha Srivatsa wrote:
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index a9c042c8dea1a82ef979c7a68204e0b55483fc28..97a5457b64fbbe9c91c6a4f41b8e1fbfe4fa604e 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -268,6 +268,30 @@ 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 a refcounted panel.
> + * The reference count is initialised to 1 and is automatically  given back
> + * by devm action.

No. I told you in my previous email that it needed to be between the arguments and returns
sections ...

> + * @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

... So here, just like you did for all the other functions you introduced.

Also, there's no reference counting yet, so that paragraph should be in
your second patch.

Maxime
Anusha Srivatsa March 31, 2025, 2:54 p.m. UTC | #2 
On Mon, Mar 31, 2025 at 10:09 AM Maxime Ripard <mripard@kernel.org> wrote:

> On Sun, Mar 30, 2025 at 10:24:12PM -0400, Anusha Srivatsa wrote:
> > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> > index
> a9c042c8dea1a82ef979c7a68204e0b55483fc28..97a5457b64fbbe9c91c6a4f41b8e1fbfe4fa604e
> 100644
> > --- a/include/drm/drm_panel.h
> > +++ b/include/drm/drm_panel.h
> > @@ -268,6 +268,30 @@ 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 a refcounted panel.
> > + * The reference count is initialised to 1 and is automatically  given
> back
> > + * by devm action.
>
> No. I told you in my previous email that it needed to be between the
> arguments and returns
> sections ...
>
> > + * @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
>
> ... So here, just like you did for all the other functions you introduced.
>
> Also, there's no reference counting yet, so that paragraph should be in
> your second patch.
>

Yup. this patch should just have the helper, arguments and returns.
Anything about refcounting should be in the next one.
Making this change.

Anusha

Maxime
>
diff mbox series

Patch

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..97a5457b64fbbe9c91c6a4f41b8e1fbfe4fa604e 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -268,6 +268,30 @@  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 a refcounted panel.
+ * The reference count is initialised to 1 and is automatically  given back
+ * by devm action.
+ *
+ * @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.
+ */
+#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);