[PATCH v2 00/16] Use drm_panel_get/put() in of_drm_find_panel() and its callers

[PATCH v2 00/16] Use drm_panel_get/put() in of_drm_find_panel() and its callers

[Anusha Srivatsa]

of_drm_find_panel() returns a pointer to a panel which the caller
holds to either do panel setup with the prepare(), enable() like
panel helpers or for APIs like devm_drm_panel_bridge_add(),
devm_drm_panel_bridge_add_typed() which expect a panel.
Either way, once the driver no longer needs the panel,
put it down with a drm_panel_put()

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
Changes in v2:
- Get the missing reference in of_drm_find_panel()
- Add changes to all drivers that call of_drm_find_panel() 
- Link to v1: https://lore.kernel.org/r/20250604-of_drm_find_panel_part1-v1-0-c632e12e131d@redhat.com

---
Anusha Srivatsa (16):
      drm/panel: get/put panel reference in drm_panel_add/remove()
      drm/panel: get the panel returned by of_drm_find_panel()
      drm/panel/samsung-dsicm: Keep up with refcounting
      drm/panel/sun4i: Keep up with refcounting
      drm/panel/exynos: Keep up with refcounting
      drm/panel/rcar_du_encoder: Keep up with refcounting
      drm/panel/rz-du: Keep up with refcounting
      drm/bridge/lvds-codec: Keep up with refcounting
      drm/bridge/fsl-ldb: Keep up with refcounting.
      drm/panel/omapdrm: Keep up with refcounting
      drm/panel/sti: Keep up with refcounting
      drm/drm_of: Keep up with refcounting
      drm/fsl-dcu: Keep up with refcounting
      drm/mcde: Keep up with refcounting
      drm/bridge/analogix: Keep up with refcounting
      drm/tegra: Keep up with refcounting

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 +
 drivers/gpu/drm/bridge/fsl-ldb.c                   | 3 +++
 drivers/gpu/drm/bridge/lvds-codec.c                | 1 +
 drivers/gpu/drm/bridge/samsung-dsim.c              | 1 +
 drivers/gpu/drm/drm_of.c                           | 2 ++
 drivers/gpu/drm/drm_panel.c                        | 6 ++++++
 drivers/gpu/drm/exynos/exynos_drm_dpi.c            | 1 +
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c          | 1 +
 drivers/gpu/drm/mcde/mcde_dsi.c                    | 5 ++++-
 drivers/gpu/drm/omapdrm/dss/output.c               | 6 ++++--
 drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c  | 2 ++
 drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c   | 1 +
 drivers/gpu/drm/sti/sti_dvo.c                      | 2 ++
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c             | 2 ++
 drivers/gpu/drm/tegra/dsi.c                        | 1 +
 15 files changed, 32 insertions(+), 3 deletions(-)
---
base-commit: 60ba94338047bb5410a3626ced3380afe9285ed8
change-id: 20250613-b4-of_drm_find_panel_part1-ad57c4f88a0c

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

[PATCH v2 01/16] drm/panel: get/put panel reference in drm_panel_add/remove()

[Anusha Srivatsa]

Take the panel reference and put it back as required.
drm_panel_add() and drm_panel_remove() add a panel to
the global registry and removes a panel respectively.
Use get() and put() helpers to keep up with refcounting.

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

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 805b4151ccefd8ec0107951162c3b71446102ffb..ced6b08923b2150ebda6e1d9001517103895def1 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -80,6 +80,7 @@ EXPORT_SYMBOL(drm_panel_init);
  */
 void drm_panel_add(struct drm_panel *panel)
 {
+	drm_panel_get(panel);
 	mutex_lock(&panel_lock);
 	list_add_tail(&panel->list, &panel_list);
 	mutex_unlock(&panel_lock);
@@ -97,6 +98,7 @@ void drm_panel_remove(struct drm_panel *panel)
 	mutex_lock(&panel_lock);
 	list_del_init(&panel->list);
 	mutex_unlock(&panel_lock);
+	drm_panel_put(panel);
 }
 EXPORT_SYMBOL(drm_panel_remove);
 

-- 
2.48.1

[PATCH v2 02/16] drm/panel: get the panel returned by of_drm_find_panel()

[Anusha Srivatsa]

of_drm_find_panel() returns a panel pointer that the caller
holds. With refcounting in place, increment the refcount
of the panel that is returned. This reference has to be put()
by the caller.
Document the behaviour expected from all callers of this
API.

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

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index ced6b08923b2150ebda6e1d9001517103895def1..a3aab725f91f76661185b3ebdf3d9b337bdf230f 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -401,6 +401,9 @@ EXPORT_SYMBOL(__devm_drm_panel_alloc);
  * Searches the set of registered panels for one that matches the given device
  * tree node. If a matching panel is found, return a pointer to it.
  *
+ * The refcount of the returned panel is incremented. Use drm_panel_put()
+ * at the caller side to maintain the count.
+ *
  * Return: A pointer to the panel registered for the specified device tree
  * node or an ERR_PTR() if no panel matching the device tree node can be found.
  *
@@ -422,6 +425,7 @@ struct drm_panel *of_drm_find_panel(const struct device_node *np)
 	list_for_each_entry(panel, &panel_list, list) {
 		if (panel->dev->of_node == np) {
 			mutex_unlock(&panel_lock);
+			drm_panel_get(panel);
 			return panel;
 		}
 	}

-- 
2.48.1

[PATCH v2 03/16] drm/panel/samsung-dsicm: Keep up with refcounting

[Anusha Srivatsa]

Put the panel reference back when driver is no
longer using it. The panel that of_drm_find_panel()
returns is or the purpose of finding a out->bridge.
After which it is no longer used. Drop the refcount
accordingly.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
v2: Explain in commit message why the refcount is put in the
attach function which is not usual.
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 0014c497e3fe7d8349a119dbdda30d65d816cccf..3667855ff0d6d1b608c579573de657af7fd14388 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1748,6 +1748,7 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host,
 			dsi->out_bridge = ERR_PTR(-EINVAL);
 	}
 
+	drm_panel_put(panel);
 	of_node_put(remote);
 
 	if (IS_ERR(dsi->out_bridge)) {

-- 
2.48.1

[PATCH v2 04/16] drm/panel/sun4i: Keep up with refcounting

[Anusha Srivatsa]

Put the panel reference back when driver is no
longer using it.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
v2: none.
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index c35b70d83e53b7397c6e38dde45824bd49fdf099..36d47d7cedde1e1e3e83eb670520664100bd0a25 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -990,6 +990,8 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host,
 
 	drm_kms_helper_hotplug_event(dsi->drm);
 
+	drm_panel_put(dsi->panel);
+
 	return 0;
 }
 

-- 
2.48.1

[PATCH v2 05/16] drm/panel/exynos: Keep up with refcounting

[Anusha Srivatsa]

Put the panel reference back when driver is no
longer using it.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
v2: move drm_panel_put() inside the if()
---
 drivers/gpu/drm/exynos/exynos_drm_dpi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index 0dc36df6ada34cd0d4544f14e616fe3c52dafc73..3a258c4623b92a5a7c9aea3371d11a6c2df1cc8c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -137,6 +137,7 @@ static void exynos_dpi_disable(struct drm_encoder *encoder)
 	if (ctx->panel) {
 		drm_panel_disable(ctx->panel);
 		drm_panel_unprepare(ctx->panel);
+		drm_panel_put(ctx->panel);
 	}
 }
 

-- 
2.48.1

[PATCH v2 06/16] drm/panel/rcar_du_encoder: Keep up with refcounting

[Anusha Srivatsa]

The panel returned by of_drm_find_panel() is used to be
passed to devm_drm_panel_bridge_add_typed(). Put the
reference back accordingly.

This patch added in v2.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
index 7ecec7b04a8d0b0794bdd784fd0d7c7c5ba0a71c..b16cafc66e60d7f6c5167beb9e9d0606b17b0281 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
@@ -69,6 +69,8 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
 
 		bridge = devm_drm_panel_bridge_add_typed(rcdu->dev, panel,
 							 DRM_MODE_CONNECTOR_DPI);
+		drm_panel_put(panel);
+
 		if (IS_ERR(bridge))
 			return PTR_ERR(bridge);
 	} else {

-- 
2.48.1

[PATCH v2 07/16] drm/panel/rz-du: Keep up with refcounting

[Anusha Srivatsa]

The panel returned by of_drm_find_panel() is mainly
for the purpose of devm_drm_panel_bridge_add_typed().
Put the panel back accordingly.

This patch added in v2.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c
index 5e6dd16705e60a196f607819336ed41135fdec44..112415f948d3e7f3685d629b29107a269865c9c2 100644
--- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c
+++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c
@@ -83,6 +83,7 @@ int rzg2l_du_encoder_init(struct rzg2l_du_device  *rcdu,
 
 		bridge = devm_drm_panel_bridge_add_typed(rcdu->dev, panel,
 							 DRM_MODE_CONNECTOR_DPI);
+		drm_panel_put(panel);
 		if (IS_ERR(bridge))
 			return PTR_ERR(bridge);
 	} else {

-- 
2.48.1

[PATCH v2 08/16] drm/bridge/lvds-codec: Keep up with refcounting

[Anusha Srivatsa]

The panel returned by of_drm_find_panel() is for
calling devm_drm_panel_bridge_add_typed and not
anywhere after that.

Put the panel reference accordingly.

Patch added in v2.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/bridge/lvds-codec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/lvds-codec.c b/drivers/gpu/drm/bridge/lvds-codec.c
index e6a7147e141b64fc77dfef03a737ee599a0ecd10..72ac427e7b88b884ce07b52212639e518dbe4bec 100644
--- a/drivers/gpu/drm/bridge/lvds-codec.c
+++ b/drivers/gpu/drm/bridge/lvds-codec.c
@@ -154,6 +154,7 @@ static int lvds_codec_probe(struct platform_device *pdev)
 	lvds_codec->panel_bridge =
 		devm_drm_panel_bridge_add_typed(dev, panel,
 						lvds_codec->connector_type);
+	drm_panel_put(panel);
 	if (IS_ERR(lvds_codec->panel_bridge))
 		return PTR_ERR(lvds_codec->panel_bridge);
 

-- 
2.48.1

[PATCH v2 09/16] drm/bridge/fsl-ldb: Keep up with refcounting.

[Anusha Srivatsa]

The panel returned by of_drm_find_panel() is for
calling devm_drm_panel_bridge_add and not
anywhere after that.

Put the panel reference accordingly.

Patch added in v2.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/bridge/fsl-ldb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
index 5c3cf37200bcee1db285c97e2b463c9355ee6acb..ef0bbc272c1dc3484d0f7955233f4e8c6590235f 100644
--- a/drivers/gpu/drm/bridge/fsl-ldb.c
+++ b/drivers/gpu/drm/bridge/fsl-ldb.c
@@ -341,6 +341,9 @@ static int fsl_ldb_probe(struct platform_device *pdev)
 		return PTR_ERR(panel);
 
 	fsl_ldb->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
+
+	drm_panel_put(panel);
+
 	if (IS_ERR(fsl_ldb->panel_bridge))
 		return PTR_ERR(fsl_ldb->panel_bridge);
 

-- 
2.48.1

[PATCH v2 10/16] drm/panel/omapdrm: Keep up with refcounting

[Anusha Srivatsa]

Put the reference of the panel returned by
of_drm_find_panel().

Patch added in v2.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/omapdrm/dss/output.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/output.c b/drivers/gpu/drm/omapdrm/dss/output.c
index 7378e855c278c3809bc431ff48a1c5a41b7dedfc..259ef19d4be2cb559ba9ffe53db9e6e4b4409b21 100644
--- a/drivers/gpu/drm/omapdrm/dss/output.c
+++ b/drivers/gpu/drm/omapdrm/dss/output.c
@@ -48,7 +48,6 @@ int omapdss_device_init_output(struct omap_dss_device *out,
 			ret = PTR_ERR(bridge);
 			goto error;
 		}
-
 		out->bridge = bridge;
 	}
 
@@ -76,9 +75,12 @@ int omapdss_device_init_output(struct omap_dss_device *out,
 
 void omapdss_device_cleanup_output(struct omap_dss_device *out)
 {
-	if (out->bridge && out->panel)
+	if (out->bridge && out->panel) {
 		drm_panel_bridge_remove(out->next_bridge ?
 					out->next_bridge : out->bridge);
+		drm_panel_put(out->panel);
+
+	}
 }
 
 void dss_mgr_set_timings(struct omap_dss_device *dssdev,

-- 
2.48.1

[PATCH v2 11/16] drm/panel/sti: Keep up with refcounting

[Anusha Srivatsa]

Put the reference of the panel returned by
of_drm_find_panel() in the disable() function
after which panel won't be used.

Patch added in v2.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/sti/sti_dvo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
index 7484d3c3f4ed5fac7eab408e30cbe2f6b87f27e5..11684279320a0e4b15fe761bdf6782cc24e8be96 100644
--- a/drivers/gpu/drm/sti/sti_dvo.c
+++ b/drivers/gpu/drm/sti/sti_dvo.c
@@ -225,6 +225,8 @@ static void sti_dvo_disable(struct drm_bridge *bridge)
 
 	drm_panel_disable(dvo->panel);
 
+	drm_panel_put(dvo->panel);
+
 	/* Disable/unprepare dvo clock */
 	clk_disable_unprepare(dvo->clk_pix);
 	clk_disable_unprepare(dvo->clk);

-- 
2.48.1

[PATCH v2 12/16] drm/drm_of: Keep up with refcounting

[Anusha Srivatsa]

Put the panel reference back to keep up
with refcounting.

Patch added in v2.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/drm_of.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index d0183dea770308e77f05da364ffe087d53f3be36..2bd07de69526781d25236f480d029a7ee2ea483e 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -280,6 +280,8 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
 
 	}
 
+	drm_panel_put(*panel);
+
 	of_node_put(remote);
 	return ret;
 }

-- 
2.48.1

[PATCH v2 13/16] drm/fsl-dcu: Keep up with refcounting

[Anusha Srivatsa]

Put the panel reference back when driver is no
longer using it.

Patch added in v2

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
index 84eff7519e322257ed7ee7dfdcb08db40a341d6e..db79b35384be9729db9abb96a8fea6bc3da9d7f3 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
@@ -100,6 +100,7 @@ static int fsl_dcu_attach_panel(struct fsl_dcu_drm_device *fsl_dev,
 	if (ret < 0)
 		goto err_sysfs;
 
+	drm_panel_put(panel);
 	return 0;
 
 err_sysfs:

-- 
2.48.1

[PATCH v2 14/16] drm/mcde: Keep up with refcounting

[Anusha Srivatsa]

Put the panel reference returned by of_drm_find_panel()
back when driver is no longer using it.

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

diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
index a3423459dd7ac8395b77d0479a02ebb3a9ba259c..af9b149345adfd91ffdd620c016a1b6ea86cf209 100644
--- a/drivers/gpu/drm/mcde/mcde_dsi.c
+++ b/drivers/gpu/drm/mcde/mcde_dsi.c
@@ -1154,8 +1154,11 @@ static void mcde_dsi_unbind(struct device *dev, struct device *master,
 {
 	struct mcde_dsi *d = dev_get_drvdata(dev);
 
-	if (d->panel)
+	if (d->panel) {
 		drm_panel_bridge_remove(d->bridge_out);
+		drm_panel_put(d->panel);
+	}
+
 	regmap_update_bits(d->prcmu, PRCM_DSI_SW_RESET,
 			   PRCM_DSI_SW_RESET_DSI0_SW_RESETN, 0);
 }

-- 
2.48.1

[PATCH v2 15/16] drm/bridge/analogix: Keep up with refcounting

[Anusha Srivatsa]

The panel is looked for in exynos_dp_probe() which
calls analogix_dp_probe() after which the prepare(),
enable(), disable() and unprepare() helpers are called.
Put the reference after unprepare()

Patch added in v2.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 505eec6b819bb26c6984a0cda4013fa40e44b674..5fe7a846da410ee59d65b854a440a1b49c6f508f 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1230,6 +1230,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
 	pm_runtime_put_sync(dp->dev);
 
 	drm_panel_unprepare(dp->plat_data->panel);
+	drm_panel_put(dp->plat_data->panel);
 
 	dp->fast_train_enable = false;
 	dp->psr_supported = false;

-- 
2.48.1

[PATCH v2 16/16] drm/tegra: Keep up with refcounting

[Anusha Srivatsa]

Put the panel reference back when driver is no
longer using it.

Patch added in v2.

Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
 drivers/gpu/drm/tegra/dsi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index b5089b7722676cfaee5d28216af4ae706a47f895..830e90178302a76e7c091c92b66ce385ad26207e 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -1523,6 +1523,7 @@ static int tegra_dsi_host_detach(struct mipi_dsi_host *host,
 
 	if (output->panel && &device->dev == output->panel->dev) {
 		output->panel = NULL;
+		drm_panel_put(output->panel);
 
 		if (output->connector.dev)
 			drm_helper_hpd_irq_event(output->connector.dev);

-- 
2.48.1

Re: [PATCH v2 01/16] drm/panel: get/put panel reference in drm_panel_add/remove()

[Luca Ceresoli]

Hello Anusha,

On Thu, 19 Jun 2025 14:15:53 -0500
Anusha Srivatsa <asrivats@redhat.com> wrote:

> Take the panel reference and put it back as required.
> drm_panel_add() and drm_panel_remove() add a panel to
> the global registry and removes a panel respectively.
> Use get() and put() helpers to keep up with refcounting.
> 
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>

This patch is good.

I'd just point out that this must be applied only after all drivers
have been converted to the the _alloc API, otherwise with the following
sequence:

  panel = devm_kzalloc();
  drm_panel_init(panel);
  drm_panel_add(panel);
  ...
  drm_panel_remove(panel); <-----

at the drm_panel_remove() you'd have a warning:

  refcount_t: addition on 0; use-after-free.

So, if all panel drivers are converted:
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

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

Re: [PATCH v2 01/16] drm/panel: get/put panel reference in drm_panel_add/remove()

[Maxime Ripard]

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

On Fri, Jun 20, 2025 at 10:33:53AM +0200, Luca Ceresoli wrote:
> Hello Anusha,
> 
> On Thu, 19 Jun 2025 14:15:53 -0500
> Anusha Srivatsa <asrivats@redhat.com> wrote:
> 
> > Take the panel reference and put it back as required.
> > drm_panel_add() and drm_panel_remove() add a panel to
> > the global registry and removes a panel respectively.
> > Use get() and put() helpers to keep up with refcounting.
> > 
> > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
> 
> This patch is good.
> 
> I'd just point out that this must be applied only after all drivers
> have been converted to the the _alloc API, otherwise with the following
> sequence:
> 
>   panel = devm_kzalloc();
>   drm_panel_init(panel);
>   drm_panel_add(panel);
>   ...
>   drm_panel_remove(panel); <-----
> 
> at the drm_panel_remove() you'd have a warning:
> 
>   refcount_t: addition on 0; use-after-free.
> 
> So, if all panel drivers are converted:

Not all panels are yet:
$ rg -l drm_panel_init -- drivers/gpu/drm/panel/ | wc -l
20

Maxime

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

Re: [PATCH v2 04/16] drm/panel/sun4i: Keep up with refcounting

[Maxime Ripard]

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

Hi,

On Thu, Jun 19, 2025 at 02:15:56PM -0500, Anusha Srivatsa wrote:
> Put the panel reference back when driver is no
> longer using it.
> 
> Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>

When I asked you to provide a rationale for why you think the
drm_panel_put() call belonged where it does, it was pretty obvious it
needed to be done for all patches with the same issue, not just a few
random ones.

Maxime

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

Re: [PATCH v2 01/16] drm/panel: get/put panel reference in drm_panel_add/remove()

[Anusha Srivatsa]

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

On Fri, Jun 20, 2025 at 4:11 AM Maxime Ripard <mripard@kernel.org> wrote:

> On Fri, Jun 20, 2025 at 10:33:53AM +0200, Luca Ceresoli wrote:
> > Hello Anusha,
> >
> > On Thu, 19 Jun 2025 14:15:53 -0500
> > Anusha Srivatsa <asrivats@redhat.com> wrote:
> >
> > > Take the panel reference and put it back as required.
> > > drm_panel_add() and drm_panel_remove() add a panel to
> > > the global registry and removes a panel respectively.
> > > Use get() and put() helpers to keep up with refcounting.
> > >
> > > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > > Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
> >
> > This patch is good.
> >
> > I'd just point out that this must be applied only after all drivers
> > have been converted to the the _alloc API, otherwise with the following
> > sequence:
> >
> >   panel = devm_kzalloc();
> >   drm_panel_init(panel);
> >   drm_panel_add(panel);
> >   ...
> >   drm_panel_remove(panel); <-----
> >
> > at the drm_panel_remove() you'd have a warning:
> >
> >   refcount_t: addition on 0; use-after-free.
>
> Yes, Luca. makes sense. Thanks.
You had reviewed this patch already, but thanks :)

> So, if all panel drivers are converted:
>
> Not all panels are yet:
> $ rg -l drm_panel_init -- drivers/gpu/drm/panel/ | wc -l
> 20
>
^^^^^
 Yup. Looks like there are some more occurences. On it.

Anusha

> Maxime
>

[-- Attachment #2: Type: text/html, Size: 2491 bytes --]

Re: [PATCH v2 04/16] drm/panel/sun4i: Keep up with refcounting

[Anusha Srivatsa]

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

On Fri, Jun 20, 2025 at 8:27 AM Maxime Ripard <mripard@kernel.org> wrote:

> Hi,
>
> On Thu, Jun 19, 2025 at 02:15:56PM -0500, Anusha Srivatsa wrote:
> > Put the panel reference back when driver is no
> > longer using it.
> >
> > Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
>
> When I asked you to provide a rationale for why you think the
> drm_panel_put() call belonged where it does, it was pretty obvious it
> needed to be done for all patches with the same issue, not just a few
> random ones.
>
> Well, not totally random. THe intention was to specifically explain the
cases
where the drm_panel_put() is part of a probe(), enable() or attach() like
function
since that caused confusion in the last version. There must have been
misunderstanding
on my end when I thought only those cases needed additional explanation. I
will
make the commit message more verbose.

Thanks,
Anusha

Maxime
>

[-- Attachment #2: Type: text/html, Size: 1584 bytes --]

Re: [PATCH v2 04/16] drm/panel/sun4i: Keep up with refcounting

[Maxime Ripard]

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

On Fri, Jun 20, 2025 at 03:54:10PM -0500, Anusha Srivatsa wrote:
> On Fri, Jun 20, 2025 at 8:27 AM Maxime Ripard <mripard@kernel.org> wrote:
> 
> > Hi,
> >
> > On Thu, Jun 19, 2025 at 02:15:56PM -0500, Anusha Srivatsa wrote:
> > > Put the panel reference back when driver is no
> > > longer using it.
> > >
> > > Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
> >
> > When I asked you to provide a rationale for why you think the
> > drm_panel_put() call belonged where it does, it was pretty obvious it
> > needed to be done for all patches with the same issue, not just a few
> > random ones.
>
> Well, not totally random. THe intention was to specifically explain
> the cases where the drm_panel_put() is part of a probe(), enable() or
> attach() like function since that caused confusion in the last
> version. There must have been misunderstanding on my end when I
> thought only those cases needed additional explanation. I will make
> the commit message more verbose.

Eventually, it's a memory management thing. You can't judge how a
driver, framework or function uses the memory from 3 lines of context.

This patch is a good example: it might make sense from the 3 lines of
context. But if you consider what this function is doing, and how it
uses the drm_panel pointer, what you did is broken.

Maxime

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

Re: [PATCH v2 14/16] drm/mcde: Keep up with refcounting

[Linus Walleij]

On Thu, Jun 19, 2025 at 9:19 PM Anusha Srivatsa <asrivats@redhat.com> wrote:

> Put the panel reference returned by of_drm_find_panel()
> back when driver is no longer using it.
>
> Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij