On Fri, May 8, 2015 at 9:54 AM, Daniel Vetter <daniel@xxxxxxxx> wrote: > 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? cocci might have been the more clever way to do it, but I did have a quick check on the call-sites for bridge vfunc's with this patch applied and didn't see any calls from drivers or unconverted call sites in core.. BR, -R > -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
