Re: [PATCH v2] of: Add videomode helper

[Sascha Hauer <s.hauer@pengutronix.de>]

On Thu, Jul 05, 2012 at 04:08:07PM +0200, Laurent Pinchart wrote:
> Hi Sascha,
> 
> Thanks for the patch.
> 
> > +++ b/Documentation/devicetree/bindings/video/displaymode
> > @@ -0,0 +1,40 @@
> > +videomode bindings
> > +=========
> > +
> > +Required properties:
> > + - xres, yres: Display resolution
> > + - left-margin, right-margin, hsync-len: Horizontal Display timing
> > parameters +   in pixels
> > +   upper-margin, lower-margin, vsync-len: Vertical display timing
> > parameters in +   lines
> > + - clock: displayclock in Hz
> > +
> > +Optional properties:
> > + - width-mm, height-mm: Display dimensions in mm
> 
> I've always had mixed feelings about the physical display dimension being part 
> of the display mode. Those are properties of the panel/display instead of the 
> mode. Storing them as part of the mode can be convenient, but we then run into 
> consistency issues (developers have to remember in which display mode 
> instances the values are available, and in which instances they're set to 0 
> for instance). If we want to clean this up, this patch would be a good 
> occasion.

This sounds like a display node with one or more node subnodes, like:

display {
	width_mm = <>;
	height_mm = <>;
	mode {
		xres = <>;
		yres = <>;
		...
	};
};

Is that what you mean or are you thinking of something else?

Sascha

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