Re: [PATCH v2] of: Add videomode helper

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

On Thu, Jul 05, 2012 at 09:51:39AM -0500, Rob Herring wrote:
> On 07/04/2012 02:56 AM, Sascha Hauer wrote:
> > +
> > +There are different ways of describing a display mode. The devicetree representation
> > +corresponds to the one used by the Linux Framebuffer framework described here in
> > +Documentation/fb/framebuffer.txt. This representation has been chosen because it's
> > +the only format which does not allow for inconsistent parameters.Unlike the Framebuffer
> > +framework the devicetree has the clock in Hz instead of ps.
> 
> This implies you are putting linux settings into DT rather than
> describing the h/w. I'm not saying the binding is wrong, but documenting
> it this way makes it seem so.

The major reason to use these values was that they do not allow for
inconsistent values (as opposed to for example with hsync_start which you
would have to check for hsync_start >= xres).
I could rephrase this if it looks too much like modelled-after-Linux
instead of modelled-after-hardware.

> 
> One important piece missing (and IIRC linux doesn't really support) is
> defining the pixel format of the interface.

I could use this aswell. I think this can be specified as additional
properties later, right? I'm afraid this needs a lot of discussion so
we should delay this to the next round.

> 
> > +Example:
> > +
> > +	display@0 {
> > +		/* 1920x1080p24 */
> > +		clock = <52000000>;
> 
> Should this use the clock binding? You probably need both constraints
> and clock binding though.

Is the clock binding suitable for this? Here we are not interested where
the clock comes from, but instead which range is allowed.

> 
> Often you don't know the frequency up front and/or have limited control
> of the frequency (i.e. integer dividers). Then you have to adjust the
> margins to get the desired refresh rate. To do that, you need to know
> the ranges of values a panel can support. Perhaps you just assume you
> can increase the right-margin and lower-margins as I think you will hit
> pixel clock frequency max before any limit on margins.

Most datasheets specify min,typ,max triplets. We could do this instead
of using single fixed values for the margins:

	left_margin = <0 10 40>;

Right now we have nothing in the kernel that could handle this, but
getting the interface to the devicetree right seems indeed important.

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 |