[PATCH v4] of: Add videomode helper
Hi Rob,
On Monday 24 September 2012 08:42:12 Rob Herring wrote:
> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
> > This patch adds a helper function for parsing videomodes from the
> > devicetree. The videomode can be either converted to a struct
> > drm_display_mode or a struct fb_videomode.
> >
> > Signed-off-by: Sascha Hauer
> > Signed-off-by: Steffen Trumtrar
> > ---
> >
> > Hi!
> >
> > changes since v3:
> > - print error messages
> > - free alloced memory
> > - general cleanup
> >
> > Regards
> > Steffen
> >
> > .../devicetree/bindings/video/displaymode | 74 +
> > drivers/of/Kconfig |5 +
> > drivers/of/Makefile|1 +
> > drivers/of/of_videomode.c | 283
> > include/linux/of_videomode.h
> > | 56
> > 5 files changed, 419 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/video/displaymode
> > create mode 100644 drivers/of/of_videomode.c
> > create mode 100644 include/linux/of_videomode.h
> >
> > diff --git a/Documentation/devicetree/bindings/video/displaymode
> > b/Documentation/devicetree/bindings/video/displaymode new file mode
> > 100644
> > index 000..990ca52
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/displaymode
> > @@ -0,0 +1,74 @@
> > +videomode bindings
> > +==
> > +
> > +Required properties:
> > + - 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
>
> A major piece missing is the LCD controller to display interface width
> and component ordering.
>
> > +
> > +Optional properties:
> > + - 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
> > + - interlaced (bool): This is an interlaced mode
> > + - doublescan (bool): This is a doublescan mode
> > +
> > +There are different ways of describing a display mode. The devicetree
> > representation
> > +corresponds to the one commonly found in datasheets for displays.
> > +The description of the display and its mode is split in two parts: first
> > the display
> > +properties like size in mm and (optionally) multiple subnodes with the
> > supported modes.
> > +
> > +Example:
> > +
> > + display at 0 {
>
> It would be useful to have a compatible string here. We may not always
> know the panel type or have a fixed panel though. We could define
> "generic-lcd" or something for cases where the panel type is unknown.
I'm working on a generic panel framework (see
http://lwn.net/Articles/512363/). DT bindings are not there yet, but they're
certainly a hot topic. A compatible string here will definitely be needed
here.
The exact properties required in the display node will likely be panel-
dependent. width-mm, height-mm and modes sound like a good baseline to me.
> > + width-mm = <800>;
> > + height-mm = <480>;
> > + modes {
> > + mode0: mode 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:
> > +
> > + mode1: mode 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 videomode can be linked to a connector via phandles. The connector
> > has to
> > +support the display- and default-mode-property to link to and select a
> > mode.
>
> Could also be phandle in the lcd controller node? What are the '-' for?
> Is "display-blah" a valid name or something?
>
> "default-mode" is pretty generic. How about display-mode or
> display-default-mode?
>
> Rob
>
> > +
> > +Example:
> > +
> > + hdmi at 0012 {
> > + status = "okay";
> > +
[PATCH v4] of: Add videomode helper
On Mon, Sep 24, 2012 at 05:09:30PM -0600, Stephen Warren wrote:
> On 09/24/2012 12:26 PM, Rob Herring wrote:
> > On 09/24/2012 10:45 AM, Stephen Warren wrote:
> >> On 09/24/2012 07:42 AM, Rob Herring wrote:
> >>> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
> This patch adds a helper function for parsing videomodes from the
> devicetree.
> The videomode can be either converted to a struct drm_display_mode or a
> struct fb_videomode.
> >>
> +++ b/Documentation/devicetree/bindings/video/displaymode
> @@ -0,0 +1,74 @@
> +videomode bindings
> +==
> +
> +Required properties:
> + - 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
> >>>
> >>> A major piece missing is the LCD controller to display interface width
> >>> and component ordering.
> >>
> >> I thought this binding was solely defining the timing of the video
> >> signal (hence "video mode"). Any definition of the physical interface to
> >> the LCD/display-connector is something entirely orthogonal, so it seems
> >> entirely reasonable to represent that separately.
> >
> > It is not orthogonal because in many cases the LCD panel defines the
> > mode.
>
> The LCD panel itself defines both the mode and the physical interface
> properties. The mode does not imply anything about the physical
> interface, nor does the physical interface imply anything about the
> mode. So, they are in fact orthogonal. In other words, just because you
> need both sets of information, doesn't make the two pieces of
> information correlated.
>
> +Example:
> +
> +display at 0 {
> >>>
> >>> It would be useful to have a compatible string here. We may not always
> >>> know the panel type or have a fixed panel though. We could define
> >>> "generic-lcd" or something for cases where the panel type is unknown.
> >>>
> +width-mm = <800>;
> +height-mm = <480>;
> >>
> >> I would hope that everything in the example above this point is just
> >> that - an example, and this binding only covers the display mode
> >> definition - i.e. that part of the example below.
> >>
> >
> > It's fairly clear this binding is being defined based on what Linux
> > supports vs. what the h/w looks like.
> >
> >> If that's not the intent, as Rob says, there's a /ton/ of stuff missing.
> >
> > Assuming not, what all is missing?
>
> Everything related to the physical interface:
>
> * For DSI, whatever it needs to be configured.
> * For LVDS, e.g. number of lanes of R, G, B.
> * Perhaps multi-pumping rates (# of clock signals to send each data
> value for, to satisfy any minimum clock rates)
> * Built-in displays typically need to be coupled with a backlight and
> all the associated control of that.
> * Pinctrl interaction.
>
> and probably a bunch of other things I haven't thought about.
I already added some of those things in my v5. For those who missed it
<1348500924-8551-1-git-send-email-s.trumtrar at pengutronix.de>
I renamed from "of videomode helper" to "of display helper", seemed to be more
clear what it is supposed to accomplish.
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- |
[PATCH v4] of: Add videomode helper
On Mon, Sep 24, 2012 at 09:16:28PM +0200, Sascha Hauer wrote:
> On Mon, Sep 24, 2012 at 10:18:39AM -0500, Rob Herring wrote:
> > On 09/24/2012 09:12 AM, Sascha Hauer wrote:
> > >>
> > >> A major piece missing is the LCD controller to display interface width
> > >> and component ordering.
> > >
> > > Psst. We silently skipped this for now...
> > >
> > > I think having such a videomode helper is useful without having the
> > > data order part. We are aware that this should be added later, but
> > > I think currently it makes sense to concentrate on the basics.
> >
> > Evolving bindings is not a good thing. We know this is needed, so we
> > should address it now. If Linux had a standard way to describe the
> > interface (it didn't a few years ago and I haven't kept up), then I
> > would bet it would already be part of this binding. Defining the
> > bindings in terms of what an OS uses or not is the wrong way around.
>
> I see your point. I'm just afraid that if we start a discussion about
> this now, it will take a long time we get something merged. In the
> meantime we will get a lot of ad-hoc bindings like this one
> suggested for the exynos:
>
> > + lcd_fimd0: lcd_panel0 {
> > + lcd-htiming = <4 4 4 480>;
> > + lcd-vtiming = <4 4 4 320>;
> > + supports-mipi-panel;
> > + };
>
> Or this one for the AMBA LCD controller;
>
> > + panel.mode.refresh = get_val(node, "refresh");
> > + panel.mode.xres = get_val(node, "xres");
> > + panel.mode.yres = get_val(node, "yres");
> > + panel.mode.pixclock = get_val(node, "pixclock");
> > + panel.mode.left_margin = get_val(node, "left_margin");
> > + panel.mode.right_margin = get_val(node, "right_margin");
> > + panel.mode.upper_margin = get_val(node, "upper_margin");
> > + panel.mode.lower_margin = get_val(node, "lower_margin");
> > + panel.mode.hsync_len= get_val(node, "hsync_len");
> > + panel.mode.vsync_len= get_val(node, "vsync_len");
> > + panel.mode.sync = get_val(node, "sync");
> > + panel.bpp = get_val(node, "bpp");
> > + panel.width = (signed short) get_val(node, "width");
> > + panel.height= (signed short) get_val(node, "height");
>
> (get_val() is a wrapper around of_property_read_u32)
>
> BTW the SoC-camera guys will need a wire format description for their
> cameras aswell.
>
> > >
> > > We expect the display nodes being subnodes of a driver (which of course
> > > has a compatible), or maybe referred to from a driver using phandles. So
> > > I don't see why the display node itself should have a compatible
> > > property. The display information is only ever useful in the context of
> > > a driver.
> >
> > A subnode or phandle will describe the h/w connection, but you need a
> > name to describe what is at each end of the connection.
> >
> > Where would the model number of an lcd panel be captured then? The
> > timing parameters are a property of a specific panel, so both should be
> > described together. You may not have any use for the compatible string
> > now, but more information is better than less and adding it would not
> > hurt anything. For pretty much any other device sitting on a board, we
> > describe the manufacturer and type of device. LCD panels should be no
> > different.
>
> You convinced me. Lets add a compatible property, it won't hurt.
Okay. That doesn't seem to be a big problem. I can add 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- |
Re: [PATCH v4] of: Add videomode helper
Hi Rob,
On Monday 24 September 2012 08:42:12 Rob Herring wrote:
> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
> > This patch adds a helper function for parsing videomodes from the
> > devicetree. The videomode can be either converted to a struct
> > drm_display_mode or a struct fb_videomode.
> >
> > Signed-off-by: Sascha Hauer
> > Signed-off-by: Steffen Trumtrar
> > ---
> >
> > Hi!
> >
> > changes since v3:
> > - print error messages
> > - free alloced memory
> > - general cleanup
> >
> > Regards
> > Steffen
> >
> > .../devicetree/bindings/video/displaymode | 74 +
> > drivers/of/Kconfig |5 +
> > drivers/of/Makefile|1 +
> > drivers/of/of_videomode.c | 283
> > include/linux/of_videomode.h
> > | 56
> > 5 files changed, 419 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/video/displaymode
> > create mode 100644 drivers/of/of_videomode.c
> > create mode 100644 include/linux/of_videomode.h
> >
> > diff --git a/Documentation/devicetree/bindings/video/displaymode
> > b/Documentation/devicetree/bindings/video/displaymode new file mode
> > 100644
> > index 000..990ca52
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/displaymode
> > @@ -0,0 +1,74 @@
> > +videomode bindings
> > +==
> > +
> > +Required properties:
> > + - 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
>
> A major piece missing is the LCD controller to display interface width
> and component ordering.
>
> > +
> > +Optional properties:
> > + - 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
> > + - interlaced (bool): This is an interlaced mode
> > + - doublescan (bool): This is a doublescan mode
> > +
> > +There are different ways of describing a display mode. The devicetree
> > representation
> > +corresponds to the one commonly found in datasheets for displays.
> > +The description of the display and its mode is split in two parts: first
> > the display
> > +properties like size in mm and (optionally) multiple subnodes with the
> > supported modes.
> > +
> > +Example:
> > +
> > + display@0 {
>
> It would be useful to have a compatible string here. We may not always
> know the panel type or have a fixed panel though. We could define
> "generic-lcd" or something for cases where the panel type is unknown.
I'm working on a generic panel framework (see
http://lwn.net/Articles/512363/). DT bindings are not there yet, but they're
certainly a hot topic. A compatible string here will definitely be needed
here.
The exact properties required in the display node will likely be panel-
dependent. width-mm, height-mm and modes sound like a good baseline to me.
> > + width-mm = <800>;
> > + height-mm = <480>;
> > + modes {
> > + mode0: mode@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:
> > +
> > + mode1: mode@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 videomode can be linked to a connector via phandles. The connector
> > has to
> > +support the display- and default-mode-property to link to and select a
> > mode.
>
> Could also be phandle in the lcd controller node? What are the '-' for?
> Is "display-blah" a valid name or something?
>
> "default-mode" is pretty generic. How about display-mode or
> display-default-mode?
>
> Rob
>
> > +
> > +Example:
> > +
> > + hdmi@0012 {
> > + status = "okay";
> > + display
Re: [PATCH v4] of: Add videomode helper
On Mon, Sep 24, 2012 at 05:09:30PM -0600, Stephen Warren wrote:
> On 09/24/2012 12:26 PM, Rob Herring wrote:
> > On 09/24/2012 10:45 AM, Stephen Warren wrote:
> >> On 09/24/2012 07:42 AM, Rob Herring wrote:
> >>> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
> This patch adds a helper function for parsing videomodes from the
> devicetree.
> The videomode can be either converted to a struct drm_display_mode or a
> struct fb_videomode.
> >>
> +++ b/Documentation/devicetree/bindings/video/displaymode
> @@ -0,0 +1,74 @@
> +videomode bindings
> +==
> +
> +Required properties:
> + - 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
> >>>
> >>> A major piece missing is the LCD controller to display interface width
> >>> and component ordering.
> >>
> >> I thought this binding was solely defining the timing of the video
> >> signal (hence "video mode"). Any definition of the physical interface to
> >> the LCD/display-connector is something entirely orthogonal, so it seems
> >> entirely reasonable to represent that separately.
> >
> > It is not orthogonal because in many cases the LCD panel defines the
> > mode.
>
> The LCD panel itself defines both the mode and the physical interface
> properties. The mode does not imply anything about the physical
> interface, nor does the physical interface imply anything about the
> mode. So, they are in fact orthogonal. In other words, just because you
> need both sets of information, doesn't make the two pieces of
> information correlated.
>
> +Example:
> +
> +display@0 {
> >>>
> >>> It would be useful to have a compatible string here. We may not always
> >>> know the panel type or have a fixed panel though. We could define
> >>> "generic-lcd" or something for cases where the panel type is unknown.
> >>>
> +width-mm = <800>;
> +height-mm = <480>;
> >>
> >> I would hope that everything in the example above this point is just
> >> that - an example, and this binding only covers the display mode
> >> definition - i.e. that part of the example below.
> >>
> >
> > It's fairly clear this binding is being defined based on what Linux
> > supports vs. what the h/w looks like.
> >
> >> If that's not the intent, as Rob says, there's a /ton/ of stuff missing.
> >
> > Assuming not, what all is missing?
>
> Everything related to the physical interface:
>
> * For DSI, whatever it needs to be configured.
> * For LVDS, e.g. number of lanes of R, G, B.
> * Perhaps multi-pumping rates (# of clock signals to send each data
> value for, to satisfy any minimum clock rates)
> * Built-in displays typically need to be coupled with a backlight and
> all the associated control of that.
> * Pinctrl interaction.
>
> and probably a bunch of other things I haven't thought about.
I already added some of those things in my v5. For those who missed it
<1348500924-8551-1-git-send-email-s.trumt...@pengutronix.de>
I renamed from "of videomode helper" to "of display helper", seemed to be more
clear what it is supposed to accomplish.
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- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] of: Add videomode helper
On Mon, Sep 24, 2012 at 09:16:28PM +0200, Sascha Hauer wrote:
> On Mon, Sep 24, 2012 at 10:18:39AM -0500, Rob Herring wrote:
> > On 09/24/2012 09:12 AM, Sascha Hauer wrote:
> > >>
> > >> A major piece missing is the LCD controller to display interface width
> > >> and component ordering.
> > >
> > > Psst. We silently skipped this for now...
> > >
> > > I think having such a videomode helper is useful without having the
> > > data order part. We are aware that this should be added later, but
> > > I think currently it makes sense to concentrate on the basics.
> >
> > Evolving bindings is not a good thing. We know this is needed, so we
> > should address it now. If Linux had a standard way to describe the
> > interface (it didn't a few years ago and I haven't kept up), then I
> > would bet it would already be part of this binding. Defining the
> > bindings in terms of what an OS uses or not is the wrong way around.
>
> I see your point. I'm just afraid that if we start a discussion about
> this now, it will take a long time we get something merged. In the
> meantime we will get a lot of ad-hoc bindings like this one
> suggested for the exynos:
>
> > + lcd_fimd0: lcd_panel0 {
> > + lcd-htiming = <4 4 4 480>;
> > + lcd-vtiming = <4 4 4 320>;
> > + supports-mipi-panel;
> > + };
>
> Or this one for the AMBA LCD controller;
>
> > + panel.mode.refresh = get_val(node, "refresh");
> > + panel.mode.xres = get_val(node, "xres");
> > + panel.mode.yres = get_val(node, "yres");
> > + panel.mode.pixclock = get_val(node, "pixclock");
> > + panel.mode.left_margin = get_val(node, "left_margin");
> > + panel.mode.right_margin = get_val(node, "right_margin");
> > + panel.mode.upper_margin = get_val(node, "upper_margin");
> > + panel.mode.lower_margin = get_val(node, "lower_margin");
> > + panel.mode.hsync_len= get_val(node, "hsync_len");
> > + panel.mode.vsync_len= get_val(node, "vsync_len");
> > + panel.mode.sync = get_val(node, "sync");
> > + panel.bpp = get_val(node, "bpp");
> > + panel.width = (signed short) get_val(node, "width");
> > + panel.height= (signed short) get_val(node, "height");
>
> (get_val() is a wrapper around of_property_read_u32)
>
> BTW the SoC-camera guys will need a wire format description for their
> cameras aswell.
>
> > >
> > > We expect the display nodes being subnodes of a driver (which of course
> > > has a compatible), or maybe referred to from a driver using phandles. So
> > > I don't see why the display node itself should have a compatible
> > > property. The display information is only ever useful in the context of
> > > a driver.
> >
> > A subnode or phandle will describe the h/w connection, but you need a
> > name to describe what is at each end of the connection.
> >
> > Where would the model number of an lcd panel be captured then? The
> > timing parameters are a property of a specific panel, so both should be
> > described together. You may not have any use for the compatible string
> > now, but more information is better than less and adding it would not
> > hurt anything. For pretty much any other device sitting on a board, we
> > describe the manufacturer and type of device. LCD panels should be no
> > different.
>
> You convinced me. Lets add a compatible property, it won't hurt.
Okay. That doesn't seem to be a big problem. I can add 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- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] of: Add videomode helper
On 09/24/2012 12:26 PM, Rob Herring wrote:
> On 09/24/2012 10:45 AM, Stephen Warren wrote:
>> On 09/24/2012 07:42 AM, Rob Herring wrote:
>>> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
This patch adds a helper function for parsing videomodes from the
devicetree.
The videomode can be either converted to a struct drm_display_mode or a
struct fb_videomode.
>>
+++ b/Documentation/devicetree/bindings/video/displaymode
@@ -0,0 +1,74 @@
+videomode bindings
+==
+
+Required properties:
+ - 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
>>>
>>> A major piece missing is the LCD controller to display interface width
>>> and component ordering.
>>
>> I thought this binding was solely defining the timing of the video
>> signal (hence "video mode"). Any definition of the physical interface to
>> the LCD/display-connector is something entirely orthogonal, so it seems
>> entirely reasonable to represent that separately.
>
> It is not orthogonal because in many cases the LCD panel defines the
> mode.
The LCD panel itself defines both the mode and the physical interface
properties. The mode does not imply anything about the physical
interface, nor does the physical interface imply anything about the
mode. So, they are in fact orthogonal. In other words, just because you
need both sets of information, doesn't make the two pieces of
information correlated.
+Example:
+
+ display@0 {
>>>
>>> It would be useful to have a compatible string here. We may not always
>>> know the panel type or have a fixed panel though. We could define
>>> "generic-lcd" or something for cases where the panel type is unknown.
>>>
+ width-mm = <800>;
+ height-mm = <480>;
>>
>> I would hope that everything in the example above this point is just
>> that - an example, and this binding only covers the display mode
>> definition - i.e. that part of the example below.
>>
>
> It's fairly clear this binding is being defined based on what Linux
> supports vs. what the h/w looks like.
>
>> If that's not the intent, as Rob says, there's a /ton/ of stuff missing.
>
> Assuming not, what all is missing?
Everything related to the physical interface:
* For DSI, whatever it needs to be configured.
* For LVDS, e.g. number of lanes of R, G, B.
* Perhaps multi-pumping rates (# of clock signals to send each data
value for, to satisfy any minimum clock rates)
* Built-in displays typically need to be coupled with a backlight and
all the associated control of that.
* Pinctrl interaction.
and probably a bunch of other things I haven't thought about.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] of: Add videomode helper
On 09/24/2012 10:45 AM, Stephen Warren wrote:
> On 09/24/2012 07:42 AM, Rob Herring wrote:
>> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
>>> This patch adds a helper function for parsing videomodes from the
>>> devicetree.
>>> The videomode can be either converted to a struct drm_display_mode or a
>>> struct fb_videomode.
>
>>> +++ b/Documentation/devicetree/bindings/video/displaymode
>>> @@ -0,0 +1,74 @@
>>> +videomode bindings
>>> +==
>>> +
>>> +Required properties:
>>> + - 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
>>
>> A major piece missing is the LCD controller to display interface width
>> and component ordering.
>
> I thought this binding was solely defining the timing of the video
> signal (hence "video mode"). Any definition of the physical interface to
> the LCD/display-connector is something entirely orthogonal, so it seems
> entirely reasonable to represent that separately.
It is not orthogonal because in many cases the LCD panel defines the
mode. It is only cases where you just have a connector like hdmi that
you have multiple modes. Ideally, you would use EDID in those cases, but
obviously there are boards where that is missing or broken.
>>> +Example:
>>> +
>>> + display@0 {
>>
>> It would be useful to have a compatible string here. We may not always
>> know the panel type or have a fixed panel though. We could define
>> "generic-lcd" or something for cases where the panel type is unknown.
>>
>>> + width-mm = <800>;
>>> + height-mm = <480>;
>
> I would hope that everything in the example above this point is just
> that - an example, and this binding only covers the display mode
> definition - i.e. that part of the example below.
>
It's fairly clear this binding is being defined based on what Linux
supports vs. what the h/w looks like.
> If that's not the intent, as Rob says, there's a /ton/ of stuff missing.
Assuming not, what all is missing?
Rob
>
>>> + modes {
>>> + mode0: mode@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;
>>> + };
>>> + };
>>> + };
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4] of: Add videomode helper
On Mon, Sep 24, 2012 at 10:18:39AM -0500, Rob Herring wrote:
> On 09/24/2012 09:12 AM, Sascha Hauer wrote:
> >>
> >> A major piece missing is the LCD controller to display interface width
> >> and component ordering.
> >
> > Psst. We silently skipped this for now...
> >
> > I think having such a videomode helper is useful without having the
> > data order part. We are aware that this should be added later, but
> > I think currently it makes sense to concentrate on the basics.
>
> Evolving bindings is not a good thing. We know this is needed, so we
> should address it now. If Linux had a standard way to describe the
> interface (it didn't a few years ago and I haven't kept up), then I
> would bet it would already be part of this binding. Defining the
> bindings in terms of what an OS uses or not is the wrong way around.
I see your point. I'm just afraid that if we start a discussion about
this now, it will take a long time we get something merged. In the
meantime we will get a lot of ad-hoc bindings like this one
suggested for the exynos:
> + lcd_fimd0: lcd_panel0 {
> + lcd-htiming = <4 4 4 480>;
> + lcd-vtiming = <4 4 4 320>;
> + supports-mipi-panel;
> + };
Or this one for the AMBA LCD controller;
> + panel.mode.refresh = get_val(node, "refresh");
> + panel.mode.xres = get_val(node, "xres");
> + panel.mode.yres = get_val(node, "yres");
> + panel.mode.pixclock = get_val(node, "pixclock");
> + panel.mode.left_margin = get_val(node, "left_margin");
> + panel.mode.right_margin = get_val(node, "right_margin");
> + panel.mode.upper_margin = get_val(node, "upper_margin");
> + panel.mode.lower_margin = get_val(node, "lower_margin");
> + panel.mode.hsync_len= get_val(node, "hsync_len");
> + panel.mode.vsync_len= get_val(node, "vsync_len");
> + panel.mode.sync = get_val(node, "sync");
> + panel.bpp = get_val(node, "bpp");
> + panel.width = (signed short) get_val(node, "width");
> + panel.height= (signed short) get_val(node, "height");
(get_val() is a wrapper around of_property_read_u32)
BTW the SoC-camera guys will need a wire format description for their
cameras aswell.
> >
> > We expect the display nodes being subnodes of a driver (which of course
> > has a compatible), or maybe referred to from a driver using phandles. So
> > I don't see why the display node itself should have a compatible
> > property. The display information is only ever useful in the context of
> > a driver.
>
> A subnode or phandle will describe the h/w connection, but you need a
> name to describe what is at each end of the connection.
>
> Where would the model number of an lcd panel be captured then? The
> timing parameters are a property of a specific panel, so both should be
> described together. You may not have any use for the compatible string
> now, but more information is better than less and adding it would not
> hurt anything. For pretty much any other device sitting on a board, we
> describe the manufacturer and type of device. LCD panels should be no
> different.
You convinced me. Lets add a compatible property, it won't hurt.
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- |
[PATCH v4] of: Add videomode helper
On 09/24/2012 12:26 PM, Rob Herring wrote:
> On 09/24/2012 10:45 AM, Stephen Warren wrote:
>> On 09/24/2012 07:42 AM, Rob Herring wrote:
>>> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
This patch adds a helper function for parsing videomodes from the
devicetree.
The videomode can be either converted to a struct drm_display_mode or a
struct fb_videomode.
>>
+++ b/Documentation/devicetree/bindings/video/displaymode
@@ -0,0 +1,74 @@
+videomode bindings
+==
+
+Required properties:
+ - 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
>>>
>>> A major piece missing is the LCD controller to display interface width
>>> and component ordering.
>>
>> I thought this binding was solely defining the timing of the video
>> signal (hence "video mode"). Any definition of the physical interface to
>> the LCD/display-connector is something entirely orthogonal, so it seems
>> entirely reasonable to represent that separately.
>
> It is not orthogonal because in many cases the LCD panel defines the
> mode.
The LCD panel itself defines both the mode and the physical interface
properties. The mode does not imply anything about the physical
interface, nor does the physical interface imply anything about the
mode. So, they are in fact orthogonal. In other words, just because you
need both sets of information, doesn't make the two pieces of
information correlated.
+Example:
+
+ display at 0 {
>>>
>>> It would be useful to have a compatible string here. We may not always
>>> know the panel type or have a fixed panel though. We could define
>>> "generic-lcd" or something for cases where the panel type is unknown.
>>>
+ width-mm = <800>;
+ height-mm = <480>;
>>
>> I would hope that everything in the example above this point is just
>> that - an example, and this binding only covers the display mode
>> definition - i.e. that part of the example below.
>>
>
> It's fairly clear this binding is being defined based on what Linux
> supports vs. what the h/w looks like.
>
>> If that's not the intent, as Rob says, there's a /ton/ of stuff missing.
>
> Assuming not, what all is missing?
Everything related to the physical interface:
* For DSI, whatever it needs to be configured.
* For LVDS, e.g. number of lanes of R, G, B.
* Perhaps multi-pumping rates (# of clock signals to send each data
value for, to satisfy any minimum clock rates)
* Built-in displays typically need to be coupled with a backlight and
all the associated control of that.
* Pinctrl interaction.
and probably a bunch of other things I haven't thought about.
[PATCH v4] of: Add videomode helper
Hi Rob,
On Mon, Sep 24, 2012 at 08:42:12AM -0500, Rob Herring wrote:
> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
> > This patch adds a helper function for parsing videomodes from the
> > devicetree.
> > The videomode can be either converted to a struct drm_display_mode or a
> > struct fb_videomode.
> >
> > +
> > +Required properties:
> > + - 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
>
> A major piece missing is the LCD controller to display interface width
> and component ordering.
Psst. We silently skipped this for now...
I think having such a videomode helper is useful without having the
data order part. We are aware that this should be added later, but
I think currently it makes sense to concentrate on the basics.
>
> > +
> > +Optional properties:
> > + - 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
> > + - interlaced (bool): This is an interlaced mode
> > + - doublescan (bool): This is a doublescan mode
> > +
> > +There are different ways of describing a display mode. The devicetree
> > representation
> > +corresponds to the one commonly found in datasheets for displays.
> > +The description of the display and its mode is split in two parts: first
> > the display
> > +properties like size in mm and (optionally) multiple subnodes with the
> > supported modes.
> > +
> > +Example:
> > +
> > + display at 0 {
>
> It would be useful to have a compatible string here. We may not always
> know the panel type or have a fixed panel though. We could define
> "generic-lcd" or something for cases where the panel type is unknown.
We expect the display nodes being subnodes of a driver (which of course
has a compatible), or maybe referred to from a driver using phandles. So
I don't see why the display node itself should have a compatible
property. The display information is only ever useful in the context of
a driver.
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- |
[PATCH v4] of: Add videomode helper
On 09/24/2012 10:45 AM, Stephen Warren wrote:
> On 09/24/2012 07:42 AM, Rob Herring wrote:
>> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
>>> This patch adds a helper function for parsing videomodes from the
>>> devicetree.
>>> The videomode can be either converted to a struct drm_display_mode or a
>>> struct fb_videomode.
>
>>> +++ b/Documentation/devicetree/bindings/video/displaymode
>>> @@ -0,0 +1,74 @@
>>> +videomode bindings
>>> +==
>>> +
>>> +Required properties:
>>> + - 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
>>
>> A major piece missing is the LCD controller to display interface width
>> and component ordering.
>
> I thought this binding was solely defining the timing of the video
> signal (hence "video mode"). Any definition of the physical interface to
> the LCD/display-connector is something entirely orthogonal, so it seems
> entirely reasonable to represent that separately.
It is not orthogonal because in many cases the LCD panel defines the
mode. It is only cases where you just have a connector like hdmi that
you have multiple modes. Ideally, you would use EDID in those cases, but
obviously there are boards where that is missing or broken.
>>> +Example:
>>> +
>>> + display at 0 {
>>
>> It would be useful to have a compatible string here. We may not always
>> know the panel type or have a fixed panel though. We could define
>> "generic-lcd" or something for cases where the panel type is unknown.
>>
>>> + width-mm = <800>;
>>> + height-mm = <480>;
>
> I would hope that everything in the example above this point is just
> that - an example, and this binding only covers the display mode
> definition - i.e. that part of the example below.
>
It's fairly clear this binding is being defined based on what Linux
supports vs. what the h/w looks like.
> If that's not the intent, as Rob says, there's a /ton/ of stuff missing.
Assuming not, what all is missing?
Rob
>
>>> + modes {
>>> + mode0: mode 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;
>>> + };
>>> + };
>>> + };
>
Re: [PATCH v4] of: Add videomode helper
On Mon, Sep 24, 2012 at 10:18:39AM -0500, Rob Herring wrote:
> On 09/24/2012 09:12 AM, Sascha Hauer wrote:
> >>
> >> A major piece missing is the LCD controller to display interface width
> >> and component ordering.
> >
> > Psst. We silently skipped this for now...
> >
> > I think having such a videomode helper is useful without having the
> > data order part. We are aware that this should be added later, but
> > I think currently it makes sense to concentrate on the basics.
>
> Evolving bindings is not a good thing. We know this is needed, so we
> should address it now. If Linux had a standard way to describe the
> interface (it didn't a few years ago and I haven't kept up), then I
> would bet it would already be part of this binding. Defining the
> bindings in terms of what an OS uses or not is the wrong way around.
I see your point. I'm just afraid that if we start a discussion about
this now, it will take a long time we get something merged. In the
meantime we will get a lot of ad-hoc bindings like this one
suggested for the exynos:
> + lcd_fimd0: lcd_panel0 {
> + lcd-htiming = <4 4 4 480>;
> + lcd-vtiming = <4 4 4 320>;
> + supports-mipi-panel;
> + };
Or this one for the AMBA LCD controller;
> + panel.mode.refresh = get_val(node, "refresh");
> + panel.mode.xres = get_val(node, "xres");
> + panel.mode.yres = get_val(node, "yres");
> + panel.mode.pixclock = get_val(node, "pixclock");
> + panel.mode.left_margin = get_val(node, "left_margin");
> + panel.mode.right_margin = get_val(node, "right_margin");
> + panel.mode.upper_margin = get_val(node, "upper_margin");
> + panel.mode.lower_margin = get_val(node, "lower_margin");
> + panel.mode.hsync_len= get_val(node, "hsync_len");
> + panel.mode.vsync_len= get_val(node, "vsync_len");
> + panel.mode.sync = get_val(node, "sync");
> + panel.bpp = get_val(node, "bpp");
> + panel.width = (signed short) get_val(node, "width");
> + panel.height= (signed short) get_val(node, "height");
(get_val() is a wrapper around of_property_read_u32)
BTW the SoC-camera guys will need a wire format description for their
cameras aswell.
> >
> > We expect the display nodes being subnodes of a driver (which of course
> > has a compatible), or maybe referred to from a driver using phandles. So
> > I don't see why the display node itself should have a compatible
> > property. The display information is only ever useful in the context of
> > a driver.
>
> A subnode or phandle will describe the h/w connection, but you need a
> name to describe what is at each end of the connection.
>
> Where would the model number of an lcd panel be captured then? The
> timing parameters are a property of a specific panel, so both should be
> described together. You may not have any use for the compatible string
> now, but more information is better than less and adding it would not
> hurt anything. For pretty much any other device sitting on a board, we
> describe the manufacturer and type of device. LCD panels should be no
> different.
You convinced me. Lets add a compatible property, it won't hurt.
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- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4] of: Add videomode helper
On 09/24/2012 09:12 AM, Sascha Hauer wrote:
> Hi Rob,
>
> On Mon, Sep 24, 2012 at 08:42:12AM -0500, Rob Herring wrote:
>> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
>>> This patch adds a helper function for parsing videomodes from the
>>> devicetree.
>>> The videomode can be either converted to a struct drm_display_mode or a
>>> struct fb_videomode.
>>>
>>> +
>>> +Required properties:
>>> + - 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
>>
>> A major piece missing is the LCD controller to display interface width
>> and component ordering.
>
> Psst. We silently skipped this for now...
>
> I think having such a videomode helper is useful without having the
> data order part. We are aware that this should be added later, but
> I think currently it makes sense to concentrate on the basics.
Evolving bindings is not a good thing. We know this is needed, so we
should address it now. If Linux had a standard way to describe the
interface (it didn't a few years ago and I haven't kept up), then I
would bet it would already be part of this binding. Defining the
bindings in terms of what an OS uses or not is the wrong way around.
>>
>>> +
>>> +Optional properties:
>>> + - 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
>>> + - interlaced (bool): This is an interlaced mode
>>> + - doublescan (bool): This is a doublescan mode
>>> +
>>> +There are different ways of describing a display mode. The devicetree
>>> representation
>>> +corresponds to the one commonly found in datasheets for displays.
>>> +The description of the display and its mode is split in two parts: first
>>> the display
>>> +properties like size in mm and (optionally) multiple subnodes with the
>>> supported modes.
>>> +
>>> +Example:
>>> +
>>> + display at 0 {
>>
>> It would be useful to have a compatible string here. We may not always
>> know the panel type or have a fixed panel though. We could define
>> "generic-lcd" or something for cases where the panel type is unknown.
>
> We expect the display nodes being subnodes of a driver (which of course
> has a compatible), or maybe referred to from a driver using phandles. So
> I don't see why the display node itself should have a compatible
> property. The display information is only ever useful in the context of
> a driver.
A subnode or phandle will describe the h/w connection, but you need a
name to describe what is at each end of the connection.
Where would the model number of an lcd panel be captured then? The
timing parameters are a property of a specific panel, so both should be
described together. You may not have any use for the compatible string
now, but more information is better than less and adding it would not
hurt anything. For pretty much any other device sitting on a board, we
describe the manufacturer and type of device. LCD panels should be no
different.
Rob
[PATCH v4] of: Add videomode helper
On 09/24/2012 07:42 AM, Rob Herring wrote:
> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
>> This patch adds a helper function for parsing videomodes from the devicetree.
>> The videomode can be either converted to a struct drm_display_mode or a
>> struct fb_videomode.
>> +++ b/Documentation/devicetree/bindings/video/displaymode
>> @@ -0,0 +1,74 @@
>> +videomode bindings
>> +==
>> +
>> +Required properties:
>> + - 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
>
> A major piece missing is the LCD controller to display interface width
> and component ordering.
I thought this binding was solely defining the timing of the video
signal (hence "video mode"). Any definition of the physical interface to
the LCD/display-connector is something entirely orthogonal, so it seems
entirely reasonable to represent that separately.
>> +Example:
>> +
>> +display at 0 {
>
> It would be useful to have a compatible string here. We may not always
> know the panel type or have a fixed panel though. We could define
> "generic-lcd" or something for cases where the panel type is unknown.
>
>> +width-mm = <800>;
>> +height-mm = <480>;
I would hope that everything in the example above this point is just
that - an example, and this binding only covers the display mode
definition - i.e. that part of the example below.
If that's not the intent, as Rob says, there's a /ton/ of stuff missing.
>> +modes {
>> +mode0: mode 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;
>> +};
>> +};
>> +};
Re: [PATCH v4] of: Add videomode helper
On 09/24/2012 07:42 AM, Rob Herring wrote:
> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
>> This patch adds a helper function for parsing videomodes from the devicetree.
>> The videomode can be either converted to a struct drm_display_mode or a
>> struct fb_videomode.
>> +++ b/Documentation/devicetree/bindings/video/displaymode
>> @@ -0,0 +1,74 @@
>> +videomode bindings
>> +==
>> +
>> +Required properties:
>> + - 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
>
> A major piece missing is the LCD controller to display interface width
> and component ordering.
I thought this binding was solely defining the timing of the video
signal (hence "video mode"). Any definition of the physical interface to
the LCD/display-connector is something entirely orthogonal, so it seems
entirely reasonable to represent that separately.
>> +Example:
>> +
>> +display@0 {
>
> It would be useful to have a compatible string here. We may not always
> know the panel type or have a fixed panel though. We could define
> "generic-lcd" or something for cases where the panel type is unknown.
>
>> +width-mm = <800>;
>> +height-mm = <480>;
I would hope that everything in the example above this point is just
that - an example, and this binding only covers the display mode
definition - i.e. that part of the example below.
If that's not the intent, as Rob says, there's a /ton/ of stuff missing.
>> +modes {
>> +mode0: mode@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;
>> +};
>> +};
>> +};
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] of: Add videomode helper
On 09/24/2012 09:12 AM, Sascha Hauer wrote:
> Hi Rob,
>
> On Mon, Sep 24, 2012 at 08:42:12AM -0500, Rob Herring wrote:
>> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
>>> This patch adds a helper function for parsing videomodes from the
>>> devicetree.
>>> The videomode can be either converted to a struct drm_display_mode or a
>>> struct fb_videomode.
>>>
>>> +
>>> +Required properties:
>>> + - 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
>>
>> A major piece missing is the LCD controller to display interface width
>> and component ordering.
>
> Psst. We silently skipped this for now...
>
> I think having such a videomode helper is useful without having the
> data order part. We are aware that this should be added later, but
> I think currently it makes sense to concentrate on the basics.
Evolving bindings is not a good thing. We know this is needed, so we
should address it now. If Linux had a standard way to describe the
interface (it didn't a few years ago and I haven't kept up), then I
would bet it would already be part of this binding. Defining the
bindings in terms of what an OS uses or not is the wrong way around.
>>
>>> +
>>> +Optional properties:
>>> + - 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
>>> + - interlaced (bool): This is an interlaced mode
>>> + - doublescan (bool): This is a doublescan mode
>>> +
>>> +There are different ways of describing a display mode. The devicetree
>>> representation
>>> +corresponds to the one commonly found in datasheets for displays.
>>> +The description of the display and its mode is split in two parts: first
>>> the display
>>> +properties like size in mm and (optionally) multiple subnodes with the
>>> supported modes.
>>> +
>>> +Example:
>>> +
>>> + display@0 {
>>
>> It would be useful to have a compatible string here. We may not always
>> know the panel type or have a fixed panel though. We could define
>> "generic-lcd" or something for cases where the panel type is unknown.
>
> We expect the display nodes being subnodes of a driver (which of course
> has a compatible), or maybe referred to from a driver using phandles. So
> I don't see why the display node itself should have a compatible
> property. The display information is only ever useful in the context of
> a driver.
A subnode or phandle will describe the h/w connection, but you need a
name to describe what is at each end of the connection.
Where would the model number of an lcd panel be captured then? The
timing parameters are a property of a specific panel, so both should be
described together. You may not have any use for the compatible string
now, but more information is better than less and adding it would not
hurt anything. For pretty much any other device sitting on a board, we
describe the manufacturer and type of device. LCD panels should be no
different.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] of: Add videomode helper
On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
> This patch adds a helper function for parsing videomodes from the devicetree.
> The videomode can be either converted to a struct drm_display_mode or a
> struct fb_videomode.
>
> Signed-off-by: Sascha Hauer
> Signed-off-by: Steffen Trumtrar
> ---
>
> Hi!
>
> changes since v3:
> - print error messages
> - free alloced memory
> - general cleanup
>
> Regards
> Steffen
>
> .../devicetree/bindings/video/displaymode | 74 +
> drivers/of/Kconfig |5 +
> drivers/of/Makefile|1 +
> drivers/of/of_videomode.c | 283
>
> include/linux/of_videomode.h | 56
> 5 files changed, 419 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/displaymode
> create mode 100644 drivers/of/of_videomode.c
> create mode 100644 include/linux/of_videomode.h
>
> diff --git a/Documentation/devicetree/bindings/video/displaymode
> b/Documentation/devicetree/bindings/video/displaymode
> new file mode 100644
> index 000..990ca52
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/displaymode
> @@ -0,0 +1,74 @@
> +videomode bindings
> +==
> +
> +Required properties:
> + - 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
A major piece missing is the LCD controller to display interface width
and component ordering.
> +
> +Optional properties:
> + - 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
> + - interlaced (bool): This is an interlaced mode
> + - doublescan (bool): This is a doublescan mode
> +
> +There are different ways of describing a display mode. The devicetree
> representation
> +corresponds to the one commonly found in datasheets for displays.
> +The description of the display and its mode is split in two parts: first the
> display
> +properties like size in mm and (optionally) multiple subnodes with the
> supported modes.
> +
> +Example:
> +
> + display@0 {
It would be useful to have a compatible string here. We may not always
know the panel type or have a fixed panel though. We could define
"generic-lcd" or something for cases where the panel type is unknown.
> + width-mm = <800>;
> + height-mm = <480>;
> + modes {
> + mode0: mode@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:
> +
> + mode1: mode@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 videomode can be linked to a connector via phandles. The connector has to
> +support the display- and default-mode-property to link to and select a mode.
Could also be phandle in the lcd controller node? What are the '-' for?
Is "display-blah" a valid name or something?
"default-mode" is pretty generic. How about display-mode or
display-default-mode?
Rob
> +
> +Example:
> +
> + hdmi@0012 {
> + status = "okay";
> + display = <&benq>;
> + default-mode = <&mode1>;
> + };
> +
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index dfba3e6..a3acaa3 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -83,4 +83,9 @@ config OF_MTD
> depends on MTD
> def_bool y
>
> +config OF_VIDEOMODE
> + def_bool y
> + help
> + helper to parse videomodes from the devicetree
> +
> endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index e027f44..80e6db3 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_
[PATCH v4] of: Add videomode helper
On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
> This patch adds a helper function for parsing videomodes from the devicetree.
> The videomode can be either converted to a struct drm_display_mode or a
> struct fb_videomode.
>
> Signed-off-by: Sascha Hauer
> Signed-off-by: Steffen Trumtrar
> ---
>
> Hi!
>
> changes since v3:
> - print error messages
> - free alloced memory
> - general cleanup
>
> Regards
> Steffen
>
> .../devicetree/bindings/video/displaymode | 74 +
> drivers/of/Kconfig |5 +
> drivers/of/Makefile|1 +
> drivers/of/of_videomode.c | 283
>
> include/linux/of_videomode.h | 56
> 5 files changed, 419 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/displaymode
> create mode 100644 drivers/of/of_videomode.c
> create mode 100644 include/linux/of_videomode.h
>
> diff --git a/Documentation/devicetree/bindings/video/displaymode
> b/Documentation/devicetree/bindings/video/displaymode
> new file mode 100644
> index 000..990ca52
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/displaymode
> @@ -0,0 +1,74 @@
> +videomode bindings
> +==
> +
> +Required properties:
> + - 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
A major piece missing is the LCD controller to display interface width
and component ordering.
> +
> +Optional properties:
> + - 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
> + - interlaced (bool): This is an interlaced mode
> + - doublescan (bool): This is a doublescan mode
> +
> +There are different ways of describing a display mode. The devicetree
> representation
> +corresponds to the one commonly found in datasheets for displays.
> +The description of the display and its mode is split in two parts: first the
> display
> +properties like size in mm and (optionally) multiple subnodes with the
> supported modes.
> +
> +Example:
> +
> + display at 0 {
It would be useful to have a compatible string here. We may not always
know the panel type or have a fixed panel though. We could define
"generic-lcd" or something for cases where the panel type is unknown.
> + width-mm = <800>;
> + height-mm = <480>;
> + modes {
> + mode0: mode 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:
> +
> + mode1: mode 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 videomode can be linked to a connector via phandles. The connector has to
> +support the display- and default-mode-property to link to and select a mode.
Could also be phandle in the lcd controller node? What are the '-' for?
Is "display-blah" a valid name or something?
"default-mode" is pretty generic. How about display-mode or
display-default-mode?
Rob
> +
> +Example:
> +
> + hdmi at 0012 {
> + status = "okay";
> + display = <&benq>;
> + default-mode = <&mode1>;
> + };
> +
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index dfba3e6..a3acaa3 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -83,4 +83,9 @@ config OF_MTD
> depends on MTD
> def_bool y
>
> +config OF_VIDEOMODE
> + def_bool y
> + help
> + helper to parse videomodes from the devicetree
> +
> endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index e027f44..80e6db3 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -11,3 +11,4 @@ o
Re: [PATCH v4] of: Add videomode helper
Hi Rob,
On Mon, Sep 24, 2012 at 08:42:12AM -0500, Rob Herring wrote:
> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote:
> > This patch adds a helper function for parsing videomodes from the
> > devicetree.
> > The videomode can be either converted to a struct drm_display_mode or a
> > struct fb_videomode.
> >
> > +
> > +Required properties:
> > + - 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
>
> A major piece missing is the LCD controller to display interface width
> and component ordering.
Psst. We silently skipped this for now...
I think having such a videomode helper is useful without having the
data order part. We are aware that this should be added later, but
I think currently it makes sense to concentrate on the basics.
>
> > +
> > +Optional properties:
> > + - 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
> > + - interlaced (bool): This is an interlaced mode
> > + - doublescan (bool): This is a doublescan mode
> > +
> > +There are different ways of describing a display mode. The devicetree
> > representation
> > +corresponds to the one commonly found in datasheets for displays.
> > +The description of the display and its mode is split in two parts: first
> > the display
> > +properties like size in mm and (optionally) multiple subnodes with the
> > supported modes.
> > +
> > +Example:
> > +
> > + display@0 {
>
> It would be useful to have a compatible string here. We may not always
> know the panel type or have a fixed panel though. We could define
> "generic-lcd" or something for cases where the panel type is unknown.
We expect the display nodes being subnodes of a driver (which of course
has a compatible), or maybe referred to from a driver using phandles. So
I don't see why the display node itself should have a compatible
property. The display information is only ever useful in the context of
a driver.
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- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4] of: Add videomode helper
On Wed September 19 2012 10:20:43 Steffen Trumtrar wrote:
> This patch adds a helper function for parsing videomodes from the devicetree.
> The videomode can be either converted to a struct drm_display_mode or a
> struct fb_videomode.
>
> Signed-off-by: Sascha Hauer
> Signed-off-by: Steffen Trumtrar
> ---
>
> Hi!
>
> changes since v3:
> - print error messages
> - free alloced memory
> - general cleanup
>
> Regards
> Steffen
>
> .../devicetree/bindings/video/displaymode | 74 +
> drivers/of/Kconfig |5 +
> drivers/of/Makefile|1 +
> drivers/of/of_videomode.c | 283
>
> include/linux/of_videomode.h | 56
> 5 files changed, 419 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/displaymode
> create mode 100644 drivers/of/of_videomode.c
> create mode 100644 include/linux/of_videomode.h
>
> diff --git a/Documentation/devicetree/bindings/video/displaymode
> b/Documentation/devicetree/bindings/video/displaymode
> new file mode 100644
> index 000..990ca52
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/displaymode
> @@ -0,0 +1,74 @@
> +videomode bindings
> +==
> +
> +Required properties:
> + - 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
In the case of interlaced formats, can the vfront-porch, vback-porch and
vsync-len
for the second field always be calculated from these values? Is that fully
standardized or can there be exceptions? struct v4l2_bt_timings has separate
fields
for these, but that may have been overkill.
> + - clock: displayclock in Hz
CEA-861 defined the option to slightly lower the clock frequency by 1.000/1.001
to
achieve frequencies like 29.97 or 59.94. In v4l2_bt_timings I made a flag for
that.
I'm not sure whether it is better to just set the clock to the correct frequency
(which is a weird value) or mark it with a bool.
> +
> +Optional properties:
> + - 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
> + - interlaced (bool): This is an interlaced mode
> + - doublescan (bool): This is a doublescan mode
> +
> +There are different ways of describing a display mode. The devicetree
> representation
> +corresponds to the one commonly found in datasheets for displays.
> +The description of the display and its mode is split in two parts: first the
> display
> +properties like size in mm and (optionally) multiple subnodes with the
> supported modes.
> +
> +Example:
> +
> + display at 0 {
> + width-mm = <800>;
> + height-mm = <480>;
> + modes {
> + mode0: mode 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:
> +
> + mode1: mode 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 videomode can be linked to a connector via phandles. The connector has to
> +support the display- and default-mode-property to link to and select a mode.
> +
> +Example:
> +
> + hdmi at 0012 {
> + status = "okay";
> + display = <&benq>;
> + default-mode = <&mode1>;
> + };
> +
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index dfba3e6..a3acaa3 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -83,4 +83,9 @@ config OF_MTD
> depends on MTD
> def_bool y
>
> +config OF_VIDEOMODE
> + def_bool y
> + help
> + helper to parse videomodes from the devicetree
> +
> endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index e027f44..80e6
[PATCH v4] of: Add videomode helper
On Thu, Sep 20, 2012 at 11:29:42PM +0200, Laurent Pinchart wrote:
> Hi,
>
> (CC'ing the linux-media mailing list, as video modes are of interest there as
> well)
>
> On Wednesday 19 September 2012 12:19:22 Tomi Valkeinen wrote:
> > On Wed, 2012-09-19 at 10:20 +0200, Steffen Trumtrar wrote:
> > > This patch adds a helper function for parsing videomodes from the
> > > devicetree. The videomode can be either converted to a struct
> > > drm_display_mode or a struct fb_videomode.
> > >
> > > Signed-off-by: Sascha Hauer
> > > Signed-off-by: Steffen Trumtrar
> > > ---
> > >
> > > Hi!
> > >
> > > changes since v3:
> > > - print error messages
> > > - free alloced memory
> > > - general cleanup
> > >
> > > Regards
> > > Steffen
> > >
> > > .../devicetree/bindings/video/displaymode | 74 +
> > > drivers/of/Kconfig |5 +
> > > drivers/of/Makefile|1 +
> > > drivers/of/of_videomode.c | 283 +++
> > > include/linux/of_videomode.h | 56
> > > 5 files changed, 419 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/video/displaymode
> > > create mode 100644 drivers/of/of_videomode.c
> > > create mode 100644 include/linux/of_videomode.h
> > >
> > > diff --git a/Documentation/devicetree/bindings/video/displaymode
> > > b/Documentation/devicetree/bindings/video/displaymode new file mode
> > > 100644
> > > index 000..990ca52
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/video/displaymode
> > > @@ -0,0 +1,74 @@
> > > +videomode bindings
> > > +==
> > > +
> > > +Required properties:
> > > + - 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
> > > +
> > > +Optional properties:
> > > + - 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
> > > + - interlaced (bool): This is an interlaced mode
> > > + - doublescan (bool): This is a doublescan mode
> > > +
> > > +There are different ways of describing a display mode. The devicetree
> > > representation
> > > +corresponds to the one commonly found in datasheets for displays.
> > > +The description of the display and its mode is split in two parts: first
> > > the display
> > > +properties like size in mm and (optionally) multiple subnodes with the
> > > supported modes.
> > > +
> > > +Example:
> > > +
> > > + display at 0 {
> > > + width-mm = <800>;
> > > + height-mm = <480>;
> > > + modes {
> > > + mode0: mode 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:
> > > +
> > > + mode1: mode 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 videomode can be linked to a connector via phandles. The connector
> > > has to
> > > +support the display- and default-mode-property to link to and select a
> > > mode.
> > > +
> > > +Example:
> > > +
> > > + hdmi at 0012 {
> > > + status = "okay";
> > > + display = <&benq>;
> > > + default-mode = <&mode1>;
> > > + };
> > > +
> > > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> > > index dfba3e6..a3acaa3 100644
> > > --- a/drivers/of/Kconfig
> > > +++ b/drivers/of/Kconfig
> > > @@ -83,4 +83,9 @@ config OF_MTD
> > >
> > > depends on MTD
> > > def_bool y
> > >
> > > +config OF_VIDEOMODE
> > > + def_bool y
> > > + help
> > > + helper to parse videomodes from the devicetree
> > > +
> > >
> > > endmenu # OF
> > >
> > > diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> > > index e027
Re: [PATCH v4] of: Add videomode helper
On Thu, Sep 20, 2012 at 11:29:42PM +0200, Laurent Pinchart wrote:
> Hi,
>
> (CC'ing the linux-media mailing list, as video modes are of interest there as
> well)
>
> On Wednesday 19 September 2012 12:19:22 Tomi Valkeinen wrote:
> > On Wed, 2012-09-19 at 10:20 +0200, Steffen Trumtrar wrote:
> > > This patch adds a helper function for parsing videomodes from the
> > > devicetree. The videomode can be either converted to a struct
> > > drm_display_mode or a struct fb_videomode.
> > >
> > > Signed-off-by: Sascha Hauer
> > > Signed-off-by: Steffen Trumtrar
> > > ---
> > >
> > > Hi!
> > >
> > > changes since v3:
> > > - print error messages
> > > - free alloced memory
> > > - general cleanup
> > >
> > > Regards
> > > Steffen
> > >
> > > .../devicetree/bindings/video/displaymode | 74 +
> > > drivers/of/Kconfig |5 +
> > > drivers/of/Makefile|1 +
> > > drivers/of/of_videomode.c | 283 +++
> > > include/linux/of_videomode.h | 56
> > > 5 files changed, 419 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/video/displaymode
> > > create mode 100644 drivers/of/of_videomode.c
> > > create mode 100644 include/linux/of_videomode.h
> > >
> > > diff --git a/Documentation/devicetree/bindings/video/displaymode
> > > b/Documentation/devicetree/bindings/video/displaymode new file mode
> > > 100644
> > > index 000..990ca52
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/video/displaymode
> > > @@ -0,0 +1,74 @@
> > > +videomode bindings
> > > +==
> > > +
> > > +Required properties:
> > > + - 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
> > > +
> > > +Optional properties:
> > > + - 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
> > > + - interlaced (bool): This is an interlaced mode
> > > + - doublescan (bool): This is a doublescan mode
> > > +
> > > +There are different ways of describing a display mode. The devicetree
> > > representation
> > > +corresponds to the one commonly found in datasheets for displays.
> > > +The description of the display and its mode is split in two parts: first
> > > the display
> > > +properties like size in mm and (optionally) multiple subnodes with the
> > > supported modes.
> > > +
> > > +Example:
> > > +
> > > + display@0 {
> > > + width-mm = <800>;
> > > + height-mm = <480>;
> > > + modes {
> > > + mode0: mode@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:
> > > +
> > > + mode1: mode@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 videomode can be linked to a connector via phandles. The connector
> > > has to
> > > +support the display- and default-mode-property to link to and select a
> > > mode.
> > > +
> > > +Example:
> > > +
> > > + hdmi@0012 {
> > > + status = "okay";
> > > + display = <&benq>;
> > > + default-mode = <&mode1>;
> > > + };
> > > +
> > > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> > > index dfba3e6..a3acaa3 100644
> > > --- a/drivers/of/Kconfig
> > > +++ b/drivers/of/Kconfig
> > > @@ -83,4 +83,9 @@ config OF_MTD
> > >
> > > depends on MTD
> > > def_bool y
> > >
> > > +config OF_VIDEOMODE
> > > + def_bool y
> > > + help
> > > + helper to parse videomodes from the devicetree
> > > +
> > >
> > > endmenu # OF
> > >
> > > diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> > > index e027f44..80e6db3
Re: [PATCH v4] of: Add videomode helper
On Wed September 19 2012 10:20:43 Steffen Trumtrar wrote:
> This patch adds a helper function for parsing videomodes from the devicetree.
> The videomode can be either converted to a struct drm_display_mode or a
> struct fb_videomode.
>
> Signed-off-by: Sascha Hauer
> Signed-off-by: Steffen Trumtrar
> ---
>
> Hi!
>
> changes since v3:
> - print error messages
> - free alloced memory
> - general cleanup
>
> Regards
> Steffen
>
> .../devicetree/bindings/video/displaymode | 74 +
> drivers/of/Kconfig |5 +
> drivers/of/Makefile|1 +
> drivers/of/of_videomode.c | 283
>
> include/linux/of_videomode.h | 56
> 5 files changed, 419 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/displaymode
> create mode 100644 drivers/of/of_videomode.c
> create mode 100644 include/linux/of_videomode.h
>
> diff --git a/Documentation/devicetree/bindings/video/displaymode
> b/Documentation/devicetree/bindings/video/displaymode
> new file mode 100644
> index 000..990ca52
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/displaymode
> @@ -0,0 +1,74 @@
> +videomode bindings
> +==
> +
> +Required properties:
> + - 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
In the case of interlaced formats, can the vfront-porch, vback-porch and
vsync-len
for the second field always be calculated from these values? Is that fully
standardized or can there be exceptions? struct v4l2_bt_timings has separate
fields
for these, but that may have been overkill.
> + - clock: displayclock in Hz
CEA-861 defined the option to slightly lower the clock frequency by 1.000/1.001
to
achieve frequencies like 29.97 or 59.94. In v4l2_bt_timings I made a flag for
that.
I'm not sure whether it is better to just set the clock to the correct frequency
(which is a weird value) or mark it with a bool.
> +
> +Optional properties:
> + - 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
> + - interlaced (bool): This is an interlaced mode
> + - doublescan (bool): This is a doublescan mode
> +
> +There are different ways of describing a display mode. The devicetree
> representation
> +corresponds to the one commonly found in datasheets for displays.
> +The description of the display and its mode is split in two parts: first the
> display
> +properties like size in mm and (optionally) multiple subnodes with the
> supported modes.
> +
> +Example:
> +
> + display@0 {
> + width-mm = <800>;
> + height-mm = <480>;
> + modes {
> + mode0: mode@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:
> +
> + mode1: mode@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 videomode can be linked to a connector via phandles. The connector has to
> +support the display- and default-mode-property to link to and select a mode.
> +
> +Example:
> +
> + hdmi@0012 {
> + status = "okay";
> + display = <&benq>;
> + default-mode = <&mode1>;
> + };
> +
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index dfba3e6..a3acaa3 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -83,4 +83,9 @@ config OF_MTD
> depends on MTD
> def_bool y
>
> +config OF_VIDEOMODE
> + def_bool y
> + help
> + helper to parse videomodes from the devicetree
> +
> endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index e027f44..80e6db3 100644
>
[PATCH v4] of: Add videomode helper
Hi,
(CC'ing the linux-media mailing list, as video modes are of interest there as
well)
On Wednesday 19 September 2012 12:19:22 Tomi Valkeinen wrote:
> On Wed, 2012-09-19 at 10:20 +0200, Steffen Trumtrar wrote:
> > This patch adds a helper function for parsing videomodes from the
> > devicetree. The videomode can be either converted to a struct
> > drm_display_mode or a struct fb_videomode.
> >
> > Signed-off-by: Sascha Hauer
> > Signed-off-by: Steffen Trumtrar
> > ---
> >
> > Hi!
> >
> > changes since v3:
> > - print error messages
> > - free alloced memory
> > - general cleanup
> >
> > Regards
> > Steffen
> >
> > .../devicetree/bindings/video/displaymode | 74 +
> > drivers/of/Kconfig |5 +
> > drivers/of/Makefile|1 +
> > drivers/of/of_videomode.c | 283 +++
> > include/linux/of_videomode.h | 56
> > 5 files changed, 419 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/video/displaymode
> > create mode 100644 drivers/of/of_videomode.c
> > create mode 100644 include/linux/of_videomode.h
> >
> > diff --git a/Documentation/devicetree/bindings/video/displaymode
> > b/Documentation/devicetree/bindings/video/displaymode new file mode
> > 100644
> > index 000..990ca52
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/displaymode
> > @@ -0,0 +1,74 @@
> > +videomode bindings
> > +==
> > +
> > +Required properties:
> > + - 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
> > +
> > +Optional properties:
> > + - 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
> > + - interlaced (bool): This is an interlaced mode
> > + - doublescan (bool): This is a doublescan mode
> > +
> > +There are different ways of describing a display mode. The devicetree
> > representation
> > +corresponds to the one commonly found in datasheets for displays.
> > +The description of the display and its mode is split in two parts: first
> > the display
> > +properties like size in mm and (optionally) multiple subnodes with the
> > supported modes.
> > +
> > +Example:
> > +
> > + display at 0 {
> > + width-mm = <800>;
> > + height-mm = <480>;
> > + modes {
> > + mode0: mode 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:
> > +
> > + mode1: mode 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 videomode can be linked to a connector via phandles. The connector
> > has to
> > +support the display- and default-mode-property to link to and select a
> > mode.
> > +
> > +Example:
> > +
> > + hdmi at 0012 {
> > + status = "okay";
> > + display = <&benq>;
> > + default-mode = <&mode1>;
> > + };
> > +
> > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> > index dfba3e6..a3acaa3 100644
> > --- a/drivers/of/Kconfig
> > +++ b/drivers/of/Kconfig
> > @@ -83,4 +83,9 @@ config OF_MTD
> >
> > depends on MTD
> > def_bool y
> >
> > +config OF_VIDEOMODE
> > + def_bool y
> > + help
> > + helper to parse videomodes from the devicetree
> > +
> >
> > endmenu # OF
> >
> > diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> > index e027f44..80e6db3 100644
> > --- a/drivers/of/Makefile
> > +++ b/drivers/of/Makefile
> > @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
> >
> > obj-$(CONFIG_OF_PCI) += of_pci.o
> > obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
> > obj-$(CONFIG_OF_MTD)
Re: [PATCH v4] of: Add videomode helper
Hi,
(CC'ing the linux-media mailing list, as video modes are of interest there as
well)
On Wednesday 19 September 2012 12:19:22 Tomi Valkeinen wrote:
> On Wed, 2012-09-19 at 10:20 +0200, Steffen Trumtrar wrote:
> > This patch adds a helper function for parsing videomodes from the
> > devicetree. The videomode can be either converted to a struct
> > drm_display_mode or a struct fb_videomode.
> >
> > Signed-off-by: Sascha Hauer
> > Signed-off-by: Steffen Trumtrar
> > ---
> >
> > Hi!
> >
> > changes since v3:
> > - print error messages
> > - free alloced memory
> > - general cleanup
> >
> > Regards
> > Steffen
> >
> > .../devicetree/bindings/video/displaymode | 74 +
> > drivers/of/Kconfig |5 +
> > drivers/of/Makefile|1 +
> > drivers/of/of_videomode.c | 283 +++
> > include/linux/of_videomode.h | 56
> > 5 files changed, 419 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/video/displaymode
> > create mode 100644 drivers/of/of_videomode.c
> > create mode 100644 include/linux/of_videomode.h
> >
> > diff --git a/Documentation/devicetree/bindings/video/displaymode
> > b/Documentation/devicetree/bindings/video/displaymode new file mode
> > 100644
> > index 000..990ca52
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/displaymode
> > @@ -0,0 +1,74 @@
> > +videomode bindings
> > +==
> > +
> > +Required properties:
> > + - 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
> > +
> > +Optional properties:
> > + - 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
> > + - interlaced (bool): This is an interlaced mode
> > + - doublescan (bool): This is a doublescan mode
> > +
> > +There are different ways of describing a display mode. The devicetree
> > representation
> > +corresponds to the one commonly found in datasheets for displays.
> > +The description of the display and its mode is split in two parts: first
> > the display
> > +properties like size in mm and (optionally) multiple subnodes with the
> > supported modes.
> > +
> > +Example:
> > +
> > + display@0 {
> > + width-mm = <800>;
> > + height-mm = <480>;
> > + modes {
> > + mode0: mode@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:
> > +
> > + mode1: mode@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 videomode can be linked to a connector via phandles. The connector
> > has to
> > +support the display- and default-mode-property to link to and select a
> > mode.
> > +
> > +Example:
> > +
> > + hdmi@0012 {
> > + status = "okay";
> > + display = <&benq>;
> > + default-mode = <&mode1>;
> > + };
> > +
> > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> > index dfba3e6..a3acaa3 100644
> > --- a/drivers/of/Kconfig
> > +++ b/drivers/of/Kconfig
> > @@ -83,4 +83,9 @@ config OF_MTD
> >
> > depends on MTD
> > def_bool y
> >
> > +config OF_VIDEOMODE
> > + def_bool y
> > + help
> > + helper to parse videomodes from the devicetree
> > +
> >
> > endmenu # OF
> >
> > diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> > index e027f44..80e6db3 100644
> > --- a/drivers/of/Makefile
> > +++ b/drivers/of/Makefile
> > @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
> >
> > obj-$(CONFIG_OF_PCI) += of_pci.o
> > obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
> > obj-$(CONFIG_OF_MTD) += of_m
[PATCH v4] of: Add videomode helper
This patch adds a helper function for parsing videomodes from the devicetree.
The videomode can be either converted to a struct drm_display_mode or a
struct fb_videomode.
Signed-off-by: Sascha Hauer
Signed-off-by: Steffen Trumtrar
---
Hi!
changes since v3:
- print error messages
- free alloced memory
- general cleanup
Regards
Steffen
.../devicetree/bindings/video/displaymode | 74 +
drivers/of/Kconfig |5 +
drivers/of/Makefile|1 +
drivers/of/of_videomode.c | 283
include/linux/of_videomode.h | 56
5 files changed, 419 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/displaymode
create mode 100644 drivers/of/of_videomode.c
create mode 100644 include/linux/of_videomode.h
diff --git a/Documentation/devicetree/bindings/video/displaymode
b/Documentation/devicetree/bindings/video/displaymode
new file mode 100644
index 000..990ca52
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/displaymode
@@ -0,0 +1,74 @@
+videomode bindings
+==
+
+Required properties:
+ - 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
+
+Optional properties:
+ - 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
+ - interlaced (bool): This is an interlaced mode
+ - doublescan (bool): This is a doublescan mode
+
+There are different ways of describing a display mode. The devicetree
representation
+corresponds to the one commonly found in datasheets for displays.
+The description of the display and its mode is split in two parts: first the
display
+properties like size in mm and (optionally) multiple subnodes with the
supported modes.
+
+Example:
+
+ display@0 {
+ width-mm = <800>;
+ height-mm = <480>;
+ modes {
+ mode0: mode@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:
+
+ mode1: mode@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 videomode can be linked to a connector via phandles. The connector has to
+support the display- and default-mode-property to link to and select a mode.
+
+Example:
+
+ hdmi@0012 {
+ status = "okay";
+ display = <&benq>;
+ default-mode = <&mode1>;
+ };
+
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index dfba3e6..a3acaa3 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -83,4 +83,9 @@ config OF_MTD
depends on MTD
def_bool y
+config OF_VIDEOMODE
+ def_bool y
+ help
+ helper to parse videomodes from the devicetree
+
endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index e027f44..80e6db3 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
obj-$(CONFIG_OF_MTD) += of_mtd.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
new file mode 100644
index 000..52bfc74
--- /dev/null
+++ b/drivers/of/of_videomode.c
@@ -0,0 +1,283 @@
+/*
+ * OF helpers for parsing display modes
+ *
+ * Copyright (c) 2012 Sascha Hauer , Pengutronix
+ * Copyright (c) 2012 Steffen Trumtrar , Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static u32 of_video_get_value(struct mode_property *prop)
+{
+ return (prop->min >= prop->ty
[PATCH v4] of: Add videomode helper
On Wed, 2012-09-19 at 10:20 +0200, Steffen Trumtrar wrote:
> This patch adds a helper function for parsing videomodes from the devicetree.
> The videomode can be either converted to a struct drm_display_mode or a
> struct fb_videomode.
>
> Signed-off-by: Sascha Hauer
> Signed-off-by: Steffen Trumtrar
> ---
>
> Hi!
>
> changes since v3:
> - print error messages
> - free alloced memory
> - general cleanup
>
> Regards
> Steffen
>
> .../devicetree/bindings/video/displaymode | 74 +
> drivers/of/Kconfig |5 +
> drivers/of/Makefile|1 +
> drivers/of/of_videomode.c | 283
>
> include/linux/of_videomode.h | 56
> 5 files changed, 419 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/displaymode
> create mode 100644 drivers/of/of_videomode.c
> create mode 100644 include/linux/of_videomode.h
>
> diff --git a/Documentation/devicetree/bindings/video/displaymode
> b/Documentation/devicetree/bindings/video/displaymode
> new file mode 100644
> index 000..990ca52
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/displaymode
> @@ -0,0 +1,74 @@
> +videomode bindings
> +==
> +
> +Required properties:
> + - 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
> +
> +Optional properties:
> + - 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
> + - interlaced (bool): This is an interlaced mode
> + - doublescan (bool): This is a doublescan mode
> +
> +There are different ways of describing a display mode. The devicetree
> representation
> +corresponds to the one commonly found in datasheets for displays.
> +The description of the display and its mode is split in two parts: first the
> display
> +properties like size in mm and (optionally) multiple subnodes with the
> supported modes.
> +
> +Example:
> +
> + display at 0 {
> + width-mm = <800>;
> + height-mm = <480>;
> + modes {
> + mode0: mode 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:
> +
> + mode1: mode 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 videomode can be linked to a connector via phandles. The connector has to
> +support the display- and default-mode-property to link to and select a mode.
> +
> +Example:
> +
> + hdmi at 0012 {
> + status = "okay";
> + display = <&benq>;
> + default-mode = <&mode1>;
> + };
> +
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index dfba3e6..a3acaa3 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -83,4 +83,9 @@ config OF_MTD
> depends on MTD
> def_bool y
>
> +config OF_VIDEOMODE
> + def_bool y
> + help
> + helper to parse videomodes from the devicetree
> +
> endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index e027f44..80e6db3 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
> obj-$(CONFIG_OF_PCI) += of_pci.o
> obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
> obj-$(CONFIG_OF_MTD) += of_mtd.o
> +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
> diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
> new file mode 100644
> index 000..52bfc74
> --- /dev/null
> +++ b/drivers/of/of_videomode.c
> @@ -0,0 +1,283 @@
> +/*
> + * OF helpers for parsing display modes
> + *
> + * Copyright (c) 2012 Sascha Hauer , Pengutronix
>
[PATCH v4] of: Add videomode helper
This patch adds a helper function for parsing videomodes from the devicetree.
The videomode can be either converted to a struct drm_display_mode or a
struct fb_videomode.
Signed-off-by: Sascha Hauer
Signed-off-by: Steffen Trumtrar
---
Hi!
changes since v3:
- print error messages
- free alloced memory
- general cleanup
Regards
Steffen
.../devicetree/bindings/video/displaymode | 74 +
drivers/of/Kconfig |5 +
drivers/of/Makefile|1 +
drivers/of/of_videomode.c | 283
include/linux/of_videomode.h | 56
5 files changed, 419 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/displaymode
create mode 100644 drivers/of/of_videomode.c
create mode 100644 include/linux/of_videomode.h
diff --git a/Documentation/devicetree/bindings/video/displaymode
b/Documentation/devicetree/bindings/video/displaymode
new file mode 100644
index 000..990ca52
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/displaymode
@@ -0,0 +1,74 @@
+videomode bindings
+==
+
+Required properties:
+ - 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
+
+Optional properties:
+ - 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
+ - interlaced (bool): This is an interlaced mode
+ - doublescan (bool): This is a doublescan mode
+
+There are different ways of describing a display mode. The devicetree
representation
+corresponds to the one commonly found in datasheets for displays.
+The description of the display and its mode is split in two parts: first the
display
+properties like size in mm and (optionally) multiple subnodes with the
supported modes.
+
+Example:
+
+ display at 0 {
+ width-mm = <800>;
+ height-mm = <480>;
+ modes {
+ mode0: mode 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:
+
+ mode1: mode 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 videomode can be linked to a connector via phandles. The connector has to
+support the display- and default-mode-property to link to and select a mode.
+
+Example:
+
+ hdmi at 0012 {
+ status = "okay";
+ display = <&benq>;
+ default-mode = <&mode1>;
+ };
+
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index dfba3e6..a3acaa3 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -83,4 +83,9 @@ config OF_MTD
depends on MTD
def_bool y
+config OF_VIDEOMODE
+ def_bool y
+ help
+ helper to parse videomodes from the devicetree
+
endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index e027f44..80e6db3 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
obj-$(CONFIG_OF_MTD) += of_mtd.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
new file mode 100644
index 000..52bfc74
--- /dev/null
+++ b/drivers/of/of_videomode.c
@@ -0,0 +1,283 @@
+/*
+ * OF helpers for parsing display modes
+ *
+ * Copyright (c) 2012 Sascha Hauer , Pengutronix
+ * Copyright (c) 2012 Steffen Trumtrar ,
Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static u32 of_video_get_value(struct mode_property *prop)
+{
+ return (prop->min
Re: [PATCH v4] of: Add videomode helper
On Wed, 2012-09-19 at 10:20 +0200, Steffen Trumtrar wrote:
> This patch adds a helper function for parsing videomodes from the devicetree.
> The videomode can be either converted to a struct drm_display_mode or a
> struct fb_videomode.
>
> Signed-off-by: Sascha Hauer
> Signed-off-by: Steffen Trumtrar
> ---
>
> Hi!
>
> changes since v3:
> - print error messages
> - free alloced memory
> - general cleanup
>
> Regards
> Steffen
>
> .../devicetree/bindings/video/displaymode | 74 +
> drivers/of/Kconfig |5 +
> drivers/of/Makefile|1 +
> drivers/of/of_videomode.c | 283
>
> include/linux/of_videomode.h | 56
> 5 files changed, 419 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/displaymode
> create mode 100644 drivers/of/of_videomode.c
> create mode 100644 include/linux/of_videomode.h
>
> diff --git a/Documentation/devicetree/bindings/video/displaymode
> b/Documentation/devicetree/bindings/video/displaymode
> new file mode 100644
> index 000..990ca52
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/displaymode
> @@ -0,0 +1,74 @@
> +videomode bindings
> +==
> +
> +Required properties:
> + - 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
> +
> +Optional properties:
> + - 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
> + - interlaced (bool): This is an interlaced mode
> + - doublescan (bool): This is a doublescan mode
> +
> +There are different ways of describing a display mode. The devicetree
> representation
> +corresponds to the one commonly found in datasheets for displays.
> +The description of the display and its mode is split in two parts: first the
> display
> +properties like size in mm and (optionally) multiple subnodes with the
> supported modes.
> +
> +Example:
> +
> + display@0 {
> + width-mm = <800>;
> + height-mm = <480>;
> + modes {
> + mode0: mode@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:
> +
> + mode1: mode@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 videomode can be linked to a connector via phandles. The connector has to
> +support the display- and default-mode-property to link to and select a mode.
> +
> +Example:
> +
> + hdmi@0012 {
> + status = "okay";
> + display = <&benq>;
> + default-mode = <&mode1>;
> + };
> +
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index dfba3e6..a3acaa3 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -83,4 +83,9 @@ config OF_MTD
> depends on MTD
> def_bool y
>
> +config OF_VIDEOMODE
> + def_bool y
> + help
> + helper to parse videomodes from the devicetree
> +
> endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index e027f44..80e6db3 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
> obj-$(CONFIG_OF_PCI) += of_pci.o
> obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
> obj-$(CONFIG_OF_MTD) += of_mtd.o
> +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
> diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
> new file mode 100644
> index 000..52bfc74
> --- /dev/null
> +++ b/drivers/of/of_videomode.c
> @@ -0,0 +1,283 @@
> +/*
> + * OF helpers for parsing display modes
> + *
> + * Copyright (c) 2012 Sascha Hauer , Pengutronix
> + * Copyrig
