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

[Ajay kumar]

On Thu, May 8, 2014 at 6:29 PM, Andrzej Hajda <a.hajda at samsung.com> wrote:
> On 05/08/2014 12:26 PM, Ajay kumar wrote:
>> Hi Andrej,
>>
>> On Thu, May 8, 2014 at 12:11 PM, Andrzej Hajda <a.hajda at samsung.com> 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.
>> This discussion should have ideally happened when Sean added bridge
>> into drm-core!
>
> Yes, I agree with this :)
>
>> And, we do need (pre|post)_(enable|disable) calls for bridges, but the drm_panel
>> framework supports only enable/disable.
>
> For true bridges pre|post can be just implemented as a part of the call,
> for example:
>
> bridge_enable()
> {
>     /* pre-enable stuff */
>     panel_enable();
>     /* post-enable stuff */
> }
>
It should ideally be like this:
1) panel enable - switch on the LCD unit.
2) bridge enable - switch on the bridge.
3) encoder->commit/dpms on - valid data starts coming out of DP/MIPI
4) switch on the backlight unit and enable BL_EN.

> And for your particular problem you have written:
>> The LVDS datasheet says at least 200ms delay is needed from "Valid
>> data" to "BL on".
>
> I am not sure what exactly means 'valid data' in this case, if it is
> after lcd_en gpio
> why not just use schedule_delayed_work. If it should be called later I
> guess it should
> be possible to find a proper callback to drm_panel.
Right. This was already discussed.
I suggested adding pre_enable/enable/disable and post_disable for drm_panel,
but Thierry was not ok with it.

Regards,
Ajay


>>> 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
>>> 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
>>>>
>