In preparation for DRM bridge hot-plugging, we need to handle the dynamic
lifetime of the following bridge in case the samsung-dsim is always present
and the following bridge (next_bridge) is hot-unplugged.

Based on the 'if (!IS_ERR(panel))' check in samsung_dsim_host_attach(), the
next_bridge could be A) a panel bridge created by this driver via
devm_drm_panel_bridge_add() or B) a pre-existing bridge obtained via
of_drm_find_and_get_bridge().

For case B) we need to put that reference when the next_bridge is removed,
which is already handled by calling drm_bridge_clear_and_put() in
samsung_dsim_host_detach() and in the samsung_dsim_host_attach() error
management code.

In case A) we additionally have to remove the panel bridge. Currently it is
created by devm_drm_panel_bridge_add(), which adds two devm actions with
the refcounted panel bridge:

 - drm_bridge_put_void() via devm_drm_bridge_alloc() on panel->dev
 - devm_drm_panel_bridge_release() via devm_drm_panel_bridge_add_typed()
   on the consumer device (samsung-dsim)

The first action is OK: being tied to panel->dev it will happen when the
panel is unplugged.

The second action is bound to the consumer device, so the devm semantics is
not useful here when introducing hotplug. Indeed we need to drop the
next_bridge in samsung_dsim_host_detach() anyway, before the driver .remove
function, in order to support {add, {attach, detach} x N, remove} hotplug
event sequences.

Thus move to the non-devm drm_panel_bridge_add() along with the matching
drm_panel_bridge_remove(), so the lifetime of the panel-bridge is tied to
the host_attach/host_detach cycle and not the whole samsung-dsim device
lifetime.

Signed-off-by: Luca Ceresoli <[email protected]>

---

In a previous discussion with Maxime he mentioned a plan to make every
drm_panel always have a wrapping bridge. With that done, all the code
handling the panel and adding the panel_bridge would become useless here
(and in many other places) and could be entirely removed. This patch is a
temporary solution until that happens. The best pointer I could find to
that discussion is [0], but there might be more recent material I could not
find at the moment.

[0] https://lore.kernel.org/lkml/20250218-faithful-white-magpie-da9ac9@houat/
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 17 ++++++++++++++---
 include/drm/bridge/samsung-dsim.h     |  2 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 5b799619e07e..2af287221e22 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1951,14 +1951,16 @@ static int samsung_dsim_host_attach(struct 
mipi_dsi_host *host,
        if (!remote)
                return -ENODEV;
 
+       dsi->panel_bridge_added = false;
        panel = of_drm_find_panel(remote);
        if (!IS_ERR(panel)) {
-               next_bridge = devm_drm_panel_bridge_add(dev, panel);
+               next_bridge = drm_panel_bridge_add(panel);
                if (IS_ERR(next_bridge)) {
                        ret = PTR_ERR(next_bridge);
                        next_bridge = NULL; // Inhibit the cleanup action on an 
ERR_PTR
                } else {
                        drm_bridge_get(next_bridge);
+                       dsi->panel_bridge_added = true;
                }
        } else {
                next_bridge = of_drm_find_and_get_bridge(remote);
@@ -1989,7 +1991,7 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host 
*host,
        if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO)) {
                ret = samsung_dsim_register_te_irq(dsi, &device->dev);
                if (ret)
-                       goto err_remove_bridge;
+                       goto err_remove_panel_bridge;
        }
 
        // The next bridge can be used by host_ops->attach
@@ -2011,8 +2013,12 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host 
*host,
        drm_bridge_clear_and_put(&dsi->bridge.next_bridge);
        if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO))
                samsung_dsim_unregister_te_irq(dsi);
-err_remove_bridge:
+err_remove_panel_bridge:
        drm_bridge_remove(&dsi->bridge);
+       if (dsi->panel_bridge_added) {
+               drm_panel_bridge_remove(next_bridge);
+               dsi->panel_bridge_added = false;
+       }
        return ret;
 }
 
@@ -2022,6 +2028,11 @@ static int samsung_dsim_host_detach(struct mipi_dsi_host 
*host,
        struct samsung_dsim *dsi = host_to_dsi(host);
        const struct samsung_dsim_plat_data *pdata = dsi->plat_data;
 
+       if (dsi->panel_bridge_added) {
+               drm_panel_bridge_remove(dsi->bridge.next_bridge);
+               dsi->panel_bridge_added = false;
+       }
+
        if (pdata->host_ops && pdata->host_ops->detach)
                pdata->host_ops->detach(dsi, device);
 
diff --git a/include/drm/bridge/samsung-dsim.h 
b/include/drm/bridge/samsung-dsim.h
index 03005e474704..3620f7aa9394 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -131,6 +131,8 @@ struct samsung_dsim {
        const struct samsung_dsim_plat_data *plat_data;
 
        void *priv;
+
+       bool panel_bridge_added; /* true iff next_bridge is a panel_bridge we 
created */
 };
 
 extern int samsung_dsim_probe(struct platform_device *pdev);

-- 
2.54.0

Reply via email to