[PATCH 1/2] of: add helper to parse display specs

2012-10-03 Thread Steffen Trumtrar
On Mon, Oct 01, 2012 at 10:53:08AM -0600, Stephen Warren wrote:
> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
> > Parse a display-node with timings and hardware-specs from devictree.
> 
> > diff --git a/Documentation/devicetree/bindings/video/display 
> > b/Documentation/devicetree/bindings/video/display
> > new file mode 100644
> > index 000..722766a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/display
> 
> This should be display.txt.
> 
Okay

> > @@ -0,0 +1,208 @@
> > +display bindings
> > +==
> > +
> > +display-node
> > +
> 
> I'm not personally convinced about the direction this is going. While I
> think it's reasonable to define DT bindings for displays, and DT
> bindings for display modes, I'm not sure that it's reasonable to couple
> them together into a single binding.
> 
> I think creating a well-defined timing binding first will be much
> simpler than doing so within the context of a display binding; the
> scope/content of a general display binding seems much less well-defined
> to me at least, for reasons I mentioned before.
> 

Yes, you are right. I'm in the middle of moving things around a little.
It seems best, to have bindings only for the timings at the moment and
get people to agree on those and use them, instead of all the adhoc solutions
based on of_videomode v2.

Then, the of_get_display_timings and the conversion via videomode to 
fb_videomode
etc can be combined with Laurent Pincharts panel proposal.

> > +required properties:
> > + - none
> > +
> > +optional properties:
> > + - default-timing: the default timing value
> > + - width-mm, height-mm: Display dimensions in mm
> 
> > + - hsync-active-high (bool): Hsync pulse is active high
> > + - vsync-active-high (bool): Vsync pulse is active high
> 
> At least those two properties should exist in the display timing instead
> (or perhaps as well). There are certainly cases where different similar
> display modes are differentiated by hsync/vsync polarity more than
> anything else. This is probably more likely with analog display
> connectors than digital, but I see no reason why a DT binding for
> display timing shouldn't cover both.
> 

Yes. Will do.

> > + - de-active-high (bool): Data-Enable pulse is active high
> > + - pixelclk-inverted (bool): pixelclock is inverted
> 
> > + - pixel-per-clk
> 
> pixel-per-clk is probably something that should either be part of the
> timing definition, or something computed internally to the display
> driver based on rules for the signal type, rather than something
> represented in DT.
> 
> The above comment assumes this property is intended to represent DVI's
> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
> it's something to do with e.g. a single-data-rate vs. double-data-rate
> property of the underlying physical connection, that's most likely
> something that should be defined in a binding specific to e.g. LVDS,
> rather than something generic.
> 
> > + - link-width: number of channels (e.g. LVDS)
> > + - bpp: bits-per-pixel
> > +
> > +timings-subnode
> > +---
> > +
> > +required properties:
> > +subnodes that specify
> > + - hactive, vactive: Display resolution
> > + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing 
> > parameters
> > +   in pixels
> > +   vfront-porch, vback-porch, vsync-len: Vertical display timing 
> > parameters in
> > +   lines
> > + - clock: displayclock in Hz
> > +
> > +There are different ways of describing a display and its capabilities. The 
> > devicetree
> > +representation corresponds to the one commonly found in datasheets for 
> > displays.
> > +The description of the display and its timing is split in two parts: first 
> > the display
> > +properties like size in mm and (optionally) multiple subnodes with the 
> > supported timings.
> > +If a display supports multiple signal timings, the default-timing can be 
> > specified.
> > +
> > +Example:
> > +
> > +   display at 0 {
> > +   width-mm = <800>;
> > +   height-mm = <480>;
> > +   default-timing = <>;
> > +   timings {
> > +   timing0: timing at 0 {
> 
> If you're going to use a unit address ("@0") to ensure that node names
> are unique (which is not mandatory), then each node also needs a reg
> property with matching value, and #address-cells/#size-cells in the
> parent. Instead, you could name the nodes something unique based on the
> mode name to avoid this, e.g. 1080p24 { ... }.
> 

Ah, okay. Wasn't sure that was valid. I prefer to not use unit addresses.



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


Re: [PATCH 1/2] of: add helper to parse display specs

2012-10-03 Thread Steffen Trumtrar
On Mon, Oct 01, 2012 at 10:53:08AM -0600, Stephen Warren wrote:
 On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
  Parse a display-node with timings and hardware-specs from devictree.
 
  diff --git a/Documentation/devicetree/bindings/video/display 
  b/Documentation/devicetree/bindings/video/display
  new file mode 100644
  index 000..722766a
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/video/display
 
 This should be display.txt.
 
Okay

  @@ -0,0 +1,208 @@
  +display bindings
  +==
  +
  +display-node
  +
 
 I'm not personally convinced about the direction this is going. While I
 think it's reasonable to define DT bindings for displays, and DT
 bindings for display modes, I'm not sure that it's reasonable to couple
 them together into a single binding.
 
 I think creating a well-defined timing binding first will be much
 simpler than doing so within the context of a display binding; the
 scope/content of a general display binding seems much less well-defined
 to me at least, for reasons I mentioned before.
 

Yes, you are right. I'm in the middle of moving things around a little.
It seems best, to have bindings only for the timings at the moment and
get people to agree on those and use them, instead of all the adhoc solutions
based on of_videomode v2.

Then, the of_get_display_timings and the conversion via videomode to 
fb_videomode
etc can be combined with Laurent Pincharts panel proposal.

  +required properties:
  + - none
  +
  +optional properties:
  + - default-timing: the default timing value
  + - width-mm, height-mm: Display dimensions in mm
 
  + - hsync-active-high (bool): Hsync pulse is active high
  + - vsync-active-high (bool): Vsync pulse is active high
 
 At least those two properties should exist in the display timing instead
 (or perhaps as well). There are certainly cases where different similar
 display modes are differentiated by hsync/vsync polarity more than
 anything else. This is probably more likely with analog display
 connectors than digital, but I see no reason why a DT binding for
 display timing shouldn't cover both.
 

Yes. Will do.

  + - de-active-high (bool): Data-Enable pulse is active high
  + - pixelclk-inverted (bool): pixelclock is inverted
 
  + - pixel-per-clk
 
 pixel-per-clk is probably something that should either be part of the
 timing definition, or something computed internally to the display
 driver based on rules for the signal type, rather than something
 represented in DT.
 
 The above comment assumes this property is intended to represent DVI's
 requirement for pixel clock doubling for low-pixel-clock-rate modes. If
 it's something to do with e.g. a single-data-rate vs. double-data-rate
 property of the underlying physical connection, that's most likely
 something that should be defined in a binding specific to e.g. LVDS,
 rather than something generic.
 
  + - link-width: number of channels (e.g. LVDS)
  + - bpp: bits-per-pixel
  +
  +timings-subnode
  +---
  +
  +required properties:
  +subnodes that specify
  + - hactive, vactive: Display resolution
  + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing 
  parameters
  +   in pixels
  +   vfront-porch, vback-porch, vsync-len: Vertical display timing 
  parameters in
  +   lines
  + - clock: displayclock in Hz
  +
  +There are different ways of describing a display and its capabilities. The 
  devicetree
  +representation corresponds to the one commonly found in datasheets for 
  displays.
  +The description of the display and its timing is split in two parts: first 
  the display
  +properties like size in mm and (optionally) multiple subnodes with the 
  supported timings.
  +If a display supports multiple signal timings, the default-timing can be 
  specified.
  +
  +Example:
  +
  +   display@0 {
  +   width-mm = 800;
  +   height-mm = 480;
  +   default-timing = timing0;
  +   timings {
  +   timing0: timing@0 {
 
 If you're going to use a unit address (@0) to ensure that node names
 are unique (which is not mandatory), then each node also needs a reg
 property with matching value, and #address-cells/#size-cells in the
 parent. Instead, you could name the nodes something unique based on the
 mode name to avoid this, e.g. 1080p24 { ... }.
 

Ah, okay. Wasn't sure that was valid. I prefer to not use unit addresses.

 

-- 
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- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 1/2] of: add helper to parse display specs

2012-10-01 Thread Stephen Warren
On 10/01/2012 01:16 PM, Mitch Bradley wrote:
> On 10/1/2012 6:53 AM, Stephen Warren wrote:
>> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
>>> Parse a display-node with timings and hardware-specs from devictree.
>>
>>> diff --git a/Documentation/devicetree/bindings/video/display 
>>> b/Documentation/devicetree/bindings/video/display
>>> new file mode 100644
>>> index 000..722766a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/video/display
>>
>> This should be display.txt.
>>
>>> @@ -0,0 +1,208 @@
>>> +display bindings
>>> +==
>>> +
>>> +display-node
>>> +
>>
>> I'm not personally convinced about the direction this is going. While I
>> think it's reasonable to define DT bindings for displays, and DT
>> bindings for display modes, I'm not sure that it's reasonable to couple
>> them together into a single binding.
>>
>> I think creating a well-defined timing binding first will be much
>> simpler than doing so within the context of a display binding; the
>> scope/content of a general display binding seems much less well-defined
>> to me at least, for reasons I mentioned before.
>>
>>> +required properties:
>>> + - none
>>> +
>>> +optional properties:
>>> + - default-timing: the default timing value
>>> + - width-mm, height-mm: Display dimensions in mm
>>
>>> + - hsync-active-high (bool): Hsync pulse is active high
>>> + - vsync-active-high (bool): Vsync pulse is active high
>>
>> At least those two properties should exist in the display timing instead
>> (or perhaps as well). There are certainly cases where different similar
>> display modes are differentiated by hsync/vsync polarity more than
>> anything else. This is probably more likely with analog display
>> connectors than digital, but I see no reason why a DT binding for
>> display timing shouldn't cover both.
>>
>>> + - de-active-high (bool): Data-Enable pulse is active high
>>> + - pixelclk-inverted (bool): pixelclock is inverted
>>
>>> + - pixel-per-clk
>>
>> pixel-per-clk is probably something that should either be part of the
>> timing definition, or something computed internally to the display
>> driver based on rules for the signal type, rather than something
>> represented in DT.
>>
>> The above comment assumes this property is intended to represent DVI's
>> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
>> it's something to do with e.g. a single-data-rate vs. double-data-rate
>> property of the underlying physical connection, that's most likely
>> something that should be defined in a binding specific to e.g. LVDS,
>> rather than something generic.
>>
>>> + - link-width: number of channels (e.g. LVDS)
>>> + - bpp: bits-per-pixel
>>> +
>>> +timings-subnode
>>> +---
>>> +
>>> +required properties:
>>> +subnodes that specify
>>> + - hactive, vactive: Display resolution
>>> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing 
>>> parameters
>>> +   in pixels
>>> +   vfront-porch, vback-porch, vsync-len: Vertical display timing 
>>> parameters in
>>> +   lines
>>> + - clock: displayclock in Hz
>>> +
>>> +There are different ways of describing a display and its capabilities. The 
>>> devicetree
>>> +representation corresponds to the one commonly found in datasheets for 
>>> displays.
>>> +The description of the display and its timing is split in two parts: first 
>>> the display
>>> +properties like size in mm and (optionally) multiple subnodes with the 
>>> supported timings.
>>> +If a display supports multiple signal timings, the default-timing can be 
>>> specified.
>>> +
>>> +Example:
>>> +
>>> +   display at 0 {
>>> +   width-mm = <800>;
>>> +   height-mm = <480>;
>>> +   default-timing = <>;
>>> +   timings {
>>> +   timing0: timing at 0 {
>>
>> If you're going to use a unit address ("@0") to ensure that node names
>> are unique (which is not mandatory), then each node also needs a reg
>> property with matching value, and #address-cells/#size-cells in the
>> parent. Instead, you could name the nodes something unique based on the
>> mode name to avoid this, e.g. 1080p24 { ... }.
> 
> 
> I'm concerned that numbered nodes are being misused as arrays.
> 
> It's easy to make real arrays by including multiple cells in the value
> of each timing parameter, and easy to choose a cell by saying the array
> index instead of using the phandle.

In this case though, arrays don't work out so well in my opinion:

We want to describe a set of unrelated display modes that the display
can handle. These are logically separate entities. I don't think
combining the values that represent say 5 different modes into a single
set of properties really makes sense here; a separate node or property
per display mode really does make sense because they're separate objects.

Related, each display timing parameter (e.g. hsync length, position,
...) has a range, so min/typical/max values. These are already
represented as a 

[PATCH 1/2] of: add helper to parse display specs

2012-10-01 Thread Mitch Bradley
On 10/1/2012 10:25 AM, Stephen Warren wrote:
> On 10/01/2012 01:16 PM, Mitch Bradley wrote:
>> On 10/1/2012 6:53 AM, Stephen Warren wrote:
>>> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
 Parse a display-node with timings and hardware-specs from devictree.
>>>
 diff --git a/Documentation/devicetree/bindings/video/display 
 b/Documentation/devicetree/bindings/video/display
 new file mode 100644
 index 000..722766a
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/video/display
>>>
>>> This should be display.txt.
>>>
 @@ -0,0 +1,208 @@
 +display bindings
 +==
 +
 +display-node
 +
>>>
>>> I'm not personally convinced about the direction this is going. While I
>>> think it's reasonable to define DT bindings for displays, and DT
>>> bindings for display modes, I'm not sure that it's reasonable to couple
>>> them together into a single binding.
>>>
>>> I think creating a well-defined timing binding first will be much
>>> simpler than doing so within the context of a display binding; the
>>> scope/content of a general display binding seems much less well-defined
>>> to me at least, for reasons I mentioned before.
>>>
 +required properties:
 + - none
 +
 +optional properties:
 + - default-timing: the default timing value
 + - width-mm, height-mm: Display dimensions in mm
>>>
 + - hsync-active-high (bool): Hsync pulse is active high
 + - vsync-active-high (bool): Vsync pulse is active high
>>>
>>> At least those two properties should exist in the display timing instead
>>> (or perhaps as well). There are certainly cases where different similar
>>> display modes are differentiated by hsync/vsync polarity more than
>>> anything else. This is probably more likely with analog display
>>> connectors than digital, but I see no reason why a DT binding for
>>> display timing shouldn't cover both.
>>>
 + - de-active-high (bool): Data-Enable pulse is active high
 + - pixelclk-inverted (bool): pixelclock is inverted
>>>
 + - pixel-per-clk
>>>
>>> pixel-per-clk is probably something that should either be part of the
>>> timing definition, or something computed internally to the display
>>> driver based on rules for the signal type, rather than something
>>> represented in DT.
>>>
>>> The above comment assumes this property is intended to represent DVI's
>>> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
>>> it's something to do with e.g. a single-data-rate vs. double-data-rate
>>> property of the underlying physical connection, that's most likely
>>> something that should be defined in a binding specific to e.g. LVDS,
>>> rather than something generic.
>>>
 + - link-width: number of channels (e.g. LVDS)
 + - bpp: bits-per-pixel
 +
 +timings-subnode
 +---
 +
 +required properties:
 +subnodes that specify
 + - hactive, vactive: Display resolution
 + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing 
 parameters
 +   in pixels
 +   vfront-porch, vback-porch, vsync-len: Vertical display timing 
 parameters in
 +   lines
 + - clock: displayclock in Hz
 +
 +There are different ways of describing a display and its capabilities. 
 The devicetree
 +representation corresponds to the one commonly found in datasheets for 
 displays.
 +The description of the display and its timing is split in two parts: 
 first the display
 +properties like size in mm and (optionally) multiple subnodes with the 
 supported timings.
 +If a display supports multiple signal timings, the default-timing can be 
 specified.
 +
 +Example:
 +
 +  display at 0 {
 +  width-mm = <800>;
 +  height-mm = <480>;
 +  default-timing = <>;
 +  timings {
 +  timing0: timing at 0 {
>>>
>>> If you're going to use a unit address ("@0") to ensure that node names
>>> are unique (which is not mandatory), then each node also needs a reg
>>> property with matching value, and #address-cells/#size-cells in the
>>> parent. Instead, you could name the nodes something unique based on the
>>> mode name to avoid this, e.g. 1080p24 { ... }.
>>
>>
>> I'm concerned that numbered nodes are being misused as arrays.
>>
>> It's easy to make real arrays by including multiple cells in the value
>> of each timing parameter, and easy to choose a cell by saying the array
>> index instead of using the phandle.
> 
> In this case though, arrays don't work out so well in my opinion:
> 
> We want to describe a set of unrelated display modes that the display
> can handle. These are logically separate entities. I don't think
> combining the values that represent say 5 different modes into a single
> set of properties really makes sense here; a separate node or property
> per display mode really does make sense because they're 

[PATCH 1/2] of: add helper to parse display specs

2012-10-01 Thread Stephen Warren
On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
> Parse a display-node with timings and hardware-specs from devictree.

> diff --git a/Documentation/devicetree/bindings/video/display 
> b/Documentation/devicetree/bindings/video/display
> new file mode 100644
> index 000..722766a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/display

This should be display.txt.

> @@ -0,0 +1,208 @@
> +display bindings
> +==
> +
> +display-node
> +

I'm not personally convinced about the direction this is going. While I
think it's reasonable to define DT bindings for displays, and DT
bindings for display modes, I'm not sure that it's reasonable to couple
them together into a single binding.

I think creating a well-defined timing binding first will be much
simpler than doing so within the context of a display binding; the
scope/content of a general display binding seems much less well-defined
to me at least, for reasons I mentioned before.

> +required properties:
> + - none
> +
> +optional properties:
> + - default-timing: the default timing value
> + - width-mm, height-mm: Display dimensions in mm

> + - hsync-active-high (bool): Hsync pulse is active high
> + - vsync-active-high (bool): Vsync pulse is active high

At least those two properties should exist in the display timing instead
(or perhaps as well). There are certainly cases where different similar
display modes are differentiated by hsync/vsync polarity more than
anything else. This is probably more likely with analog display
connectors than digital, but I see no reason why a DT binding for
display timing shouldn't cover both.

> + - de-active-high (bool): Data-Enable pulse is active high
> + - pixelclk-inverted (bool): pixelclock is inverted

> + - pixel-per-clk

pixel-per-clk is probably something that should either be part of the
timing definition, or something computed internally to the display
driver based on rules for the signal type, rather than something
represented in DT.

The above comment assumes this property is intended to represent DVI's
requirement for pixel clock doubling for low-pixel-clock-rate modes. If
it's something to do with e.g. a single-data-rate vs. double-data-rate
property of the underlying physical connection, that's most likely
something that should be defined in a binding specific to e.g. LVDS,
rather than something generic.

> + - link-width: number of channels (e.g. LVDS)
> + - bpp: bits-per-pixel
> +
> +timings-subnode
> +---
> +
> +required properties:
> +subnodes that specify
> + - hactive, vactive: Display resolution
> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
> +   in pixels
> +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters 
> in
> +   lines
> + - clock: displayclock in Hz
> +
> +There are different ways of describing a display and its capabilities. The 
> devicetree
> +representation corresponds to the one commonly found in datasheets for 
> displays.
> +The description of the display and its timing is split in two parts: first 
> the display
> +properties like size in mm and (optionally) multiple subnodes with the 
> supported timings.
> +If a display supports multiple signal timings, the default-timing can be 
> specified.
> +
> +Example:
> +
> + display at 0 {
> + width-mm = <800>;
> + height-mm = <480>;
> + default-timing = <>;
> + timings {
> + timing0: timing at 0 {

If you're going to use a unit address ("@0") to ensure that node names
are unique (which is not mandatory), then each node also needs a reg
property with matching value, and #address-cells/#size-cells in the
parent. Instead, you could name the nodes something unique based on the
mode name to avoid this, e.g. 1080p24 { ... }.



[PATCH 1/2] of: add helper to parse display specs

2012-10-01 Thread Mitch Bradley
On 10/1/2012 6:53 AM, Stephen Warren wrote:
> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
>> Parse a display-node with timings and hardware-specs from devictree.
> 
>> diff --git a/Documentation/devicetree/bindings/video/display 
>> b/Documentation/devicetree/bindings/video/display
>> new file mode 100644
>> index 000..722766a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/video/display
> 
> This should be display.txt.
> 
>> @@ -0,0 +1,208 @@
>> +display bindings
>> +==
>> +
>> +display-node
>> +
> 
> I'm not personally convinced about the direction this is going. While I
> think it's reasonable to define DT bindings for displays, and DT
> bindings for display modes, I'm not sure that it's reasonable to couple
> them together into a single binding.
> 
> I think creating a well-defined timing binding first will be much
> simpler than doing so within the context of a display binding; the
> scope/content of a general display binding seems much less well-defined
> to me at least, for reasons I mentioned before.
> 
>> +required properties:
>> + - none
>> +
>> +optional properties:
>> + - default-timing: the default timing value
>> + - width-mm, height-mm: Display dimensions in mm
> 
>> + - hsync-active-high (bool): Hsync pulse is active high
>> + - vsync-active-high (bool): Vsync pulse is active high
> 
> At least those two properties should exist in the display timing instead
> (or perhaps as well). There are certainly cases where different similar
> display modes are differentiated by hsync/vsync polarity more than
> anything else. This is probably more likely with analog display
> connectors than digital, but I see no reason why a DT binding for
> display timing shouldn't cover both.
> 
>> + - de-active-high (bool): Data-Enable pulse is active high
>> + - pixelclk-inverted (bool): pixelclock is inverted
> 
>> + - pixel-per-clk
> 
> pixel-per-clk is probably something that should either be part of the
> timing definition, or something computed internally to the display
> driver based on rules for the signal type, rather than something
> represented in DT.
> 
> The above comment assumes this property is intended to represent DVI's
> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
> it's something to do with e.g. a single-data-rate vs. double-data-rate
> property of the underlying physical connection, that's most likely
> something that should be defined in a binding specific to e.g. LVDS,
> rather than something generic.
> 
>> + - link-width: number of channels (e.g. LVDS)
>> + - bpp: bits-per-pixel
>> +
>> +timings-subnode
>> +---
>> +
>> +required properties:
>> +subnodes that specify
>> + - hactive, vactive: Display resolution
>> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing 
>> parameters
>> +   in pixels
>> +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters 
>> in
>> +   lines
>> + - clock: displayclock in Hz
>> +
>> +There are different ways of describing a display and its capabilities. The 
>> devicetree
>> +representation corresponds to the one commonly found in datasheets for 
>> displays.
>> +The description of the display and its timing is split in two parts: first 
>> the display
>> +properties like size in mm and (optionally) multiple subnodes with the 
>> supported timings.
>> +If a display supports multiple signal timings, the default-timing can be 
>> specified.
>> +
>> +Example:
>> +
>> +display at 0 {
>> +width-mm = <800>;
>> +height-mm = <480>;
>> +default-timing = <>;
>> +timings {
>> +timing0: timing at 0 {
> 
> If you're going to use a unit address ("@0") to ensure that node names
> are unique (which is not mandatory), then each node also needs a reg
> property with matching value, and #address-cells/#size-cells in the
> parent. Instead, you could name the nodes something unique based on the
> mode name to avoid this, e.g. 1080p24 { ... }.


I'm concerned that numbered nodes are being misused as arrays.

It's easy to make real arrays by including multiple cells in the value
of each timing parameter, and easy to choose a cell by saying the array
index instead of using the phandle.



> 
> ___
> devicetree-discuss mailing list
> devicetree-discuss at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
> 


Re: [PATCH 1/2] of: add helper to parse display specs

2012-10-01 Thread Stephen Warren
On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
 Parse a display-node with timings and hardware-specs from devictree.

 diff --git a/Documentation/devicetree/bindings/video/display 
 b/Documentation/devicetree/bindings/video/display
 new file mode 100644
 index 000..722766a
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/video/display

This should be display.txt.

 @@ -0,0 +1,208 @@
 +display bindings
 +==
 +
 +display-node
 +

I'm not personally convinced about the direction this is going. While I
think it's reasonable to define DT bindings for displays, and DT
bindings for display modes, I'm not sure that it's reasonable to couple
them together into a single binding.

I think creating a well-defined timing binding first will be much
simpler than doing so within the context of a display binding; the
scope/content of a general display binding seems much less well-defined
to me at least, for reasons I mentioned before.

 +required properties:
 + - none
 +
 +optional properties:
 + - default-timing: the default timing value
 + - width-mm, height-mm: Display dimensions in mm

 + - hsync-active-high (bool): Hsync pulse is active high
 + - vsync-active-high (bool): Vsync pulse is active high

At least those two properties should exist in the display timing instead
(or perhaps as well). There are certainly cases where different similar
display modes are differentiated by hsync/vsync polarity more than
anything else. This is probably more likely with analog display
connectors than digital, but I see no reason why a DT binding for
display timing shouldn't cover both.

 + - de-active-high (bool): Data-Enable pulse is active high
 + - pixelclk-inverted (bool): pixelclock is inverted

 + - pixel-per-clk

pixel-per-clk is probably something that should either be part of the
timing definition, or something computed internally to the display
driver based on rules for the signal type, rather than something
represented in DT.

The above comment assumes this property is intended to represent DVI's
requirement for pixel clock doubling for low-pixel-clock-rate modes. If
it's something to do with e.g. a single-data-rate vs. double-data-rate
property of the underlying physical connection, that's most likely
something that should be defined in a binding specific to e.g. LVDS,
rather than something generic.

 + - link-width: number of channels (e.g. LVDS)
 + - bpp: bits-per-pixel
 +
 +timings-subnode
 +---
 +
 +required properties:
 +subnodes that specify
 + - hactive, vactive: Display resolution
 + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
 +   in pixels
 +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters 
 in
 +   lines
 + - clock: displayclock in Hz
 +
 +There are different ways of describing a display and its capabilities. The 
 devicetree
 +representation corresponds to the one commonly found in datasheets for 
 displays.
 +The description of the display and its timing is split in two parts: first 
 the display
 +properties like size in mm and (optionally) multiple subnodes with the 
 supported timings.
 +If a display supports multiple signal timings, the default-timing can be 
 specified.
 +
 +Example:
 +
 + display@0 {
 + width-mm = 800;
 + height-mm = 480;
 + default-timing = timing0;
 + timings {
 + timing0: timing@0 {

If you're going to use a unit address (@0) to ensure that node names
are unique (which is not mandatory), then each node also needs a reg
property with matching value, and #address-cells/#size-cells in the
parent. Instead, you could name the nodes something unique based on the
mode name to avoid this, e.g. 1080p24 { ... }.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 1/2] of: add helper to parse display specs

2012-09-24 Thread Steffen Trumtrar
Parse a display-node with timings and hardware-specs from devictree.

Signed-off-by: Steffen Trumtrar 
---
 Documentation/devicetree/bindings/video/display |  208 +++
 drivers/of/Kconfig  |5 +
 drivers/of/Makefile |1 +
 drivers/of/of_display.c |  157 +
 include/linux/display.h |   85 +
 include/linux/of_display.h  |   15 ++
 6 files changed, 471 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display
 create mode 100644 drivers/of/of_display.c
 create mode 100644 include/linux/display.h
 create mode 100644 include/linux/of_display.h

diff --git a/Documentation/devicetree/bindings/video/display 
b/Documentation/devicetree/bindings/video/display
new file mode 100644
index 000..722766a
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display
@@ -0,0 +1,208 @@
+display bindings
+==
+
+display-node
+
+
+required properties:
+ - none
+
+optional properties:
+ - default-timing: the default timing value
+ - width-mm, height-mm: Display dimensions in mm
+ - hsync-active-high (bool): Hsync pulse is active high
+ - vsync-active-high (bool): Vsync pulse is active high
+ - de-active-high (bool): Data-Enable pulse is active high
+ - pixelclk-inverted (bool): pixelclock is inverted
+ - pixel-per-clk
+ - link-width: number of channels (e.g. LVDS)
+ - bpp: bits-per-pixel
+
+timings-subnode
+---
+
+required properties:
+subnodes that specify
+ - hactive, vactive: Display resolution
+ - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
+   in pixels
+   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
+   lines
+ - clock: displayclock in Hz
+
+There are different ways of describing a display and its capabilities. The 
devicetree
+representation corresponds to the one commonly found in datasheets for 
displays.
+The description of the display and its timing is split in two parts: first the 
display
+properties like size in mm and (optionally) multiple subnodes with the 
supported timings.
+If a display supports multiple signal timings, the default-timing can be 
specified.
+
+Example:
+
+   display at 0 {
+   width-mm = <800>;
+   height-mm = <480>;
+   default-timing = <>;
+   timings {
+   timing0: timing at 0 {
+   /* 1920x1080p24 */
+   clock = <5200>;
+   hactive = <1920>;
+   vactive = <1080>;
+   hfront-porch = <25>;
+   hback-porch = <25>;
+   hsync-len = <25>;
+   vback-porch = <2>;
+   vfront-porch = <2>;
+   vsync-len = <2>;
+   hsync-active-high;
+   };
+   };
+   };
+
+Every property also supports the use of ranges, so the commonly used datasheet
+description with -tuples can be used.
+
+Example:
+
+   timing1: timing at 1 {
+   /* 1920x1080p24 */
+   clock = <14850>;
+   hactive = <1920>;
+   vactive = <1080>;
+   hsync-len = <0 44 60>;
+   hfront-porch = <80 88 95>;
+   hback-porch = <100 148 160>;
+   vfront-porch = <0 4 6>;
+   vback-porch = <0 36 50>;
+   vsync-len = <0 5 6>;
+   };
+
+The "display"-property in a connector-node (e.g. hdmi, ldb,...) is used to 
connect
+the display to that driver. 
+of_display expects a phandle, that specifies the display-node, in that 
property.
+
+Example:
+
+   hdmi at 0012 {
+   status = "okay";
+   display = <>;
+   };
+
+Usage in backend
+
+
+A backend driver may choose to use the display directly and convert the timing
+ranges to a suitable mode. Or it may just use the conversion of the display 
timings
+to the required mode via the generic videomode struct.
+
+   dtb
+|
+|  of_get_display
+?
+  struct display
+|
+|  videomode_from_timings
+?
+   ---  struct videomode ---
+   |   |
+ videomode_to_displaymode   |  |   
videomode_to_fb_videomode
+   ?   ?
+drm_display_mode fb_videomode
+
+
+Conversion to videomode

[PATCH 1/2] of: add helper to parse display specs

2012-09-24 Thread Steffen Trumtrar
Parse a display-node with timings and hardware-specs from devictree.

Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
 Documentation/devicetree/bindings/video/display |  208 +++
 drivers/of/Kconfig  |5 +
 drivers/of/Makefile |1 +
 drivers/of/of_display.c |  157 +
 include/linux/display.h |   85 +
 include/linux/of_display.h  |   15 ++
 6 files changed, 471 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display
 create mode 100644 drivers/of/of_display.c
 create mode 100644 include/linux/display.h
 create mode 100644 include/linux/of_display.h

diff --git a/Documentation/devicetree/bindings/video/display 
b/Documentation/devicetree/bindings/video/display
new file mode 100644
index 000..722766a
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display
@@ -0,0 +1,208 @@
+display bindings
+==
+
+display-node
+
+
+required properties:
+ - none
+
+optional properties:
+ - default-timing: the default timing value
+ - width-mm, height-mm: Display dimensions in mm
+ - hsync-active-high (bool): Hsync pulse is active high
+ - vsync-active-high (bool): Vsync pulse is active high
+ - de-active-high (bool): Data-Enable pulse is active high
+ - pixelclk-inverted (bool): pixelclock is inverted
+ - pixel-per-clk
+ - link-width: number of channels (e.g. LVDS)
+ - bpp: bits-per-pixel
+
+timings-subnode
+---
+
+required properties:
+subnodes that specify
+ - hactive, vactive: Display resolution
+ - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
+   in pixels
+   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
+   lines
+ - clock: displayclock in Hz
+
+There are different ways of describing a display and its capabilities. The 
devicetree
+representation corresponds to the one commonly found in datasheets for 
displays.
+The description of the display and its timing is split in two parts: first the 
display
+properties like size in mm and (optionally) multiple subnodes with the 
supported timings.
+If a display supports multiple signal timings, the default-timing can be 
specified.
+
+Example:
+
+   display@0 {
+   width-mm = 800;
+   height-mm = 480;
+   default-timing = timing0;
+   timings {
+   timing0: timing@0 {
+   /* 1920x1080p24 */
+   clock = 5200;
+   hactive = 1920;
+   vactive = 1080;
+   hfront-porch = 25;
+   hback-porch = 25;
+   hsync-len = 25;
+   vback-porch = 2;
+   vfront-porch = 2;
+   vsync-len = 2;
+   hsync-active-high;
+   };
+   };
+   };
+
+Every property also supports the use of ranges, so the commonly used datasheet
+description with min typ max-tuples can be used.
+
+Example:
+
+   timing1: timing@1 {
+   /* 1920x1080p24 */
+   clock = 14850;
+   hactive = 1920;
+   vactive = 1080;
+   hsync-len = 0 44 60;
+   hfront-porch = 80 88 95;
+   hback-porch = 100 148 160;
+   vfront-porch = 0 4 6;
+   vback-porch = 0 36 50;
+   vsync-len = 0 5 6;
+   };
+
+The display-property in a connector-node (e.g. hdmi, ldb,...) is used to 
connect
+the display to that driver. 
+of_display expects a phandle, that specifies the display-node, in that 
property.
+
+Example:
+
+   hdmi@0012 {
+   status = okay;
+   display = acme;
+   };
+
+Usage in backend
+
+
+A backend driver may choose to use the display directly and convert the timing
+ranges to a suitable mode. Or it may just use the conversion of the display 
timings
+to the required mode via the generic videomode struct.
+
+   dtb
+|
+|  of_get_display
+↓
+  struct display
+|
+|  videomode_from_timings
+↓
+   ---  struct videomode ---
+   |   |
+ videomode_to_displaymode   |  |   
videomode_to_fb_videomode
+   ↓   ↓
+drm_display_mode fb_videomode
+
+
+Conversion to videomode