[RFC 0/2] drm/bridge: panel and chaining

[Ajay kumar]

On Wednesday, April 30, 2014, Rob Clark <robdclark at gmail.com> wrote:

> On Wed, Apr 30, 2014 at 11:13 AM, Ajay kumar <ajaynumb at gmail.com<javascript:;>>
> wrote:
> >
> > On Wed, Apr 30, 2014 at 8:25 PM, AJAY KUMAR RAMAKRISHNA SHYMALAMMA <
> ajaykumar.rs at samsung.com <javascript:;>> wrote:
> >>
> >>
> >>
> >>
> >>
> >> ------- Original Message -------
> >>
> >> Sender : Sean Paul<seanpaul at chromium.org <javascript:;>>
> >>
> >> Date : Apr 30, 2014 02:34 (GMT+05:30)
> >>
> >> Title : Re: [RFC 0/2] drm/bridge: panel and chaining
> >>
> >>
> >>
> >> On Tue, Apr 29, 2014 at 3:57 PM, Rob Clark wrote:
> >> > So I thought it would be easier to explain what I had in mind
> regarding
> >> > Ajay's patchset (to add panel support) in patch form.  Originally
> Thierry
> >> > had some concerns with adding panel support in bridges ad-hoc.  So
> this
> >> > idea adds the support of chaining multiple bridges, one of which may
> be
> >> > a panal adapter (maybe I should have called it
> drm_panel_adapter_bridge).
> >> > There are a few rough edges and TODOs, it isn't trying to be complete
> >> > yet.
> >> >
> >> > So the one question is about that hunk I had to move in ptn3460 from
> >> > pre_enable() to enable().  If that really needs to come before the
> >> > encoder and after the panel, then we should go for a slightly
> different
> >> > approach instead: add a 'struct drm_bridge *next' ptr in 'struct
> >> > drm_bridge'.  Then each bridge implementation is responsible to call
> >> > the next in the chain (if not null) at the appropriate points.  That
> >> > gives a bit more flexibility to bridges to have something both pre and
> >> > post the downstream bridge/panel in each of the
> pre/enable/disable/post
> >> > steps.
> >>
> >> Arbitrarily chaining bridges seems like a more robust solution to me
> >> than the composite bridge.
> >>
> >> For the panel case, I wonder if we could change drm_bridge_init to
> >> accept a panel, then we could just chain the panel calls off the
> >> various places the bridge hooks are invoked in the drm layer.
> >
> >
> > This idea originated from Rob itself. He wanted to move out drm_panel
> calls
> > from both ptn3460 and ps8622 drivers and he wanted them at a common
> place.
> > But Daniel suggested that having a chain of bridges is good. That is how
> > composite_bridge was formed.
>
> so I'm thinking that, given what Sean and others have said, that the
> chaining inside bridge implementation would give more flexibility.  By
> which I mean:
>
>  struct drm_bridge {
> +    struct drm_bridge *next;    /* the next in the chain */
>       ....
>  };
>
> and then in each bridge implementation would do something like this
> for each fxn:
>
>  static void foo_bridge_pre_enable(...)
>  {
>       ... do stuff before ...
> +    if (bridge->next)
> +         bridge->next->pre_enable(...);
>      ... do stuff after ...
>  }
>
> it would mean now all bridge fxns are now required, even if they only
> call next in chain.. we can toss in some helpers for that.
>
> I don't think we would need new helpers or a struct bridge *next ptr.
This can be easily done with existing helpers itself.
drm_bridge_init anyhow adds onto a common list of bridges - "bridge_list".
We just need to stop calling bridge callbacks via
encoder->bridge->funcs->xyz() and instead parse the bridge_list to get each
of the bridge ptr in the list, and call their corresponding callbacks.
The order of bridge chain would be the order in which drm_bridge_init was
called.

> For dealing with panels, and this gets into Inki's proposal, I think
> we can just declare that panels themselves implement drm_bridge
> interface if needed.  So drm_panel_funcs is for extra API's needed by
> connector (like get_modes()) and everything else is part of
> drm_bridge_funcs.
>
> BR,
> -R
>
> > I still think we are addressing a very simple problem in a complex
> manner.
> > I tried testing this patchset on my board, with some tweaks(explained in
> PATCH 2/2]),
> > and I could get it working. This code basically adds 3 bridge structures
> to handle the calls,
> > but in actual hardware you can map them to only one bridge device.
> > I am still not sure what's the problem in just having the panel calls
> around
> > the bridge calls in drm core?
> >
> >>
> >> Feel free to ignore if this has already been explored on the other
> >> thread (which I haven't been following).
> >>
> >> Sean
> >>
> >>
> >>
> >> >
> >> >
> >> > Rob Clark (2):
> >> >   drm/bridge: add composite and panel bridges
> >> >   drm/bridge/ptn3460: add panel support
> >> >
> >> >  drivers/gpu/drm/bridge/Makefile          |   2 +
> >> >  drivers/gpu/drm/bridge/drm_bridge_util.c | 251
> +++++++++++++++++++++++++++++++
> >> >  drivers/gpu/drm/bridge/drm_bridge_util.h |  29 ++++
> >> >  drivers/gpu/drm/bridge/ptn3460.c         |  39 ++++-
> >> >  include/drm/bridge/ptn3460.h             |   6 +-
> >> >  5 files changed, 319 insertions(+), 8 deletions(-)
> >> >  create mode 100644 drivers/gpu/drm/bridge/drm_bridge_util.c
> >> >  create mode 100644 drivers/gpu/drm/bridge/drm_bridge_util.h
> >> >
> >> > --
> >> > 1.9.0
> >> >
> >> > _______________________________________________
> >> > dri-devel mailing list
> >> > dri-devel at lists.freedesktop.org <javascript:;>
> >> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>
> >>
> >>
> >>
> >>
> >>
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140501/df78f2fe/attachment-0001.html>