Thanks, we test and resend v2 patchset. On 14 May 2013 18:55, Steffen Trumtrar wrote: > Hi! > > On Tue, May 14, 2013 at 06:40:00PM +0530, Vikas Sajjan wrote: > > 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 wrote: > > > > > HI Tomasz, > > > > > > > > > On 10 May 2013 13:03, Tomasz Figa wrote: > > > > > >> On Friday 10 of May 2013 11:57:34 Vikas Sajjan wrote: > > >> > Hi Olof, > > >> > > > >> > On 10 May 2013 11:51, Vikas Sajjan wrote: > > >> > > Hi Olof, > > >> > > > > >> > > On 10 May 2013 11:08, Olof Johansson wrote: > > >> > >> On Thu, May 9, 2013 at 7:55 PM, Vikas Sajjan > > >> > >> > > >> > >> > > >> > >> wrote: > > >> > >> > Hi Tomasz, > > >> > >> > > > >> > >> > On 10 May 2013 05:35, Tomasz Figa > wrote: > > >> > >> >> Hi Vikas, > > >> > >> >> > > >> > >> >> On Wednesday 08 of May 2013 11:31:34 Vikas Sajjan wrote: > > >> > >> >> > Adds display timing node for a DP panel to Arndale Board DTS > > >> > >> >> > file > > >> > >> >> > > > >> > >> >> > Signed-off-by: Vikas Sajjan > > >> > >> >> > --- > > >> > >> >> > > > >> > >> >> > 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 > > >> > >> > > >> > >> inside > > >> > >> > > >> > >> >> node of that display? > > >> > >> > > > >> > >> > This timing information will be used parsed by the FIMD probe > with > > >> > >> > the > > >> > >> > > >> > >> help > > >> > >> > > >> > >> > of "Video Helper function" as done in this link > > >> > >> > > >> > >> > > >> https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/c > > >> > >> > ommit/?h=exynos-drm-next&id=7f4596f4aebcc9fcf2d50d3fe398508d710c4dd0 > > >> > >> > > >> > >> 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 > > >> > > > >> > here it dosen't mention about whether display timings > node > > >> > 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 > > >> FIMD0 and FIMD1), each connected to a completely different display. > This > > >> 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 > > 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