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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 05/08/2015 08:00 PM, Rob Clark wrote:
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..

I'll repost with headerdoc for the new functions.

Thanks,
Archit

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux