On Tue, May 19, 2026 at 12:37:35PM +0200, Luca Ceresoli wrote:
> 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;
> + }This is a pretty big abstraction leak. We don't want to have that in everything driver. The removal path should be the same for both cases, and it's not something the driver should take care of. Maxime
signature.asc
Description: PGP signature
