This bridge driver calls drm_bridge_add() in the DSI host .attach callback
instead of in the probe function. This looks strange, even though
apparently not a problem for currently supported use cases.
However it is a problem for supporting hotplug of DRM bridges, which is in
the works [0][1][2][3]. The problematic case is when this DSI host is
always present while its DSI device is hot-pluggable. In such case with the
current code the DRM card will not be populated until after the DSI device
attaches to the host, and which could happen a very long time after
booting, or even not happen at all.
The reason is that the previous pipeline component (the encoder in this
case) when probing cannot find the samsung-dsim bridge. What happens is:
[1 and 2 can happen in any order, same result]
1) samsung-dsim probes (does not drm_bridge_add() itself)
2) The lcdif starts probing multiple times, but
lcdif_probe
-> lcdif_load
-> lcdif_attach_bridge
-> devm_drm_of_get_bridge() returns -EPROBE_DEFER because
the samsung-dsim is not in the global bridge_list
(deferred probe pending: imx-lcdif: Cannot connect bridge)
The samsung-dsim will not drm_bridge_add() itself until a DSI device will
try to mipi_dsi_attach() to the DSI Host, which can happen arbitratily late
or never on hot-pluggable hardware.
As a preliminary step to supporting hotplug move drm_bridge_add() at probe
time, so that the samsung-dsim DSI host bridge is available during boot,
even without a connected DSI device. This results in:
1) samsung-dsim probes (and adds to drm_bridge_add() itself)
2) The lcdif starts probing multiple times, but
lcdif_probe
-> lcdif_load
-> lcdif_attach_bridge
-> devm_drm_of_get_bridge() --> OK, returns samsung-dsim ptr
-> drm_bridge_attach()
-> samsung_dsim_attach()
-> drm_bridge_attach()
-> -EINVAL because dsi->bridge.next_bridge is still NULL
So moving drm_bridge_add() allows one step further but it is not
enough. The reason is:
* now the encoder driver finds this bridge instead of getting
-EPROBE_DEFER as before
* but it cannot attach it because the bridge attach function in turn tries
to attach to the following bridge, which has not yet been hot-plugged
Solve this by returning 0 in the bridge attach function in case the
following bridge (i.e. the DSI device) is not yet present. In other words,
for the samsung-dsim bridge it is OK to not have a following bridge. It can
be hotplugged later on.
[0] https://lpc.events/event/18/contributions/1750/
[1] https://www.youtube.com/watch?v=C8dEQ4OzMnc
[2] https://lore.kernel.org/lkml/20240924174254.711c7138@booty/
[3]
https://lore.kernel.org/lkml/20260507-drm-bridge-alloc-getput-panel_or_bridge-v5-0-472b913b5...@bootlin.com/
Signed-off-by: Luca Ceresoli <[email protected]>
---
This patch is similar to [4] but different in code and with a largely
rewritten commit message.
[4]
https://lore.kernel.org/lkml/20250725-drm-bridge-samsung-dsim-add-in-probe-v1-1-b23d29c23...@bootlin.com/
---
drivers/gpu/drm/bridge/samsung-dsim.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index dc3ff880d7ac..6c48404fd60a 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1827,6 +1827,9 @@ static int samsung_dsim_attach(struct drm_bridge *bridge,
{
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+ if (!dsi->bridge.next_bridge)
+ return 0;
+
return drm_bridge_attach(encoder, dsi->bridge.next_bridge, bridge,
flags);
}
@@ -1965,8 +1968,6 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host
*host,
mipi_dsi_pixel_format_to_bpp(device->format),
device->mode_flags);
- drm_bridge_add(&dsi->bridge);
-
/*
* This is a temporary solution and should be made by more generic way.
*
@@ -1976,7 +1977,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;
+ return ret;
}
// The next bridge can be used by host_ops->attach
@@ -1998,8 +1999,6 @@ 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:
- drm_bridge_remove(&dsi->bridge);
return ret;
}
@@ -2016,8 +2015,6 @@ static int samsung_dsim_host_detach(struct mipi_dsi_host
*host,
samsung_dsim_unregister_te_irq(dsi);
- drm_bridge_remove(&dsi->bridge);
-
return 0;
}
@@ -2216,6 +2213,8 @@ int samsung_dsim_probe(struct platform_device *pdev)
goto err_disable_runtime;
}
+ drm_bridge_add(&dsi->bridge);
+
return 0;
err_disable_runtime:
@@ -2229,6 +2228,8 @@ void samsung_dsim_remove(struct platform_device *pdev)
{
struct samsung_dsim *dsi = platform_get_drvdata(pdev);
+ drm_bridge_remove(&dsi->bridge);
+
pm_runtime_disable(&pdev->dev);
if (dsi->plat_data->host_ops &&
dsi->plat_data->host_ops->unregister_host)
--
2.55.0
