<div dir="ltr">Thanks, we test and resend v2 patchset.<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On 14 May 2013 18:55, Steffen Trumtrar <span dir="ltr"><<a href="mailto:s.trumtrar@pengutronix.de" target="_blank">s.trumtrar@pengutronix.de</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi!<br>
<div><div class="h5"><br>
On Tue, May 14, 2013 at 06:40:00PM +0530, Vikas Sajjan wrote:<br>
> Hi Steffen Trumtrar and Sascha Hauer,<br>
><br>
> Olof and Tomasz Figa have a concern in putting "display-timings"  as root<br>
> node in device tree file.<br>
><br>
><br>
> On 10 May 2013 13:45, Vikas Sajjan <<a href="mailto:vikas.sajjan@linaro.org">vikas.sajjan@linaro.org</a>> wrote:<br>
><br>
> > HI Tomasz,<br>
> ><br>
> ><br>
> > On 10 May 2013 13:03, Tomasz Figa <<a href="mailto:tomasz.figa@gmail.com">tomasz.figa@gmail.com</a>> wrote:<br>
> ><br>
> >> On Friday 10 of May 2013 11:57:34 Vikas Sajjan wrote:<br>
> >> > Hi Olof,<br>
> >> ><br>
> >> > On 10 May 2013 11:51, Vikas Sajjan <<a href="mailto:vikas.sajjan@linaro.org">vikas.sajjan@linaro.org</a>> wrote:<br>
> >> > > Hi Olof,<br>
> >> > ><br>
> >> > > On 10 May 2013 11:08, Olof Johansson <<a href="mailto:olof@lixom.net">olof@lixom.net</a>> wrote:<br>
> >> > >> On Thu, May 9, 2013 at 7:55 PM, Vikas Sajjan<br>
> >> > >> <<a href="mailto:vikas.sajjan@linaro.org">vikas.sajjan@linaro.org</a>><br>
> >> > >><br>
> >> > >> wrote:<br>
> >> > >> > Hi Tomasz,<br>
> >> > >> ><br>
> >> > >> > On 10 May 2013 05:35, Tomasz Figa <<a href="mailto:tomasz.figa@gmail.com">tomasz.figa@gmail.com</a>> wrote:<br>
> >> > >> >> Hi Vikas,<br>
> >> > >> >><br>
> >> > >> >> On Wednesday 08 of May 2013 11:31:34 Vikas Sajjan wrote:<br>
> >> > >> >> > Adds display timing node for a DP panel to Arndale Board DTS<br>
> >> > >> >> > file<br>
> >> > >> >> ><br>
> >> > >> >> > Signed-off-by: Vikas Sajjan <<a href="mailto:vikas.sajjan@linaro.org">vikas.sajjan@linaro.org</a>><br>
> >> > >> >> > ---<br>
> >> > >> >> ><br>
> >> > >> >> >  arch/arm/boot/dts/exynos5250-arndale.dts |   16<br>
> >> > >> >> >  ++++++++++++++++<br>
> >> > >> >> >  1 file changed, 16 insertions(+)<br>
> >> > >> >> ><br>
> >> > >> >> > diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts<br>
> >> > >> >> > b/arch/arm/boot/dts/exynos5250-arndale.dts index<br>
> >> > >> >> > f68b820..c831a5c<br>
> >> > >> >> > 100644<br>
> >> > >> >> > --- a/arch/arm/boot/dts/exynos5250-arndale.dts<br>
> >> > >> >> > +++ b/arch/arm/boot/dts/exynos5250-arndale.dts<br>
> >> > >> >> > @@ -459,4 +459,20 @@<br>
> >> > >> >> ><br>
> >> > >> >> >               samsung,lane-count = <4>;<br>
> >> > >> >> ><br>
> >> > >> >> >       };<br>
> >> > >> >> ><br>
> >> > >> >> > +     display-timings {<br>
> >> > >> >> > +             native-mode = <&timing0>;<br>
> >> > >> >> > +             timing0: timing@0 {<br>
> >> > >> >> > +                     /* 2560x1600 DP panel */<br>
> >> > >> >> > +                     clock-frequency = <50000>;<br>
> >> > >> >> > +                     hactive = <2560>;<br>
> >> > >> >> > +                     vactive = <1600>;<br>
> >> > >> >> > +                     hfront-porch = <48>;<br>
> >> > >> >> > +                     hback-porch = <80>;<br>
> >> > >> >> > +                     hsync-len = <32>;<br>
> >> > >> >> > +                     vback-porch = <16>;<br>
> >> > >> >> > +                     vfront-porch = <8>;<br>
> >> > >> >> > +                     vsync-len = <6>;<br>
> >> > >> >> > +             };<br>
> >> > >> >> > +     };<br>
> >> > >> >><br>
> >> > >> >> What display are those timings used for? Shouldn't they be placed<br>
> >> > >><br>
> >> > >> inside<br>
> >> > >><br>
> >> > >> >> node of that display?<br>
> >> > >> ><br>
> >> > >> > This timing information will be used parsed by the FIMD probe with<br>
> >> > >> > the<br>
> >> > >><br>
> >> > >> help<br>
> >> > >><br>
> >> > >> > of "Video Helper function"  as done in this link<br>
> >> > >><br>
> >> > >><br>
> >> <a href="https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/c" target="_blank">https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/c</a><br>
> >> > >> ommit/?h=exynos-drm-next&id=7f4596f4aebcc9fcf2d50d3fe398508d710c4dd0<br>
> >> > >><br>
> >> > >> That function passes in a device node pointer to the FIMD device<br>
> >> > >> node,<br>
> >> > >> and only looks for a display-timings node under there. Tomasz is<br>
> >> > >> right, as far as I can tell.<br>
> >> > >><br>
> >> > >> Placing display timings at the root of the device tree seems<br>
> >> > >> completely<br>
> >> > >> wrong.<br>
> >> > >><br>
> >> > >>         I referred this "<br>
> >> ><br>
> >> > Documentation/devicetree/bindings/video/display-timing.txt" for example<br>
> >> ><br>
> >> >             here it dosen't mention about whether display timings node<br>
> >> > can be placed at the root of the device tree or NOT.<br>
> >> >  Not sure how we should be placing the node then.<br>
> >><br>
> >> I think this is pretty obvious.<br>
> >><br>
> >> Imagine a bit more complex case: a board has two video outputs (let's say<br>
> >> FIMD0 and FIMD1), each connected to a completely different display. This<br>
> >> means that each display needs different timings.<br>
> >><br>
> >> How would you know which timings to use if all of them were be placed<br>
> >> under the root node?<br>
> >><br>
> >>     so what do you think, how  should we modify this patch so that it<br>
> > belongs to a specific video outputs.<br>
> ><br>
> ><br>
><br>
>   So what do you think, how should we modify this patch so that it belongs<br>
> to a specific video outputs.<br>
><br>
</div></div> Something like<br>
<br>
        &fimd {<br>
                display-timings {<br>
                        timing0: timing@0 {<br>
<div class="im">                                /* 2560x1600 DP panel */<br>
</div>                                clock-frequency = <50000>;<br>
                                hactive = <2560>;<br>
                                vactive = <1600>;<br>
                                hfront-porch = <48>;<br>
                                (...)<br>
                        };<br>
                };<br>
        };<br>
<br>
Would be correct, if fimd is the output on which the display is connected.<br>
The fimd driver than has to utilize of_get_display_timings, of_get_fb_videomode<br>
or something like that.<br>
<br>
Regards,<br>
Steffen<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Pengutronix e.K.                           |                             |<br>
Industrial Linux Solutions                 | <a href="http://www.pengutronix.de/" target="_blank">http://www.pengutronix.de/</a>  |<br>
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |<br>
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |<br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br>Thanks and Regards<div> Vikas Sajjan</div>
</div>