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:

quoted

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 <s.hauer@pengutronix.de>
Signed-off-by: Steffen Trumtrar <redacted>
---

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

quoted

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

quoted

+		width-mm = <800>;
+		height-mm = <480>;
+		modes {
+			mode0: mode@0 {
+				/* 1920x1080p24 */
+				clock = <52000000>;
+				hactive = <1920>;
+				vactive = <1080>;
+				hfront-porch = <25>;
+				hback-porch = <25>;
+				hsync-len = <25>;
+				vback-porch = <2>;
+				vfront-porch = <2>;
+				vsync-len = <2>;
+				hsync-active-high;
+			};
+		};
+	};
+
+Every property also supports the use of ranges, so the commonly used
datasheet +description with <min typ max>-tuples can be used.
+
+Example:
+
+	mode1: mode@1 {
+		/* 1920x1080p24 */
+		clock = <148500000>;
+		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

quoted

+
+Example:
+
+	hdmi@00120000 {
+		status = "okay";
+		display = <&benq>;
+		default-mode = <&mode1>;
+	};
+

-- 
Regards,

Laurent Pinchart