[RFC V2 0/3] drm/bridge: panel and chaining

[Rob Clark]

On Thu, May 8, 2014 at 7:57 AM, Inki Dae <inki.dae at samsung.com> wrote:
> On 2014년 05월 08일 19:52, Ajay kumar wrote:
>> +Dave
>> +Thierry
>>
>> On Thu, May 8, 2014 at 1:14 PM, Inki Dae <inki.dae at samsung.com> wrote:
>>>
>>> Just re-sending with text mode. Sorry for this.
>>>
>>>
>>> On 2014년 05월 08일 15:41, Andrzej Hajda wrote:
>>>> On 05/05/2014 09:52 PM, Ajay Kumar wrote:
>>>>> This patchset is based on exynos-drm-next-todo branch of Inki Dae's tree at:
>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
>>>>>
>>>>> I have just put up Rob's and Sean's idea of chaining up the bridges
>>>>> in code, and have implemented basic panel controls as a chained bridge.
>>>>> This works well with ptn3460 bridge chip on exynos5250-snow board.
>>>>>
>>>>> Still need to make use of standard list calls and figure out proper way
>>>>> of deleting the bridge chain. So, this is just a rough version.
>>>>
>>>> As I understand this patchset tries to solve two things:
>>>> 1. Implement panel as drm_bridge, to ease support for hardware chains:
>>>> Crtc -> Encoder -> Bridge -> Panel
>>>> 2. Add support to drm_bridge chaining, to allow software chains:
>>>> drm_crtc -> drm_encoder -> drm_bridge -> drm_bridge,panel
>>>>
>>>> It is done using Russian doll schema, ops from the bridge calls the same
>>>> ops from the next bridge and the next bridge ops can do the same.
>>>>
>>>> This schema means that all the bridges including the last one are seen
>>>> from the drm core point of view as a one big drm_bridge. Additionally in
>>>> this particular case, the first bridge (ptn3460) implements connector
>>>> so it is hard to guess what is the location of the 2nd bridge in video
>>>> stream chain, sometimes it is after the connector, sometimes before.
>>>> All this is quite confusing.
>>>>
>>>> But if you look at the bridge from upstream video interface point of
>>>> view it is just a panel, edp panel in case of ptn3460, ie ptn3460 on its
>>>> video input side acts as a panel. On the output side it expects a panel,
>>>> lvds panel in this case.
>>>>
>>>> So why not implement ptn3460 bridge as drm_panel which internally uses
>>>> another drm_panel. With this approach everything fits much better.
>>>> You do not need those (pre|post)_(enable|disable) calls, you do not need
>>>> to implement connector in the bridge and you have a driver following
>>>> linux driver model. And no single bit changed in drm core.
>>>>
>>>> I have implemented this way DSI/LVDS bridge, it was sent as RFC [1][2].
>>>> It was not accepted as Inki preferred drm_bridge but as I see the
>>>
>>> Yes, in above email threads, I disagreed to using drm_panel framework
>>> for bridge device, especially, to implement connector/encoder to crtc
>>> driver.
>>>
>>> However, I thought that it'd be more generic way that lvds drivers use
>>> driver-model, and the use of drm_panel infrastructure would be suitable
>>> to doing this.
>>>
>>> So my intention in turn, was that LVDS driver uses integrated drm_bridge
>>> based on drm_panel infrastructure[1], and RFC patch[2] for it. This way
>>> is same as your proposal in the point that LVDS and Panel drivers use
>>> driver-model. The only different point is that LVDS driver has some ops
>>> specific to LVDS device, not using existing ops of drm_panel commonly:
>>> we may need to consider the characteristic of LVDS device.
>>>
>>> [1]:http://www.spinics.net/lists/dri-devel/msg55555.html
>>> [2]:http://www.spinics.net/lists/dri-devel/msg55658.html
>>>
>>> Thanks,
>>> Inki Dae
>> I am just consolidating the discussion happening here.
>> 1) This patchset started from a discussion wherein I tried to combine
>> drm_panel with drm_bridge.
>>     https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg28943.html
>> 2) Sean and Rob suggested to implement a chain of bridges and then
>> consider adding
>>     basic panel controls as a bridge.
>> 3) Andrej's idea is to drop the existing bridge infrastructure and
>> implement ptn3460 using drm_panel,
>>     the same way he has implemented
>> http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559.
>> 4) Inki's suggestion is to combine drm_bridge, drm_panel and
>> drm_enhance into a single
>>     drm_hw_block.
>>
>
> And more thing, we would need to consider image enhancer device placed
> between crtc and connector/encoder devices. And it'd better to rename
> drm_hw_block to drm_bridge, and existing drm_bridge relevant codes
> should be removed.

I don't object to changing the name to hw_block or something else.
Although to avoid introducing too much confusion in this discussion,
for now I am calling it bridge/drm_bridge_funcs/etc for now ;-)

BR,
-R

> Thanks,
> Inki Dae
>
>> I am currently trying to implement (2):chaining of bridges, and I
>> think we have not yet
>> reached to a consensus. So adding few other people from drm community
>> to comment.
>>
>> Regards,
>> Ajay
>>
>>>> problems with drm_bridges I have decide to attract attention to much
>>>> more cleaner solution.
>>>>
>>>> [1]: http://permalink.gmane.org/gmane.linux.drivers.devicetree/61559
>>>> [2]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/27044
>>>>
>>>> Regards
>>>> Andrzej
>>>>
>>>>
>>>>>
>>>>> Ajay Kumar (3):
>>>>>   [RFC V2 1/3] drm: implement chaining of drm bridges
>>>>>   [RFC V2 2/3] drm/bridge: add a dummy panel driver to support lvds bridges
>>>>>   [RFC V2 3/3] drm/bridge: ptn3460: support bridge chaining
>>>>>
>>>>>  .../bindings/drm/bridge/bridge_panel.txt           |  45 ++++
>>>>>  drivers/gpu/drm/bridge/Kconfig                     |   6 +
>>>>>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>>>>>  drivers/gpu/drm/bridge/bridge_panel.c              | 240 +++++++++++++++++++++
>>>>>  drivers/gpu/drm/bridge/ptn3460.c                   |  21 +-
>>>>>  drivers/gpu/drm/drm_crtc.c                         |  13 +-
>>>>>  include/drm/bridge/bridge_panel.h                  |  37 ++++
>>>>>  include/drm/drm_crtc.h                             |   2 +
>>>>>  8 files changed, 360 insertions(+), 5 deletions(-)
>>>>>  create mode 100644 Documentation/devicetree/bindings/drm/bridge/bridge_panel.txt
>>>>>  create mode 100644 drivers/gpu/drm/bridge/bridge_panel.c
>>>>>  create mode 100644 include/drm/bridge/bridge_panel.h
>>>>>
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>>> the body of a message to majordomo at vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>
>