[PATCH v2] drm: bridge: Allow daisy chaining of bridges

[Daniel Vetter]

On Fri, May 08, 2015 at 09:03:19AM -0400, Rob Clark wrote:
> On Fri, May 8, 2015 at 6:11 AM, Archit Taneja <architt at codeaurora.org> wrote:
> > Allow drm_bridge objects to link to each other in order to form an encoder
> > chain. The requirement for creating a chain of bridges comes because the
> > MSM drm driver uses up its encoder and bridge objects for blocks within
> > the SoC itself. There isn't anything left to use if the SoC display output
> > is connected to an external encoder IC. Having an additional bridge
> > connected to the existing bridge helps here. In general, it is possible for
> > platforms to have  multiple devices between the encoder and the
> > connector/panel that require some sort of configuration.
> >
> > We create drm bridge helper functions corresponding to each op in
> > 'drm_bridge_funcs'. These helpers call the corresponding
> > 'drm_bridge_funcs' op for the entire chain of bridges. These helpers are
> > used internally by drm_atomic_helper.c and drm_crtc_helper.c.
> >
> > The drm_bridge_enable/pre_enable helpers execute enable/pre_enable ops of
> > the bridge closet to the encoder, and proceed until the last bridge in the
> > chain is enabled. The same holds for drm_bridge_mode_set/mode_fixup
> > helpers. The drm_bridge_disable/post_disable helpers disable the last
> > bridge in the chain first, and proceed until the first bridge in the chain
> > is disabled.
> >
> > drm_bridge_attach() remains the same. As before, the driver calling this
> > function should make sure it has set the links correctly. The order in
> > which the bridges are connected to each other determines the order in which
> > the calls are made. One requirement is that every bridge in the chain
> > should point the parent encoder object. This is required since bridge
> > drivers expect a valid encoder pointer in drm_bridge. For example, consider
> > a chain where an encoder's output is connected to bridge1, and bridge1's
> > output is connected to bridge2:
> >
> >         /* Like before, attach bridge to an encoder */
> >         bridge1->encoder = encoder;
> >         ret = drm_bridge_attach(dev, bridge1);
> >         ..
> >
> >         /*
> >          * set the first bridge's 'next' bridge to bridge2, set its encoder
> >          * as bridge1's encoder
> >          */
> >         bridge1->next = bridge2
> >         bridge2->encoder = bridge1->encoder;
> >         ret = drm_bridge_attach(dev, bridge2);
> >
> >         ...
> >         ...
> >
> > This method of bridge chaining isn't intrusive and existing drivers that
> > use drm_bridge will behave the same way as before. The bridge helpers also
> > cleans up the atomic and crtc helper files a bit.
> >
> > Signed-off-by: Archit Taneja <architt at codeaurora.org>
> 
> Looks good. But I guess we probably should have some headerdoc for the
> new fxns, and somewhere a comment about not calling the bridge vfuncs
> directly but using the new helper funcs which handle the chaining.  I
> guess it isn't *as* critical as it would be if these were things
> intended to be called by drivers, rather than just from core, but all
> the same I think it would be a good idea.  With that,

Yeah, at least for patches that I'll take in through drm-misc I really
want kerneldoc. Also shouldnt' we do a cocci-run over all the drm drivers
to make sure they use the new helpers now?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch