On 09/24/2012 10:45 AM, Stephen Warren wrote:

On 09/24/2012 07:42 AM, Rob Herring wrote:

quoted

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.

quoted

quoted

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

quoted

quoted

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

quoted

+		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

quoted

quoted

+		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;
+			};
+		};
+	};