Re: [PATCH] drm/bridge: nwl-dsi: Move bridge add/remove to dsi callbacks

On Wed, Oct 13, 2021 at 5:44 PM Guido Günther agx@sigxcpu.org 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@sigxcpu.org

This was prompted by the discussion at https://lore.kernel.org/dri-devel/00493cc61d1443dab1c131c46c5890f95f6f9b25.1...

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.