On Wed, Apr 30, 2014 at 11:13 AM, Ajay kumar <ajaynumb@xxxxxxxxx> wrote:
>
> On Wed, Apr 30, 2014 at 8:25 PM, AJAY KUMAR RAMAKRISHNA SHYMALAMMA <ajaykumar.rs@xxxxxxxxxxx> wrote:
>>
>>
>>
>>
>>
>> ------- Original Message -------
>>
>> Sender : Sean Paul<seanpaul@xxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxxxxxxxxx
>> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>>
>>
>>
>>
>>
>
>
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
