On Sun, 24 Nov 2019 14:17:27 +0100
Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote:
> > > @@ -1687,16 +1705,18 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
> > > drm_encoder_init(drm_dev, encoder, &exynos_dsi_encoder_funcs,
> > > DRM_MODE_ENCODER_TMDS, NULL);
> > >
> > > - drm_encoder_helper_add(encoder, &exynos_dsi_encoder_helper_funcs);
> > > -
> > > ret = exynos_drm_set_possible_crtcs(encoder, EXYNOS_DISPLAY_TYPE_LCD);
> > > if (ret < 0)
> > > return ret;
> > >
> > > + /* Declare ourself as the first bridge element. */
> > > + dsi->bridge.funcs = &exynos_dsi_bridge_funcs;
> > > + drm_bridge_attach(encoder, &dsi->bridge, NULL);
> > > +
> > > if (dsi->in_bridge_node) {
> > > in_bridge = of_drm_find_bridge(dsi->in_bridge_node);
> > > if (in_bridge)
> > > - drm_bridge_attach(encoder, in_bridge, NULL);
> > > + drm_bridge_attach(encoder, in_bridge, &dsi->bridge);
> > > }
> >
> > Same as for patch 01/21, maybe this could be moved to this bridge's
> > attach operation ? Actually, now that I've read the code, this in_bridge
> > part looks weird. Why would the DSI encoder have an input bridge that is
> > has to manage itself ?
>
> Yes, I know, it doesn't make any sense. Either we're dealing with a
> bridge which can be chained to other bridges (can be placed in the
> middle of a chain as well), or we're dealing with an encoder which
> precedes any bridges. In the latter case (which is how exynos_dsi is
> implemented) in_bridge doesn't have any meaning, and that's even worse
> since we're placing the so-called input bridge (AKA previous bridge)
> after our encoder (that's what drm_bridge_attach(encoder, in_bridge,
> NULL) does).
More on that topic: I checked the exynos dts we have in mainline and
no one is making use of the ports part of the exynos_dsim bindings, so
maybe we should just deprecate it. The other option would be to patch
the driver to act as a real bridge, but I can't do that without someone
testing my changes and I didn't get much feedback from Exynos
maintainers so far...
