On Wed, Oct 13, 2021 at 5:44 PM Guido Günther <agx@xxxxxxxxxxx> wrote:
>
> Move the panel and bridge_{add,remove} from the bridge callbacks to the
> DSI callbacks to make sure we don't indicate readiness to participate in
> the display pipeline before the panel is attached.
>
> This was prompted by commit fb8d617f8fd6 ("drm/bridge: Centralize error
> message when bridge attach fails") which triggered
>
> [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@30800000/mipi-dsi@30a0 0000 to encoder None-34: -517
>
> during boot.
>
> Signed-off-by: Guido Günther <agx@xxxxxxxxxxx>
> ---
> This was prompted by the discussion at
> https://lore.kernel.org/dri-devel/00493cc61d1443dab1c131c46c5890f95f6f9b25.1634068657.git.agx@xxxxxxxxxxx/
>
> drivers/gpu/drm/bridge/nwl-dsi.c | 64 ++++++++++++++++++--------------
> 1 file changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c
> index a7389a0facfb..77aa6f13afef 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -355,6 +355,9 @@ static int nwl_dsi_host_attach(struct mipi_dsi_host *dsi_host,
> {
> struct nwl_dsi *dsi = container_of(dsi_host, struct nwl_dsi, dsi_host);
> struct device *dev = dsi->dev;
> + struct drm_bridge *panel_bridge;
> + struct drm_panel *panel;
> + int ret;
>
> DRM_DEV_INFO(dev, "lanes=%u, format=0x%x flags=0x%lx\n", device->lanes,
> device->format, device->mode_flags);
> @@ -362,10 +365,43 @@ static int nwl_dsi_host_attach(struct mipi_dsi_host *dsi_host,
> if (device->lanes < 1 || device->lanes > 4)
> return -EINVAL;
>
> + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> + &panel_bridge);
> + if (ret)
> + return ret;
> +
> + if (panel) {
> + panel_bridge = drm_panel_bridge_add(panel);
> + if (IS_ERR(panel_bridge))
> + return PTR_ERR(panel_bridge);
> + }
> + if (!panel_bridge)
> + return -EPROBE_DEFER;
> +
> + dsi->panel_bridge = panel_bridge;
> dsi->lanes = device->lanes;
> dsi->format = device->format;
> dsi->dsi_mode_flags = device->mode_flags;
>
> + /*
> + * The DSI output has been properly configured, we can now safely
> + * register the input to the bridge framework so that it can take place
> + * in a display pipeline.
> + */
> + drm_bridge_add(&dsi->bridge);
> +
> + return 0;
> +}
> +
> +static int nwl_dsi_host_detach(struct mipi_dsi_host *dsi_host,
> + struct mipi_dsi_device *dev)
> +{
> + struct nwl_dsi *dsi = container_of(dsi_host, struct nwl_dsi, dsi_host);
> +
> + drm_bridge_remove(&dsi->bridge);
> + if (dsi->panel_bridge)
> + drm_panel_bridge_remove(dsi->panel_bridge);
> +
If I'm correct this logic will failed to find the direct and I2C based
bridges. As these peripheral bridges are trying to find the bridge
device from bridge_attach unlike DSI panels are trying to find the
panel via host attach directly from probe.
Similar issue we have encounters with dw-mipi-dsi bridge.
c206c7faeb3263a7cc7b4de443a3877cd7a5e74b
Jagan.
