On Thu, May 8, 2014 at 6:29 PM, Andrzej Hajda a.hajda@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@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:
- 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