On Wednesday, April 30, 2014, Rob Clark <<a href="mailto:robdclark@gmail.com">robdclark@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Wed, Apr 30, 2014 at 11:13 AM, Ajay kumar <<a href="javascript:;" onclick="_e(event, 'cvml', 'ajaynumb@gmail.com')">ajaynumb@gmail.com</a>> wrote: > > On Wed, Apr 30, 2014 at 8:25 PM, AJAY KUMAR RAMAKRISHNA SHYMALAMMA <<a href="javascript:;" onclick="_e(event, 'cvml', 'ajaykumar.rs@samsung.com')">ajaykumar.rs@samsung.com</a>> wrote: >> >> >> >> >> >> ------- Original Message ------- >> >> Sender : Sean Paul<<a href="javascript:;" onclick="_e(event, 'cvml', 'seanpaul@chromium.org')">seanpaul@chromium.org</a>> >> >> 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. </blockquote><div>I don't think we would need new helpers or a struct bridge *next ptr.</div><div>This can be easily done with existing helpers itself.</div><div>drm_bridge_init anyhow adds onto a common list of bridges - "bridge_list".</div> <div>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.</div><div> The order of bridge chain would be the order in which drm_bridge_init was called.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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 >> > <a href="javascript:;" onclick="_e(event, 'cvml', 'dri-devel@lists.freedesktop.org')">dri-devel@lists.freedesktop.org</a> >> > <a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a> >> >> >> >> >> >> > > </blockquote>