Hi Adrian, Thank you for the patch. On Mon, Apr 27, 2020 at 11:19:46AM +0300, Adrian Ratiu wrote: > Up until now the assumption was that the synopsis dsi bridge will > directly connect to an encoder provided by the platform driver, but > the current practice for drivers is to leave the encoder empty via > the simple encoder API and add their logic to their own drm_bridge. > > Thus we need an ablility to connect the DSI bridge to another bridge > provided by the platform driver, so we extend the dw_mipi_dsi bind() > API with a new "previous bridge" arg instead of just hardcoding NULL. > > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Signed-off-by: Adrian Ratiu <adrian.ratiu@xxxxxxxxxxxxx> > --- > New in v8. > --- > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 6 ++++-- > drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 2 +- > include/drm/bridge/dw_mipi_dsi.h | 5 ++++- > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > index 16fd87055e7b7..140ff40fa1b62 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > @@ -1456,11 +1456,13 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove); > /* > * Bind/unbind API, used from platforms based on the component framework. > */ > -int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct drm_encoder *encoder) > +int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, > + struct drm_encoder *encoder, > + struct drm_bridge *prev_bridge) > { > int ret; > > - ret = drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); > + ret = drm_bridge_attach(encoder, &dsi->bridge, prev_bridge, 0); Please note that chaining of bridges doesn't work well if multiple bridges in the chain try to create a connector. This is why a DRM_BRIDGE_ATTACH_NO_CONNECTOR flag has been added, with a helper to create a connector for a chain of bridges (drm_bridge_connector_init()). This won't play well with the component framework. I would recommend using the of_drm_find_bridge() instead in the rockchip driver, and deprecating dw_mipi_dsi_bind(). > if (ret) { > DRM_ERROR("Failed to initialize bridge with drm\n"); > return ret; > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > index 3feff0c45b3f7..83ef43be78135 100644 > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > @@ -929,7 +929,7 @@ static int dw_mipi_dsi_rockchip_bind(struct device *dev, > return ret; > } > > - ret = dw_mipi_dsi_bind(dsi->dmd, &dsi->encoder); > + ret = dw_mipi_dsi_bind(dsi->dmd, &dsi->encoder, NULL); > if (ret) { > DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret); > return ret; > diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h > index b0e390b3288e8..699b3531f5b36 100644 > --- a/include/drm/bridge/dw_mipi_dsi.h > +++ b/include/drm/bridge/dw_mipi_dsi.h > @@ -14,6 +14,7 @@ > #include <drm/drm_modes.h> > > struct drm_display_mode; > +struct drm_bridge; > struct drm_encoder; > struct dw_mipi_dsi; > struct mipi_dsi_device; > @@ -62,7 +63,9 @@ struct dw_mipi_dsi *dw_mipi_dsi_probe(struct platform_device *pdev, > const struct dw_mipi_dsi_plat_data > *plat_data); > void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi); > -int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct drm_encoder *encoder); > +int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, > + struct drm_encoder *encoder, > + struct drm_bridge *prev_bridge); > void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi); > void dw_mipi_dsi_set_slave(struct dw_mipi_dsi *dsi, struct dw_mipi_dsi *slave); > -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel