Re: [PATCH 3/3] ARM: dts: Add display timing node to exynos5250-arndale.dts

Thanks, we test and resend v2 patchset.


On 14 May 2013 18:55, Steffen Trumtrar [off-list ref] wrote:

Hi!

On Tue, May 14, 2013 at 06:40:00PM +0530, Vikas Sajjan wrote:

quoted

Hi Steffen Trumtrar and Sascha Hauer,

Olof and Tomasz Figa have a concern in putting "display-timings"  as root
node in device tree file.


On 10 May 2013 13:45, Vikas Sajjan [off-list ref] wrote:

quoted

HI Tomasz,


On 10 May 2013 13:03, Tomasz Figa [off-list ref] wrote:

quoted

On Friday 10 of May 2013 11:57:34 Vikas Sajjan wrote:

quoted

Hi Olof,

On 10 May 2013 11:51, Vikas Sajjan [off-list ref] wrote:

quoted

Hi Olof,

On 10 May 2013 11:08, Olof Johansson [off-list ref] wrote:

quoted

On Thu, May 9, 2013 at 7:55 PM, Vikas Sajjan
[off-list ref]

wrote:

quoted

Hi Tomasz,

On 10 May 2013 05:35, Tomasz Figa [off-list ref]

wrote:

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

Hi Vikas,

On Wednesday 08 of May 2013 11:31:34 Vikas Sajjan wrote:

quoted

Adds display timing node for a DP panel to Arndale Board DTS
file

Signed-off-by: Vikas Sajjan <redacted>
---

 arch/arm/boot/dts/exynos5250-arndale.dts |   16
 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts

b/arch/arm/boot/dts/exynos5250-arndale.dts index
f68b820..c831a5c
100644

--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts

@@ -459,4 +459,20 @@

              samsung,lane-count = <4>;

      };

+     display-timings {
+             native-mode = <&timing0>;
+             timing0: timing@0 {
+                     /* 2560x1600 DP panel */
+                     clock-frequency = <50000>;
+                     hactive = <2560>;
+                     vactive = <1600>;
+                     hfront-porch = <48>;
+                     hback-porch = <80>;
+                     hsync-len = <32>;
+                     vback-porch = <16>;
+                     vfront-porch = <8>;
+                     vsync-len = <6>;
+             };
+     };

What display are those timings used for? Shouldn't they be

placed

quoted

quoted

quoted

quoted

quoted

quoted

inside

quoted

quoted

node of that display?

This timing information will be used parsed by the FIMD probe

with

quoted

quoted

quoted

quoted

quoted

quoted

quoted

the

help

quoted

of "Video Helper function"  as done in this link

https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/c

quoted

quoted

quoted

ommit/?h=exynos-drm-next&id=7f4596f4aebcc9fcf2d50d3fe398508d710c4dd0

quoted

quoted

quoted

quoted

quoted

quoted

That function passes in a device node pointer to the FIMD device
node,
and only looks for a display-timings node under there. Tomasz is
right, as far as I can tell.

Placing display timings at the root of the device tree seems
completely
wrong.

        I referred this "

Documentation/devicetree/bindings/video/display-timing.txt" for

example

quoted

quoted

quoted

quoted

            here it dosen't mention about whether display timings

node

quoted

quoted

quoted

quoted

can be placed at the root of the device tree or NOT.
 Not sure how we should be placing the node then.

I think this is pretty obvious.

Imagine a bit more complex case: a board has two video outputs (let's

say

quoted

quoted

quoted

FIMD0 and FIMD1), each connected to a completely different display.

This

quoted

quoted

quoted

means that each display needs different timings.

How would you know which timings to use if all of them were be placed
under the root node?

    so what do you think, how  should we modify this patch so that it

belongs to a specific video outputs.

  So what do you think, how should we modify this patch so that it

belongs

quoted

to a specific video outputs.

 Something like

        &fimd {
                display-timings {
                        timing0: timing@0 {
                                /* 2560x1600 DP panel */
                                clock-frequency = <50000>;
                                hactive = <2560>;
                                vactive = <1600>;
                                hfront-porch = <48>;
                                (...)
                        };
                };
        };

Would be correct, if fimd is the output on which the display is connected.
The fimd driver than has to utilize of_get_display_timings,
of_get_fb_videomode
or something like that.

Regards,
Steffen

--
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

-- 
Thanks and Regards
 Vikas Sajjan