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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> > > 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
