On Wed, Apr 30, 2014 at 1:46 PM, Sean Paul <seanpaul@xxxxxxxxxxxx> wrote:
> On Wed, Apr 30, 2014 at 11:38 AM, Rob Clark <robdclark@xxxxxxxxx> wrote:
>> 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.
>>
>> 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.
>>
>
> So if we do this, we can add panels off the end (or wherever) of the
> chain transparently, masquerading as bridges? That sounds like a
> pretty good solution to me.
yup, that is pretty much what I'm thinking. Some difference in
implementation details, but I think it covers what Inki wants too
BR,
-R
> Sean
>
>
>> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
