Re: [PATCH 1/4] drm/simple_kms_helper: enable use of external encoder

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Feb 07, 2019 at 10:02:17PM +0100, Linus Walleij wrote:
> On Thu, Feb 7, 2019 at 10:17 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Feb 07, 2019 at 09:36:44AM +0100, Linus Walleij wrote:
> 
> > > This makes it possible to pass a connector with an already
> > > attached external encoder into the simple KMS helper.
> > >
> > > This is helpful for my MCDE drivers, as it is pretty simple
> > > but uses DSI to communicate with the displays and bridges.
> > > DSI requires the use of the DSI bus which in turn requires
> > > us to set up a custom connector from the display driver.
> >
> > So the idea was that you'd just use a bridge for this, if you need more
> > than a dummy encoder. I'm a bit worried about mission creep for the simple
> > helpers, sooner or later we'll add extensions and then we're back to full
> > kms, except it's a hairball of classic midlayer mistake ...
> >
> > See drm_simple_display_pipe_attach_bridge().
> 
> Attaching bridges usually work fine for simple KMS drivers,
> like the panel bridge, or the dumb VGA bridge or even
> the HDMI encoder bridges so I'm on board with that.
> 
> The thing is that the DSI driver is using the panel bridge
> (and has a placeholder for using other bridges) already.
> So we end up wrapping the DSI host device(s) with a
> "my DSI bridge" using another
> bridge (panel) IIUC.
> 
> display driver -> custom DSI bridge -> panel bridge -> panel
> 
> The endpoint bridge (the panel) has all information on
> resolution etc. So this means we will need to just pass this
> information through to the next step in the pipe I
> suppose.

Hm, if you have you're mode_check functions working correctly (i.e. make
sure the mode userspace asks for is the one the panel ones), then the mode
will be in the crtc_state->requested_mode that the atomic helpers already
hand to all the bridges. Or well would, if bridge wouldn't have predated
atomic, so this is all a bit a more a mess than strictly needed. But you
should be getting the right mode already.

> I just want to confirm that I'm on the right track here before
> I code another thousand lines of code for this :)

bridges are supposed to be stackable, but I think you're the first to find
out whether that actually works. Afaiui the real stacking fun is if you
need to have some state between the bridges, because you e.g. have a
scaler or transcoder or something like that in-between.

Looking at your patch, converting the encoder into a bridge, and chaining
up the bridges in the right order, is all that should be needed really.
Not retyping the 1k lines of code that actually does stuff.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel