Re: [PATCH v2] of: Add videomode helper

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sascha,

Sorry for the late reply.

On Thursday 05 July 2012 18:50:29 Sascha Hauer wrote:
> On Thu, Jul 05, 2012 at 04:08:07PM +0200, Laurent Pinchart wrote:
> > > +++ 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?

Yes, that's exactly what I meant.

-- 
Regards,

Laurent Pinchart