Hi Enric, On Thu, Apr 16, 2020 at 11:33:24PM +0200, Enric Balletbo i Serra wrote: > On 16/4/20 19:36, Laurent Pinchart wrote: > > On Thu, Apr 16, 2020 at 08:35:26PM +0300, Laurent Pinchart wrote: > >> On Thu, Apr 16, 2020 at 05:57:19PM +0200, Enric Balletbo i Serra wrote: > >>> Use the drm_bridge_connector helper to create a connector for pipelines > >>> that use drm_bridge. This allows splitting connector operations across > >>> multiple bridges when necessary, instead of having the last bridge in > >>> the chain creating the connector and handling all connector operations > >>> internally. > >> > >> That's the right direction, but this should be done in the mtk display > >> controller driver core, not in here. I'm OK with the code being here as > >> an interim measure if needed to move forward, but that should then be > >> temporary only. > > It'd be nice if we can do this as an interim measure for now, so at least we > have the embedded display working. IIUC to move that to the display controller > driver core I should also convert/rework the mtk_dpi and mtk_hdmi drivers. This > is used for the external display on my device but to fully support this I'll > also need to rework the bridge chain logic to handle the multi-sink/multi-source > use case. This is something I plan to work on but I suspect won't be easy and > will trigger lots of discussions, and, of course, some time. > > So, if is fine I won't move this for now. That's totally fine with me, I just wanted to make sure you were aware that more work was needed :-) Thanks for all your efforts ! > > I forgot to mention that the drm_encoder should also move out of the > > bridge driver to the display controller driver. > > > >>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx> > >>> --- > >>> > >>> Changes in v2: None > >>> > >>> drivers/gpu/drm/mediatek/mtk_dsi.c | 14 +++++++++++++- > >>> 1 file changed, 13 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > >>> index 44718fa3d1ca..2f8876c32864 100644 > >>> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > >>> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > >>> @@ -17,6 +17,7 @@ > >>> > >>> #include <drm/drm_atomic_helper.h> > >>> #include <drm/drm_bridge.h> > >>> +#include <drm/drm_bridge_connector.h> > >>> #include <drm/drm_mipi_dsi.h> > >>> #include <drm/drm_of.h> > >>> #include <drm/drm_panel.h> > >>> @@ -184,6 +185,7 @@ struct mtk_dsi { > >>> struct drm_bridge bridge; > >>> struct drm_bridge *panel_bridge; > >>> struct drm_bridge *next_bridge; > >>> + struct drm_connector *connector; > >>> struct phy *phy; > >>> > >>> void __iomem *regs; > >>> @@ -983,10 +985,19 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi) > >>> */ > >>> dsi->encoder.possible_crtcs = 1; > >>> > >>> - ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, 0); > >>> + ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, > >>> + DRM_BRIDGE_ATTACH_NO_CONNECTOR); > >>> if (ret) > >>> goto err_cleanup_encoder; > >>> > >>> + dsi->connector = drm_bridge_connector_init(drm, &dsi->encoder); > >>> + if (IS_ERR(dsi->connector)) { > >>> + DRM_ERROR("Unable to create bridge connector\n"); > >>> + ret = PTR_ERR(dsi->connector); > >>> + goto err_cleanup_encoder; > >>> + } > >>> + drm_connector_attach_encoder(dsi->connector, &dsi->encoder); > >>> + > >>> return 0; > >>> > >>> err_cleanup_encoder: > >>> @@ -1144,6 +1155,7 @@ static int mtk_dsi_probe(struct platform_device *pdev) > >>> > >>> dsi->bridge.funcs = &mtk_dsi_bridge_funcs; > >>> dsi->bridge.of_node = dev->of_node; > >>> + dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; > >> > >> I think this line belongs to the patch that adds drm_bridge support to > >> this driver. > >> > >>> > >>> drm_bridge_add(&dsi->bridge); > >>> -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel