All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v10 0/6] of: add display helper
@ 2012-11-15  9:23 ` Steffen Trumtrar
  0 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Hi!

Changes since v9:
	- don't leak memory when previous timings were correct
	- CodingStyle fixes
	- move blank lines around

Regards,
Steffen


Steffen Trumtrar (6):
  video: add display_timing and videomode
  video: add of helper for videomode
  fbmon: add videomode helpers
  fbmon: add of_videomode helpers
  drm_modes: add videomode helpers
  drm_modes: add of_videomode helpers

 .../devicetree/bindings/video/display-timings.txt  |  107 ++++++++++
 drivers/gpu/drm/drm_modes.c                        |   70 +++++++
 drivers/video/Kconfig                              |   19 ++
 drivers/video/Makefile                             |    4 +
 drivers/video/display_timing.c                     |   24 +++
 drivers/video/fbmon.c                              |   86 ++++++++
 drivers/video/of_display_timing.c                  |  212 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   47 +++++
 drivers/video/videomode.c                          |   45 +++++
 include/drm/drmP.h                                 |   12 ++
 include/linux/display_timing.h                     |   69 +++++++
 include/linux/fb.h                                 |   12 ++
 include/linux/of_display_timings.h                 |   20 ++
 include/linux/of_videomode.h                       |   17 ++
 include/linux/videomode.h                          |   40 ++++
 15 files changed, 784 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/linux/display_timing.h
 create mode 100644 include/linux/of_display_timings.h
 create mode 100644 include/linux/of_videomode.h
 create mode 100644 include/linux/videomode.h

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v10 0/6] of: add display helper
@ 2012-11-15  9:23 ` Steffen Trumtrar
  0 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Hi!

Changes since v9:
	- don't leak memory when previous timings were correct
	- CodingStyle fixes
	- move blank lines around

Regards,
Steffen


Steffen Trumtrar (6):
  video: add display_timing and videomode
  video: add of helper for videomode
  fbmon: add videomode helpers
  fbmon: add of_videomode helpers
  drm_modes: add videomode helpers
  drm_modes: add of_videomode helpers

 .../devicetree/bindings/video/display-timings.txt  |  107 ++++++++++
 drivers/gpu/drm/drm_modes.c                        |   70 +++++++
 drivers/video/Kconfig                              |   19 ++
 drivers/video/Makefile                             |    4 +
 drivers/video/display_timing.c                     |   24 +++
 drivers/video/fbmon.c                              |   86 ++++++++
 drivers/video/of_display_timing.c                  |  212 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   47 +++++
 drivers/video/videomode.c                          |   45 +++++
 include/drm/drmP.h                                 |   12 ++
 include/linux/display_timing.h                     |   69 +++++++
 include/linux/fb.h                                 |   12 ++
 include/linux/of_display_timings.h                 |   20 ++
 include/linux/of_videomode.h                       |   17 ++
 include/linux/videomode.h                          |   40 ++++
 15 files changed, 784 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/linux/display_timing.h
 create mode 100644 include/linux/of_display_timings.h
 create mode 100644 include/linux/of_videomode.h
 create mode 100644 include/linux/videomode.h

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v10 1/6] video: add display_timing and videomode
  2012-11-15  9:23 ` Steffen Trumtrar
@ 2012-11-15  9:23   ` Steffen Trumtrar
  -1 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Add display_timing structure and the according helper functions. This allows
the description of a display via its supported timing parameters.

Every timing parameter can be specified as a single value or a range
<min typ max>.

Also, add helper functions to convert from display timings to a generic videomode
structure. This videomode can then be converted to the corresponding subsystem
mode representation (e.g. fb_videomode).

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/Kconfig          |    6 ++++
 drivers/video/Makefile         |    2 ++
 drivers/video/display_timing.c |   24 ++++++++++++++
 drivers/video/videomode.c      |   45 ++++++++++++++++++++++++++
 include/linux/display_timing.h |   69 ++++++++++++++++++++++++++++++++++++++++
 include/linux/videomode.h      |   40 +++++++++++++++++++++++
 6 files changed, 186 insertions(+)
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/linux/display_timing.h
 create mode 100644 include/linux/videomode.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d08d799..2a23b18 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -33,6 +33,12 @@ config VIDEO_OUTPUT_CONTROL
 	  This framework adds support for low-level control of the video 
 	  output switch.
 
+config DISPLAY_TIMING
+       bool
+
+config VIDEOMODE
+       bool
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 23e948e..fc30439 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -167,3 +167,5 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_VIDEOMODE) += videomode.o
diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
new file mode 100644
index 0000000..ac9bbbc
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/display_timing.h>
+#include <linux/export.h>
+#include <linux/slab.h>
+
+void display_timings_release(struct display_timings *disp)
+{
+	if (disp->timings) {
+		unsigned int i;
+
+		for (i = 0; i < disp->num_timings; i++)
+			kfree(disp->timings[i]);
+		kfree(disp->timings);
+	}
+	kfree(disp);
+}
+EXPORT_SYMBOL_GPL(display_timings_release);
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 0000000..087374a
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,45 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/export.h>
+#include <linux/errno.h>
+#include <linux/display_timing.h>
+#include <linux/kernel.h>
+#include <linux/videomode.h>
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+			  unsigned int index)
+{
+	struct display_timing *dt;
+
+	dt = display_timings_get(disp, index);
+	if (!dt)
+		return -EINVAL;
+
+	vm->pixelclock = display_timing_get_value(&dt->pixelclock, 0);
+	vm->hactive = display_timing_get_value(&dt->hactive, 0);
+	vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, 0);
+	vm->hback_porch = display_timing_get_value(&dt->hback_porch, 0);
+	vm->hsync_len = display_timing_get_value(&dt->hsync_len, 0);
+
+	vm->vactive = display_timing_get_value(&dt->vactive, 0);
+	vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, 0);
+	vm->vback_porch = display_timing_get_value(&dt->vback_porch, 0);
+	vm->vsync_len = display_timing_get_value(&dt->vsync_len, 0);
+
+	vm->vah = dt->vsync_pol_active;
+	vm->hah = dt->hsync_pol_active;
+	vm->de = dt->de_pol_active;
+	vm->pixelclk_pol = dt->pixelclk_pol;
+
+	vm->interlaced = dt->interlaced;
+	vm->doublescan = dt->doublescan;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_from_timing);
diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
new file mode 100644
index 0000000..caee2a8
--- /dev/null
+++ b/include/linux/display_timing.h
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * description of display timings
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_DISPLAY_TIMINGS_H
+#define __LINUX_DISPLAY_TIMINGS_H
+
+#include <linux/types.h>
+
+struct timing_entry {
+	u32 min;
+	u32 typ;
+	u32 max;
+};
+
+struct display_timing {
+	struct timing_entry pixelclock;
+
+	struct timing_entry hactive;
+	struct timing_entry hfront_porch;
+	struct timing_entry hback_porch;
+	struct timing_entry hsync_len;
+
+	struct timing_entry vactive;
+	struct timing_entry vfront_porch;
+	struct timing_entry vback_porch;
+	struct timing_entry vsync_len;
+
+	unsigned int vsync_pol_active;
+	unsigned int hsync_pol_active;
+	unsigned int de_pol_active;
+	unsigned int pixelclk_pol;
+	bool interlaced;
+	bool doublescan;
+};
+
+struct display_timings {
+	unsigned int num_timings;
+	unsigned int native_mode;
+
+	struct display_timing **timings;
+};
+
+/*
+ * placeholder function until ranges are really needed
+ * the index parameter should then be used to select one of [min typ max]
+ */
+static inline u32 display_timing_get_value(struct timing_entry *te,
+					   unsigned int index)
+{
+	return te->typ;
+}
+
+static inline struct display_timing *display_timings_get(struct display_timings *disp,
+							 unsigned int index)
+{
+	if (disp->num_timings > index)
+		return disp->timings[index];
+	else
+		return NULL;
+}
+
+void display_timings_release(struct display_timings *disp);
+
+#endif
diff --git a/include/linux/videomode.h b/include/linux/videomode.h
new file mode 100644
index 0000000..704db7b
--- /dev/null
+++ b/include/linux/videomode.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * generic videomode description
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_VIDEOMODE_H
+#define __LINUX_VIDEOMODE_H
+
+#include <linux/display_timing.h>
+
+struct videomode {
+	u32 pixelclock;
+	u32 refreshrate;
+
+	u32 hactive;
+	u32 hfront_porch;
+	u32 hback_porch;
+	u32 hsync_len;
+
+	u32 vactive;
+	u32 vfront_porch;
+	u32 vback_porch;
+	u32 vsync_len;
+
+	u32 hah;
+	u32 vah;
+	u32 de;
+	u32 pixelclk_pol;
+
+	bool interlaced;
+	bool doublescan;
+};
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+			  unsigned int index);
+
+#endif
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v10 1/6] video: add display_timing and videomode
@ 2012-11-15  9:23   ` Steffen Trumtrar
  0 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Add display_timing structure and the according helper functions. This allows
the description of a display via its supported timing parameters.

Every timing parameter can be specified as a single value or a range
<min typ max>.

Also, add helper functions to convert from display timings to a generic videomode
structure. This videomode can then be converted to the corresponding subsystem
mode representation (e.g. fb_videomode).

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/Kconfig          |    6 ++++
 drivers/video/Makefile         |    2 ++
 drivers/video/display_timing.c |   24 ++++++++++++++
 drivers/video/videomode.c      |   45 ++++++++++++++++++++++++++
 include/linux/display_timing.h |   69 ++++++++++++++++++++++++++++++++++++++++
 include/linux/videomode.h      |   40 +++++++++++++++++++++++
 6 files changed, 186 insertions(+)
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/linux/display_timing.h
 create mode 100644 include/linux/videomode.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d08d799..2a23b18 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -33,6 +33,12 @@ config VIDEO_OUTPUT_CONTROL
 	  This framework adds support for low-level control of the video 
 	  output switch.
 
+config DISPLAY_TIMING
+       bool
+
+config VIDEOMODE
+       bool
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 23e948e..fc30439 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -167,3 +167,5 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_VIDEOMODE) += videomode.o
diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
new file mode 100644
index 0000000..ac9bbbc
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/display_timing.h>
+#include <linux/export.h>
+#include <linux/slab.h>
+
+void display_timings_release(struct display_timings *disp)
+{
+	if (disp->timings) {
+		unsigned int i;
+
+		for (i = 0; i < disp->num_timings; i++)
+			kfree(disp->timings[i]);
+		kfree(disp->timings);
+	}
+	kfree(disp);
+}
+EXPORT_SYMBOL_GPL(display_timings_release);
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 0000000..087374a
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,45 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/export.h>
+#include <linux/errno.h>
+#include <linux/display_timing.h>
+#include <linux/kernel.h>
+#include <linux/videomode.h>
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+			  unsigned int index)
+{
+	struct display_timing *dt;
+
+	dt = display_timings_get(disp, index);
+	if (!dt)
+		return -EINVAL;
+
+	vm->pixelclock = display_timing_get_value(&dt->pixelclock, 0);
+	vm->hactive = display_timing_get_value(&dt->hactive, 0);
+	vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, 0);
+	vm->hback_porch = display_timing_get_value(&dt->hback_porch, 0);
+	vm->hsync_len = display_timing_get_value(&dt->hsync_len, 0);
+
+	vm->vactive = display_timing_get_value(&dt->vactive, 0);
+	vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, 0);
+	vm->vback_porch = display_timing_get_value(&dt->vback_porch, 0);
+	vm->vsync_len = display_timing_get_value(&dt->vsync_len, 0);
+
+	vm->vah = dt->vsync_pol_active;
+	vm->hah = dt->hsync_pol_active;
+	vm->de = dt->de_pol_active;
+	vm->pixelclk_pol = dt->pixelclk_pol;
+
+	vm->interlaced = dt->interlaced;
+	vm->doublescan = dt->doublescan;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_from_timing);
diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
new file mode 100644
index 0000000..caee2a8
--- /dev/null
+++ b/include/linux/display_timing.h
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * description of display timings
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_DISPLAY_TIMINGS_H
+#define __LINUX_DISPLAY_TIMINGS_H
+
+#include <linux/types.h>
+
+struct timing_entry {
+	u32 min;
+	u32 typ;
+	u32 max;
+};
+
+struct display_timing {
+	struct timing_entry pixelclock;
+
+	struct timing_entry hactive;
+	struct timing_entry hfront_porch;
+	struct timing_entry hback_porch;
+	struct timing_entry hsync_len;
+
+	struct timing_entry vactive;
+	struct timing_entry vfront_porch;
+	struct timing_entry vback_porch;
+	struct timing_entry vsync_len;
+
+	unsigned int vsync_pol_active;
+	unsigned int hsync_pol_active;
+	unsigned int de_pol_active;
+	unsigned int pixelclk_pol;
+	bool interlaced;
+	bool doublescan;
+};
+
+struct display_timings {
+	unsigned int num_timings;
+	unsigned int native_mode;
+
+	struct display_timing **timings;
+};
+
+/*
+ * placeholder function until ranges are really needed
+ * the index parameter should then be used to select one of [min typ max]
+ */
+static inline u32 display_timing_get_value(struct timing_entry *te,
+					   unsigned int index)
+{
+	return te->typ;
+}
+
+static inline struct display_timing *display_timings_get(struct display_timings *disp,
+							 unsigned int index)
+{
+	if (disp->num_timings > index)
+		return disp->timings[index];
+	else
+		return NULL;
+}
+
+void display_timings_release(struct display_timings *disp);
+
+#endif
diff --git a/include/linux/videomode.h b/include/linux/videomode.h
new file mode 100644
index 0000000..704db7b
--- /dev/null
+++ b/include/linux/videomode.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * generic videomode description
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_VIDEOMODE_H
+#define __LINUX_VIDEOMODE_H
+
+#include <linux/display_timing.h>
+
+struct videomode {
+	u32 pixelclock;
+	u32 refreshrate;
+
+	u32 hactive;
+	u32 hfront_porch;
+	u32 hback_porch;
+	u32 hsync_len;
+
+	u32 vactive;
+	u32 vfront_porch;
+	u32 vback_porch;
+	u32 vsync_len;
+
+	u32 hah;
+	u32 vah;
+	u32 de;
+	u32 pixelclk_pol;
+
+	bool interlaced;
+	bool doublescan;
+};
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+			  unsigned int index);
+
+#endif
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v10 2/6] video: add of helper for videomode
  2012-11-15  9:23 ` Steffen Trumtrar
@ 2012-11-15  9:23   ` Steffen Trumtrar
  -1 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Philipp Zabel, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Tomi Valkeinen,
	Stephen Warren, kernel

This adds support for reading display timings from DT or/and convert one of those
timings to a videomode.
The of_display_timing implementation supports multiple children where each
property can have up to 3 values. All children are read into an array, that
can be queried.
of_get_videomode converts exactly one of that timings to a struct videomode.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Stephen Warren <swarren@nvidia.com>
---
 .../devicetree/bindings/video/display-timings.txt  |  107 ++++++++++
 drivers/video/Kconfig                              |   13 ++
 drivers/video/Makefile                             |    2 +
 drivers/video/of_display_timing.c                  |  212 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   47 +++++
 include/linux/of_display_timings.h                 |   20 ++
 include/linux/of_videomode.h                       |   17 ++
 7 files changed, 418 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 include/linux/of_display_timings.h
 create mode 100644 include/linux/of_videomode.h

diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt
new file mode 100644
index 0000000..a05cade
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timings.txt
@@ -0,0 +1,107 @@
+display-timings bindings
+========================
+
+display-timings node
+--------------------
+
+required properties:
+ - none
+
+optional properties:
+ - native-mode: The native mode for the display, in case multiple modes are
+		provided. When omitted, assume the first node is the native.
+
+timings subnode
+---------------
+
+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-frequency: display clock in Hz
+
+optional properties:
+ - hsync-active: Hsync pulse is active low/high/ignored
+ - vsync-active: Vsync pulse is active low/high/ignored
+ - de-active: Data-Enable pulse is active low/high/ignored
+ - pixelclk-inverted: pixelclock is inverted/non-inverted/ignored
+ - interlaced (bool)
+ - doublescan (bool)
+
+All the optional properties that are not bool follow the following logic:
+    <1>: high active
+    <0>: low active
+    omitted: not used on hardware
+
+There are different ways of describing the capabilities of a display. The devicetree
+representation corresponds to the one commonly found in datasheets for displays.
+If a display supports multiple signal timings, the native-mode can be specified.
+
+The parameters are defined as
+
+struct display_timing
+=====================
+
+  +----------+---------------------------------------------+----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |vback_porch                 |          |       |
+  |          |                ↓                            |          |       |
+  +----------###############################################----------+-------+
+  |          #                ↑                            #          |       |
+  |          #                |                            #          |       |
+  |  hback   #                |                            #  hfront  | hsync |
+  |   porch  #                |       hactive              #  porch   |  len  |
+  |<-------->#<---------------+--------------------------->#<-------->|<----->|
+  |          #                |                            #          |       |
+  |          #                |vactive                     #          |       |
+  |          #                |                            #          |       |
+  |          #                ↓                            #          |       |
+  +----------###############################################----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |vfront_porch                |          |       |
+  |          |                ↓                            |          |       |
+  +----------+---------------------------------------------+----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |vsync_len                   |          |       |
+  |          |                ↓                            |          |       |
+  +----------+---------------------------------------------+----------+-------+
+
+
+Example:
+
+	display-timings {
+		native-mode = <&timing0>;
+		timing0: 1920p24 {
+			/* 1920x1080p24 */
+			clock-frequency = <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 = <1>;
+		};
+	};
+
+Every required property also supports the use of ranges, so the commonly used
+datasheet description with <min typ max>-tuples can be used.
+
+Example:
+
+	timing1: timing {
+		/* 1920x1080p24 */
+		clock-frequency = <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>;
+	};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2a23b18..a324457 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -39,6 +39,19 @@ config DISPLAY_TIMING
 config VIDEOMODE
        bool
 
+config OF_DISPLAY_TIMING
+	def_bool y
+	select DISPLAY_TIMING
+	help
+	  helper to parse display timings from the devicetree
+
+config OF_VIDEOMODE
+	def_bool y
+	select VIDEOMODE
+	select OF_DISPLAY_TIMING
+	help
+	  helper to get videomodes from the devicetree
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index fc30439..b936b00 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -168,4 +168,6 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
 obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_OF_DISPLAY_TIMING) += of_display_timing.o
 obj-$(CONFIG_VIDEOMODE) += videomode.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
new file mode 100644
index 0000000..b187f31
--- /dev/null
+++ b/drivers/video/of_display_timing.c
@@ -0,0 +1,212 @@
+/*
+ * OF helpers for parsing display timings
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * based on of_videomode.c by Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/export.h>
+#include <linux/of_display_timings.h>
+
+/**
+ * parse_property - parse timing_entry from device_node
+ * @np: device_node with the property
+ * @name: name of the property
+ * @result: will be set to the return value
+ *
+ * DESCRIPTION:
+ * Every display_timing can be specified with either just the typical value or
+ * a range consisting of min/typ/max. This function helps handling this
+ **/
+static int parse_property(struct device_node *np, char *name,
+				struct timing_entry *result)
+{
+	struct property *prop;
+	int length, cells, ret;
+
+	prop = of_find_property(np, name, &length);
+	if (!prop) {
+		pr_err("%s: could not find property %s\n", __func__, name);
+		return -EINVAL;
+	}
+
+	cells = length / sizeof(u32);
+	if (cells == 1) {
+		ret = of_property_read_u32(np, name, &result->typ);
+		result->min = result->typ;
+		result->max = result->typ;
+	} else if (cells == 3) {
+		ret = of_property_read_u32_array(np, name, &result->min, cells);
+	} else {
+		pr_err("%s: illegal timing specification in %s\n", __func__, name);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+/**
+ * of_get_display_timing - parse display_timing entry from device_node
+ * @np: device_node with the properties
+ **/
+static struct display_timing *of_get_display_timing(struct device_node *np)
+{
+	struct display_timing *dt;
+	int ret = 0;
+
+	dt = kzalloc(sizeof(*dt), GFP_KERNEL);
+	if (!dt) {
+		pr_err("%s: could not allocate display_timing struct\n", __func__);
+		return NULL;
+	}
+
+	ret |= parse_property(np, "hback-porch", &dt->hback_porch);
+	ret |= parse_property(np, "hfront-porch", &dt->hfront_porch);
+	ret |= parse_property(np, "hactive", &dt->hactive);
+	ret |= parse_property(np, "hsync-len", &dt->hsync_len);
+	ret |= parse_property(np, "vback-porch", &dt->vback_porch);
+	ret |= parse_property(np, "vfront-porch", &dt->vfront_porch);
+	ret |= parse_property(np, "vactive", &dt->vactive);
+	ret |= parse_property(np, "vsync-len", &dt->vsync_len);
+	ret |= parse_property(np, "clock-frequency", &dt->pixelclock);
+
+	of_property_read_u32(np, "vsync-active", &dt->vsync_pol_active);
+	of_property_read_u32(np, "hsync-active", &dt->hsync_pol_active);
+	of_property_read_u32(np, "de-active", &dt->de_pol_active);
+	of_property_read_u32(np, "pixelclk-inverted", &dt->pixelclk_pol);
+	dt->interlaced = of_property_read_bool(np, "interlaced");
+	dt->doublescan = of_property_read_bool(np, "doublescan");
+
+	if (ret) {
+		pr_err("%s: error reading timing properties\n", __func__);
+		kfree(dt);
+		return NULL;
+	}
+
+	return dt;
+}
+
+/**
+ * of_get_display_timings - parse all display_timing entries from a device_node
+ * @np: device_node with the subnodes
+ **/
+struct display_timings *of_get_display_timings(struct device_node *np)
+{
+	struct device_node *timings_np;
+	struct device_node *entry;
+	struct device_node *native_mode;
+	struct display_timings *disp;
+
+	if (!np) {
+		pr_err("%s: no devicenode given\n", __func__);
+		return NULL;
+	}
+
+	timings_np = of_find_node_by_name(np, "display-timings");
+	if (!timings_np) {
+		pr_err("%s: could not find display-timings node\n", __func__);
+		return NULL;
+	}
+
+	disp = kzalloc(sizeof(*disp), GFP_KERNEL);
+	if (!disp) {
+		pr_err("%s: could not allocate struct disp'\n", __func__);
+		goto dispfail;
+	}
+
+	entry = of_parse_phandle(timings_np, "native-mode", 0);
+	/* assume first child as native mode if none provided */
+	if (!entry)
+		entry = of_get_next_child(np, NULL);
+	/* if there is no child, it is useless to go on */
+	if (!entry) {
+		pr_err("%s: no timing specifications given\n", __func__);
+		goto entryfail;
+	}
+
+	pr_info("%s: using %s as default timing\n", __func__, entry->name);
+
+	native_mode = entry;
+
+	disp->num_timings = of_get_child_count(timings_np);
+	if (disp->num_timings == 0) {
+		/* should never happen, as entry was already found above */
+		pr_err("%s: no timings specified\n", __func__);
+		goto entryfail;
+	}
+
+	disp->timings = kzalloc(sizeof(struct display_timing *)*disp->num_timings,
+				GFP_KERNEL);
+	if (!disp->timings) {
+		pr_err("%s: could not allocate timings array\n", __func__);
+		goto entryfail;
+	}
+
+	disp->num_timings = 0;
+	disp->native_mode = 0;
+
+	for_each_child_of_node(timings_np, entry) {
+		struct display_timing *dt;
+
+		dt = of_get_display_timing(entry);
+		if (!dt) {
+			/* to not encourage wrong devicetrees, fail in case of an error */
+			pr_err("%s: error in timing %d\n", __func__, disp->num_timings+1);
+			goto timingfail;
+		}
+
+		if (native_mode == entry)
+			disp->native_mode = disp->num_timings;
+
+		disp->timings[disp->num_timings] = dt;
+		disp->num_timings++;
+	}
+	of_node_put(timings_np);
+	of_node_put(native_mode);
+
+	if (disp->num_timings > 0)
+		pr_info("%s: got %d timings. Using timing #%d as default\n", __func__,
+			disp->num_timings , disp->native_mode + 1);
+	else {
+		pr_err("%s: no valid timings specified\n", __func__);
+		display_timings_release(disp);
+		return NULL;
+	}
+	return disp;
+
+timingfail:
+	if (native_mode)
+		of_node_put(native_mode);
+	display_timings_release(disp);
+entryfail:
+	if (disp)
+		kfree(disp);
+dispfail:
+	of_node_put(timings_np);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timings);
+
+/**
+ * of_display_timings_exists - check if a display-timings node is provided
+ * @np: device_node with the timing
+ **/
+int of_display_timings_exists(struct device_node *np)
+{
+	struct device_node *timings_np;
+
+	if (!np)
+		return -EINVAL;
+
+	timings_np = of_parse_phandle(np, "display-timings", 0);
+	if (!timings_np)
+		return -EINVAL;
+
+	of_node_put(timings_np);
+	return 1;
+}
+EXPORT_SYMBOL_GPL(of_display_timings_exists);
diff --git a/drivers/video/of_videomode.c b/drivers/video/of_videomode.c
new file mode 100644
index 0000000..7051701
--- /dev/null
+++ b/drivers/video/of_videomode.c
@@ -0,0 +1,47 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/of.h>
+#include <linux/of_display_timings.h>
+#include <linux/of_videomode.h>
+#include <linux/export.h>
+
+/**
+ * of_get_videomode - get the videomode #<index> from devicetree
+ * @np - devicenode with the display_timings
+ * @vm - set to return value
+ * @index - index into list of display_timings
+ * DESCRIPTION:
+ * Get a list of all display timings and put the one
+ * specified by index into *vm. This function should only be used, if
+ * only one videomode is to be retrieved. A driver that needs to work
+ * with multiple/all videomodes should work with
+ * of_get_display_timings instead.
+ **/
+int of_get_videomode(struct device_node *np, struct videomode *vm, int index)
+{
+	struct display_timings *disp;
+	int ret;
+
+	disp = of_get_display_timings(np);
+	if (!disp) {
+		pr_err("%s: no timings specified\n", __func__);
+		return -EINVAL;
+	}
+
+	if (index == OF_USE_NATIVE_MODE)
+		index = disp->native_mode;
+
+	ret = videomode_from_timing(disp, vm, index);
+	if (ret)
+		return ret;
+
+	display_timings_release(disp);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
diff --git a/include/linux/of_display_timings.h b/include/linux/of_display_timings.h
new file mode 100644
index 0000000..e18cf94
--- /dev/null
+++ b/include/linux/of_display_timings.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * display timings of helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_DISPLAY_TIMINGS_H
+#define __LINUX_OF_DISPLAY_TIMINGS_H
+
+#include <linux/display_timing.h>
+#include <linux/of.h>
+
+#define OF_USE_NATIVE_MODE -1
+
+struct display_timings *of_get_display_timings(struct device_node *np);
+int of_display_timings_exists(struct device_node *np);
+
+#endif
diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
new file mode 100644
index 0000000..5bb7198
--- /dev/null
+++ b/include/linux/of_videomode.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * videomode of-helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_VIDEOMODE_H
+#define __LINUX_OF_VIDEOMODE_H
+
+#include <linux/videomode.h>
+#include <linux/of.h>
+
+int of_get_videomode(struct device_node *np, struct videomode *vm, int index);
+
+#endif /* __LINUX_OF_VIDEOMODE_H */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v10 2/6] video: add of helper for videomode
@ 2012-11-15  9:23   ` Steffen Trumtrar
  0 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Philipp Zabel, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Tomi Valkeinen,
	Stephen Warren, kernel

This adds support for reading display timings from DT or/and convert one of those
timings to a videomode.
The of_display_timing implementation supports multiple children where each
property can have up to 3 values. All children are read into an array, that
can be queried.
of_get_videomode converts exactly one of that timings to a struct videomode.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Stephen Warren <swarren@nvidia.com>
---
 .../devicetree/bindings/video/display-timings.txt  |  107 ++++++++++
 drivers/video/Kconfig                              |   13 ++
 drivers/video/Makefile                             |    2 +
 drivers/video/of_display_timing.c                  |  212 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   47 +++++
 include/linux/of_display_timings.h                 |   20 ++
 include/linux/of_videomode.h                       |   17 ++
 7 files changed, 418 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 include/linux/of_display_timings.h
 create mode 100644 include/linux/of_videomode.h

diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt
new file mode 100644
index 0000000..a05cade
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timings.txt
@@ -0,0 +1,107 @@
+display-timings bindings
+============
+
+display-timings node
+--------------------
+
+required properties:
+ - none
+
+optional properties:
+ - native-mode: The native mode for the display, in case multiple modes are
+		provided. When omitted, assume the first node is the native.
+
+timings subnode
+---------------
+
+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-frequency: display clock in Hz
+
+optional properties:
+ - hsync-active: Hsync pulse is active low/high/ignored
+ - vsync-active: Vsync pulse is active low/high/ignored
+ - de-active: Data-Enable pulse is active low/high/ignored
+ - pixelclk-inverted: pixelclock is inverted/non-inverted/ignored
+ - interlaced (bool)
+ - doublescan (bool)
+
+All the optional properties that are not bool follow the following logic:
+    <1>: high active
+    <0>: low active
+    omitted: not used on hardware
+
+There are different ways of describing the capabilities of a display. The devicetree
+representation corresponds to the one commonly found in datasheets for displays.
+If a display supports multiple signal timings, the native-mode can be specified.
+
+The parameters are defined as
+
+struct display_timing
+==========+
+  +----------+---------------------------------------------+----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |vback_porch                 |          |       |
+  |          |                ↓                            |          |       |
+  +----------###############################################----------+-------+
+  |          #                ↑                            #          |       |
+  |          #                |                            #          |       |
+  |  hback   #                |                            #  hfront  | hsync |
+  |   porch  #                |       hactive              #  porch   |  len  |
+  |<-------->#<---------------+--------------------------->#<-------->|<----->|
+  |          #                |                            #          |       |
+  |          #                |vactive                     #          |       |
+  |          #                |                            #          |       |
+  |          #                ↓                            #          |       |
+  +----------###############################################----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |vfront_porch                |          |       |
+  |          |                ↓                            |          |       |
+  +----------+---------------------------------------------+----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |vsync_len                   |          |       |
+  |          |                ↓                            |          |       |
+  +----------+---------------------------------------------+----------+-------+
+
+
+Example:
+
+	display-timings {
+		native-mode = <&timing0>;
+		timing0: 1920p24 {
+			/* 1920x1080p24 */
+			clock-frequency = <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 = <1>;
+		};
+	};
+
+Every required property also supports the use of ranges, so the commonly used
+datasheet description with <min typ max>-tuples can be used.
+
+Example:
+
+	timing1: timing {
+		/* 1920x1080p24 */
+		clock-frequency = <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>;
+	};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2a23b18..a324457 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -39,6 +39,19 @@ config DISPLAY_TIMING
 config VIDEOMODE
        bool
 
+config OF_DISPLAY_TIMING
+	def_bool y
+	select DISPLAY_TIMING
+	help
+	  helper to parse display timings from the devicetree
+
+config OF_VIDEOMODE
+	def_bool y
+	select VIDEOMODE
+	select OF_DISPLAY_TIMING
+	help
+	  helper to get videomodes from the devicetree
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index fc30439..b936b00 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -168,4 +168,6 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
 obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_OF_DISPLAY_TIMING) += of_display_timing.o
 obj-$(CONFIG_VIDEOMODE) += videomode.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
new file mode 100644
index 0000000..b187f31
--- /dev/null
+++ b/drivers/video/of_display_timing.c
@@ -0,0 +1,212 @@
+/*
+ * OF helpers for parsing display timings
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * based on of_videomode.c by Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/export.h>
+#include <linux/of_display_timings.h>
+
+/**
+ * parse_property - parse timing_entry from device_node
+ * @np: device_node with the property
+ * @name: name of the property
+ * @result: will be set to the return value
+ *
+ * DESCRIPTION:
+ * Every display_timing can be specified with either just the typical value or
+ * a range consisting of min/typ/max. This function helps handling this
+ **/
+static int parse_property(struct device_node *np, char *name,
+				struct timing_entry *result)
+{
+	struct property *prop;
+	int length, cells, ret;
+
+	prop = of_find_property(np, name, &length);
+	if (!prop) {
+		pr_err("%s: could not find property %s\n", __func__, name);
+		return -EINVAL;
+	}
+
+	cells = length / sizeof(u32);
+	if (cells = 1) {
+		ret = of_property_read_u32(np, name, &result->typ);
+		result->min = result->typ;
+		result->max = result->typ;
+	} else if (cells = 3) {
+		ret = of_property_read_u32_array(np, name, &result->min, cells);
+	} else {
+		pr_err("%s: illegal timing specification in %s\n", __func__, name);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+/**
+ * of_get_display_timing - parse display_timing entry from device_node
+ * @np: device_node with the properties
+ **/
+static struct display_timing *of_get_display_timing(struct device_node *np)
+{
+	struct display_timing *dt;
+	int ret = 0;
+
+	dt = kzalloc(sizeof(*dt), GFP_KERNEL);
+	if (!dt) {
+		pr_err("%s: could not allocate display_timing struct\n", __func__);
+		return NULL;
+	}
+
+	ret |= parse_property(np, "hback-porch", &dt->hback_porch);
+	ret |= parse_property(np, "hfront-porch", &dt->hfront_porch);
+	ret |= parse_property(np, "hactive", &dt->hactive);
+	ret |= parse_property(np, "hsync-len", &dt->hsync_len);
+	ret |= parse_property(np, "vback-porch", &dt->vback_porch);
+	ret |= parse_property(np, "vfront-porch", &dt->vfront_porch);
+	ret |= parse_property(np, "vactive", &dt->vactive);
+	ret |= parse_property(np, "vsync-len", &dt->vsync_len);
+	ret |= parse_property(np, "clock-frequency", &dt->pixelclock);
+
+	of_property_read_u32(np, "vsync-active", &dt->vsync_pol_active);
+	of_property_read_u32(np, "hsync-active", &dt->hsync_pol_active);
+	of_property_read_u32(np, "de-active", &dt->de_pol_active);
+	of_property_read_u32(np, "pixelclk-inverted", &dt->pixelclk_pol);
+	dt->interlaced = of_property_read_bool(np, "interlaced");
+	dt->doublescan = of_property_read_bool(np, "doublescan");
+
+	if (ret) {
+		pr_err("%s: error reading timing properties\n", __func__);
+		kfree(dt);
+		return NULL;
+	}
+
+	return dt;
+}
+
+/**
+ * of_get_display_timings - parse all display_timing entries from a device_node
+ * @np: device_node with the subnodes
+ **/
+struct display_timings *of_get_display_timings(struct device_node *np)
+{
+	struct device_node *timings_np;
+	struct device_node *entry;
+	struct device_node *native_mode;
+	struct display_timings *disp;
+
+	if (!np) {
+		pr_err("%s: no devicenode given\n", __func__);
+		return NULL;
+	}
+
+	timings_np = of_find_node_by_name(np, "display-timings");
+	if (!timings_np) {
+		pr_err("%s: could not find display-timings node\n", __func__);
+		return NULL;
+	}
+
+	disp = kzalloc(sizeof(*disp), GFP_KERNEL);
+	if (!disp) {
+		pr_err("%s: could not allocate struct disp'\n", __func__);
+		goto dispfail;
+	}
+
+	entry = of_parse_phandle(timings_np, "native-mode", 0);
+	/* assume first child as native mode if none provided */
+	if (!entry)
+		entry = of_get_next_child(np, NULL);
+	/* if there is no child, it is useless to go on */
+	if (!entry) {
+		pr_err("%s: no timing specifications given\n", __func__);
+		goto entryfail;
+	}
+
+	pr_info("%s: using %s as default timing\n", __func__, entry->name);
+
+	native_mode = entry;
+
+	disp->num_timings = of_get_child_count(timings_np);
+	if (disp->num_timings = 0) {
+		/* should never happen, as entry was already found above */
+		pr_err("%s: no timings specified\n", __func__);
+		goto entryfail;
+	}
+
+	disp->timings = kzalloc(sizeof(struct display_timing *)*disp->num_timings,
+				GFP_KERNEL);
+	if (!disp->timings) {
+		pr_err("%s: could not allocate timings array\n", __func__);
+		goto entryfail;
+	}
+
+	disp->num_timings = 0;
+	disp->native_mode = 0;
+
+	for_each_child_of_node(timings_np, entry) {
+		struct display_timing *dt;
+
+		dt = of_get_display_timing(entry);
+		if (!dt) {
+			/* to not encourage wrong devicetrees, fail in case of an error */
+			pr_err("%s: error in timing %d\n", __func__, disp->num_timings+1);
+			goto timingfail;
+		}
+
+		if (native_mode = entry)
+			disp->native_mode = disp->num_timings;
+
+		disp->timings[disp->num_timings] = dt;
+		disp->num_timings++;
+	}
+	of_node_put(timings_np);
+	of_node_put(native_mode);
+
+	if (disp->num_timings > 0)
+		pr_info("%s: got %d timings. Using timing #%d as default\n", __func__,
+			disp->num_timings , disp->native_mode + 1);
+	else {
+		pr_err("%s: no valid timings specified\n", __func__);
+		display_timings_release(disp);
+		return NULL;
+	}
+	return disp;
+
+timingfail:
+	if (native_mode)
+		of_node_put(native_mode);
+	display_timings_release(disp);
+entryfail:
+	if (disp)
+		kfree(disp);
+dispfail:
+	of_node_put(timings_np);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timings);
+
+/**
+ * of_display_timings_exists - check if a display-timings node is provided
+ * @np: device_node with the timing
+ **/
+int of_display_timings_exists(struct device_node *np)
+{
+	struct device_node *timings_np;
+
+	if (!np)
+		return -EINVAL;
+
+	timings_np = of_parse_phandle(np, "display-timings", 0);
+	if (!timings_np)
+		return -EINVAL;
+
+	of_node_put(timings_np);
+	return 1;
+}
+EXPORT_SYMBOL_GPL(of_display_timings_exists);
diff --git a/drivers/video/of_videomode.c b/drivers/video/of_videomode.c
new file mode 100644
index 0000000..7051701
--- /dev/null
+++ b/drivers/video/of_videomode.c
@@ -0,0 +1,47 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/of.h>
+#include <linux/of_display_timings.h>
+#include <linux/of_videomode.h>
+#include <linux/export.h>
+
+/**
+ * of_get_videomode - get the videomode #<index> from devicetree
+ * @np - devicenode with the display_timings
+ * @vm - set to return value
+ * @index - index into list of display_timings
+ * DESCRIPTION:
+ * Get a list of all display timings and put the one
+ * specified by index into *vm. This function should only be used, if
+ * only one videomode is to be retrieved. A driver that needs to work
+ * with multiple/all videomodes should work with
+ * of_get_display_timings instead.
+ **/
+int of_get_videomode(struct device_node *np, struct videomode *vm, int index)
+{
+	struct display_timings *disp;
+	int ret;
+
+	disp = of_get_display_timings(np);
+	if (!disp) {
+		pr_err("%s: no timings specified\n", __func__);
+		return -EINVAL;
+	}
+
+	if (index = OF_USE_NATIVE_MODE)
+		index = disp->native_mode;
+
+	ret = videomode_from_timing(disp, vm, index);
+	if (ret)
+		return ret;
+
+	display_timings_release(disp);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
diff --git a/include/linux/of_display_timings.h b/include/linux/of_display_timings.h
new file mode 100644
index 0000000..e18cf94
--- /dev/null
+++ b/include/linux/of_display_timings.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * display timings of helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_DISPLAY_TIMINGS_H
+#define __LINUX_OF_DISPLAY_TIMINGS_H
+
+#include <linux/display_timing.h>
+#include <linux/of.h>
+
+#define OF_USE_NATIVE_MODE -1
+
+struct display_timings *of_get_display_timings(struct device_node *np);
+int of_display_timings_exists(struct device_node *np);
+
+#endif
diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
new file mode 100644
index 0000000..5bb7198
--- /dev/null
+++ b/include/linux/of_videomode.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * videomode of-helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_VIDEOMODE_H
+#define __LINUX_OF_VIDEOMODE_H
+
+#include <linux/videomode.h>
+#include <linux/of.h>
+
+int of_get_videomode(struct device_node *np, struct videomode *vm, int index);
+
+#endif /* __LINUX_OF_VIDEOMODE_H */
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v10 3/6] fbmon: add videomode helpers
  2012-11-15  9:23 ` Steffen Trumtrar
@ 2012-11-15  9:23   ` Steffen Trumtrar
  -1 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Add a function to convert from the generic videomode to a fb_videomode.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/fbmon.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h    |    6 ++++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..247e079 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
+#include <linux/videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
 #include <asm/pci-bridge.h>
@@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
 	kfree(timings);
 	return err;
 }
+
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int fb_videomode_from_videomode(struct videomode *vm,
+				struct fb_videomode *fbmode)
+{
+	unsigned int htotal, vtotal;
+
+	fbmode->xres = vm->hactive;
+	fbmode->left_margin = vm->hback_porch;
+	fbmode->right_margin = vm->hfront_porch;
+	fbmode->hsync_len = vm->hsync_len;
+
+	fbmode->yres = vm->vactive;
+	fbmode->upper_margin = vm->vback_porch;
+	fbmode->lower_margin = vm->vfront_porch;
+	fbmode->vsync_len = vm->vsync_len;
+
+	fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000);
+
+	fbmode->sync = 0;
+	fbmode->vmode = 0;
+	if (vm->hah)
+		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
+	if (vm->vah)
+		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->interlaced)
+		fbmode->vmode |= FB_VMODE_INTERLACED;
+	if (vm->doublescan)
+		fbmode->vmode |= FB_VMODE_DOUBLE;
+	if (vm->de)
+		fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
+	fbmode->flag = 0;
+
+	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
+		 vm->hsync_len;
+	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
+		 vm->vsync_len;
+	fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
+#endif
+
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c7a9571..4024136 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -14,6 +14,7 @@
 #include <linux/backlight.h>
 #include <linux/slab.h>
 #include <asm/io.h>
+#include <linux/videomode.h>
 
 struct vm_area_struct;
 struct fb_info;
@@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+extern int fb_videomode_from_videomode(struct videomode *vm,
+				       struct fb_videomode *fbmode);
+#endif
+
 /* drivers/video/modedb.c */
 #define VESA_MODEDB_SIZE 34
 extern void fb_var_to_videomode(struct fb_videomode *mode,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v10 3/6] fbmon: add videomode helpers
@ 2012-11-15  9:23   ` Steffen Trumtrar
  0 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Add a function to convert from the generic videomode to a fb_videomode.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/fbmon.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h    |    6 ++++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..247e079 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
+#include <linux/videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
 #include <asm/pci-bridge.h>
@@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
 	kfree(timings);
 	return err;
 }
+
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int fb_videomode_from_videomode(struct videomode *vm,
+				struct fb_videomode *fbmode)
+{
+	unsigned int htotal, vtotal;
+
+	fbmode->xres = vm->hactive;
+	fbmode->left_margin = vm->hback_porch;
+	fbmode->right_margin = vm->hfront_porch;
+	fbmode->hsync_len = vm->hsync_len;
+
+	fbmode->yres = vm->vactive;
+	fbmode->upper_margin = vm->vback_porch;
+	fbmode->lower_margin = vm->vfront_porch;
+	fbmode->vsync_len = vm->vsync_len;
+
+	fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000);
+
+	fbmode->sync = 0;
+	fbmode->vmode = 0;
+	if (vm->hah)
+		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
+	if (vm->vah)
+		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->interlaced)
+		fbmode->vmode |= FB_VMODE_INTERLACED;
+	if (vm->doublescan)
+		fbmode->vmode |= FB_VMODE_DOUBLE;
+	if (vm->de)
+		fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
+	fbmode->flag = 0;
+
+	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
+		 vm->hsync_len;
+	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
+		 vm->vsync_len;
+	fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
+#endif
+
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c7a9571..4024136 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -14,6 +14,7 @@
 #include <linux/backlight.h>
 #include <linux/slab.h>
 #include <asm/io.h>
+#include <linux/videomode.h>
 
 struct vm_area_struct;
 struct fb_info;
@@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+extern int fb_videomode_from_videomode(struct videomode *vm,
+				       struct fb_videomode *fbmode);
+#endif
+
 /* drivers/video/modedb.c */
 #define VESA_MODEDB_SIZE 34
 extern void fb_var_to_videomode(struct fb_videomode *mode,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v10 4/6] fbmon: add of_videomode helpers
  2012-11-15  9:23 ` Steffen Trumtrar
@ 2012-11-15  9:23   ` Steffen Trumtrar
  -1 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Add helper to get fb_videomode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/fbmon.c |   42 +++++++++++++++++++++++++++++++++++++++++-
 include/linux/fb.h    |    6 ++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 247e079..9e8c5fa 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,7 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
-#include <linux/videomode.h>
+#include <linux/of_videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
 #include <asm/pci-bridge.h>
@@ -1418,6 +1418,46 @@ int fb_videomode_from_videomode(struct videomode *vm,
 EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static inline void dump_fb_videomode(struct fb_videomode *m)
+{
+	pr_debug("fb_videomode = %ux%u@%uHz (%ukHz) %u %u %u %u %u %u %u %u %u\n",
+		 m->xres, m->yres, m->refresh, m->pixclock, m->left_margin,
+		 m->right_margin, m->upper_margin, m->lower_margin,
+		 m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag);
+}
+
+/**
+ * of_get_fb_videomode - get a fb_videomode from devicetree
+ * @np: device_node with the timing specification
+ * @fb: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings ond
+ * work with that instead.
+ */
+int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+			unsigned int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	fb_videomode_from_videomode(&vm, fb);
+
+	pr_info("%s: got %dx%d display mode from %s\n", __func__, vm.hactive,
+		vm.vactive, np->name);
+	dump_fb_videomode(fb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_fb_videomode);
+#endif
 
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 4024136..dc8f693 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -15,6 +15,8 @@
 #include <linux/slab.h>
 #include <asm/io.h>
 #include <linux/videomode.h>
+#include <linux/of.h>
+#include <linux/of_videomode.h>
 
 struct vm_area_struct;
 struct fb_info;
@@ -715,6 +717,10 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+extern int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+			       unsigned int index);
+#endif
 #if IS_ENABLED(CONFIG_VIDEOMODE)
 extern int fb_videomode_from_videomode(struct videomode *vm,
 				       struct fb_videomode *fbmode);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v10 4/6] fbmon: add of_videomode helpers
@ 2012-11-15  9:23   ` Steffen Trumtrar
  0 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Add helper to get fb_videomode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/fbmon.c |   42 +++++++++++++++++++++++++++++++++++++++++-
 include/linux/fb.h    |    6 ++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 247e079..9e8c5fa 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,7 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
-#include <linux/videomode.h>
+#include <linux/of_videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
 #include <asm/pci-bridge.h>
@@ -1418,6 +1418,46 @@ int fb_videomode_from_videomode(struct videomode *vm,
 EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static inline void dump_fb_videomode(struct fb_videomode *m)
+{
+	pr_debug("fb_videomode = %ux%u@%uHz (%ukHz) %u %u %u %u %u %u %u %u %u\n",
+		 m->xres, m->yres, m->refresh, m->pixclock, m->left_margin,
+		 m->right_margin, m->upper_margin, m->lower_margin,
+		 m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag);
+}
+
+/**
+ * of_get_fb_videomode - get a fb_videomode from devicetree
+ * @np: device_node with the timing specification
+ * @fb: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings ond
+ * work with that instead.
+ */
+int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+			unsigned int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	fb_videomode_from_videomode(&vm, fb);
+
+	pr_info("%s: got %dx%d display mode from %s\n", __func__, vm.hactive,
+		vm.vactive, np->name);
+	dump_fb_videomode(fb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_fb_videomode);
+#endif
 
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 4024136..dc8f693 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -15,6 +15,8 @@
 #include <linux/slab.h>
 #include <asm/io.h>
 #include <linux/videomode.h>
+#include <linux/of.h>
+#include <linux/of_videomode.h>
 
 struct vm_area_struct;
 struct fb_info;
@@ -715,6 +717,10 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+extern int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+			       unsigned int index);
+#endif
 #if IS_ENABLED(CONFIG_VIDEOMODE)
 extern int fb_videomode_from_videomode(struct videomode *vm,
 				       struct fb_videomode *fbmode);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v10 5/6] drm_modes: add videomode helpers
  2012-11-15  9:23 ` Steffen Trumtrar
@ 2012-11-15  9:23   ` Steffen Trumtrar
  -1 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Add conversion from videomode to drm_display_mode

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/gpu/drm/drm_modes.c |   37 +++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h          |    6 ++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 59450f3..23d951a0 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
+#include <linux/videomode.h>
 
 /**
  * drm_mode_debug_printmodeline - debug print a mode
@@ -504,6 +505,42 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,
 }
 EXPORT_SYMBOL(drm_gtf_mode);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int drm_display_mode_from_videomode(struct videomode *vm,
+				    struct drm_display_mode *dmode)
+{
+	dmode->hdisplay = vm->hactive;
+	dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
+	dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
+	dmode->htotal = dmode->hsync_end + vm->hback_porch;
+
+	dmode->vdisplay = vm->vactive;
+	dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
+	dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
+	dmode->vtotal = dmode->vsync_end + vm->vback_porch;
+
+	dmode->clock = vm->pixelclock / 1000;
+
+	dmode->flags = 0;
+	if (vm->hah)
+		dmode->flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		dmode->flags |= DRM_MODE_FLAG_NHSYNC;
+	if (vm->vah)
+		dmode->flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		dmode->flags |= DRM_MODE_FLAG_NVSYNC;
+	if (vm->interlaced)
+		dmode->flags |= DRM_MODE_FLAG_INTERLACE;
+	if (vm->doublescan)
+		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
+	drm_mode_set_name(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3fd8280..341049c 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -56,6 +56,7 @@
 #include <linux/cdev.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/videomode.h>
 #if defined(__alpha__) || defined(__powerpc__)
 #include <asm/pgtable.h>	/* For pte_wrprotect */
 #endif
@@ -1454,6 +1455,11 @@ extern struct drm_display_mode *
 drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 				  struct drm_cmdline_mode *cmd);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+extern int drm_display_mode_from_videomode(struct videomode *vm,
+					   struct drm_display_mode *dmode);
+#endif
+
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
 extern void drm_vblank_post_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v10 5/6] drm_modes: add videomode helpers
@ 2012-11-15  9:23   ` Steffen Trumtrar
  0 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Add conversion from videomode to drm_display_mode

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/gpu/drm/drm_modes.c |   37 +++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h          |    6 ++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 59450f3..23d951a0 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
+#include <linux/videomode.h>
 
 /**
  * drm_mode_debug_printmodeline - debug print a mode
@@ -504,6 +505,42 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,
 }
 EXPORT_SYMBOL(drm_gtf_mode);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int drm_display_mode_from_videomode(struct videomode *vm,
+				    struct drm_display_mode *dmode)
+{
+	dmode->hdisplay = vm->hactive;
+	dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
+	dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
+	dmode->htotal = dmode->hsync_end + vm->hback_porch;
+
+	dmode->vdisplay = vm->vactive;
+	dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
+	dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
+	dmode->vtotal = dmode->vsync_end + vm->vback_porch;
+
+	dmode->clock = vm->pixelclock / 1000;
+
+	dmode->flags = 0;
+	if (vm->hah)
+		dmode->flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		dmode->flags |= DRM_MODE_FLAG_NHSYNC;
+	if (vm->vah)
+		dmode->flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		dmode->flags |= DRM_MODE_FLAG_NVSYNC;
+	if (vm->interlaced)
+		dmode->flags |= DRM_MODE_FLAG_INTERLACE;
+	if (vm->doublescan)
+		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
+	drm_mode_set_name(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3fd8280..341049c 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -56,6 +56,7 @@
 #include <linux/cdev.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/videomode.h>
 #if defined(__alpha__) || defined(__powerpc__)
 #include <asm/pgtable.h>	/* For pte_wrprotect */
 #endif
@@ -1454,6 +1455,11 @@ extern struct drm_display_mode *
 drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 				  struct drm_cmdline_mode *cmd);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+extern int drm_display_mode_from_videomode(struct videomode *vm,
+					   struct drm_display_mode *dmode);
+#endif
+
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
 extern void drm_vblank_post_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v10 6/6] drm_modes: add of_videomode helpers
  2012-11-15  9:23 ` Steffen Trumtrar
@ 2012-11-15  9:23   ` Steffen Trumtrar
  -1 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Add helper to get drm_display_mode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/gpu/drm/drm_modes.c |   35 ++++++++++++++++++++++++++++++++++-
 include/drm/drmP.h          |    6 ++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 23d951a0..810b534 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,7 +35,8 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
-#include <linux/videomode.h>
+#include <linux/of.h>
+#include <linux/of_videomode.h>
 
 /**
  * drm_mode_debug_printmodeline - debug print a mode
@@ -541,6 +542,38 @@ int drm_display_mode_from_videomode(struct videomode *vm,
 EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+/**
+ * of_get_drm_display_mode - get a drm_display_mode from devicetree
+ * @np: device_node with the timing specification
+ * @dmode: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings and
+ * work with that instead.
+ */
+int of_get_drm_display_mode(struct device_node *np,
+			    struct drm_display_mode *dmode, unsigned int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	display_mode_from_videomode(&vm, dmode);
+
+	pr_info("%s: got %dx%d display mode from %s\n", __func__, vm.hactive,
+		vm.vactive, np->name);
+	drm_mode_debug_printmodeline(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 341049c..ae132be 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -56,6 +56,7 @@
 #include <linux/cdev.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <linux/videomode.h>
 #if defined(__alpha__) || defined(__powerpc__)
 #include <asm/pgtable.h>	/* For pte_wrprotect */
@@ -1459,6 +1460,11 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 extern int drm_display_mode_from_videomode(struct videomode *vm,
 					   struct drm_display_mode *dmode);
 #endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+extern int of_get_drm_display_mode(struct device_node *np,
+				   struct drm_display_mode *dmode,
+				   unsigned int index);
+#endif
 
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v10 6/6] drm_modes: add of_videomode helpers
@ 2012-11-15  9:23   ` Steffen Trumtrar
  0 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15  9:23 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel

Add helper to get drm_display_mode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/gpu/drm/drm_modes.c |   35 ++++++++++++++++++++++++++++++++++-
 include/drm/drmP.h          |    6 ++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 23d951a0..810b534 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,7 +35,8 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
-#include <linux/videomode.h>
+#include <linux/of.h>
+#include <linux/of_videomode.h>
 
 /**
  * drm_mode_debug_printmodeline - debug print a mode
@@ -541,6 +542,38 @@ int drm_display_mode_from_videomode(struct videomode *vm,
 EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+/**
+ * of_get_drm_display_mode - get a drm_display_mode from devicetree
+ * @np: device_node with the timing specification
+ * @dmode: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings and
+ * work with that instead.
+ */
+int of_get_drm_display_mode(struct device_node *np,
+			    struct drm_display_mode *dmode, unsigned int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	display_mode_from_videomode(&vm, dmode);
+
+	pr_info("%s: got %dx%d display mode from %s\n", __func__, vm.hactive,
+		vm.vactive, np->name);
+	drm_mode_debug_printmodeline(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 341049c..ae132be 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -56,6 +56,7 @@
 #include <linux/cdev.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <linux/videomode.h>
 #if defined(__alpha__) || defined(__powerpc__)
 #include <asm/pgtable.h>	/* For pte_wrprotect */
@@ -1459,6 +1460,11 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 extern int drm_display_mode_from_videomode(struct videomode *vm,
 					   struct drm_display_mode *dmode);
 #endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+extern int of_get_drm_display_mode(struct device_node *np,
+				   struct drm_display_mode *dmode,
+				   unsigned int index);
+#endif
 
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 6/6] drm_modes: add of_videomode helpers
  2012-11-15  9:23   ` Steffen Trumtrar
  (?)
@ 2012-11-15  9:58       ` Thierry Reding
  -1 siblings, 0 replies; 35+ messages in thread
From: Thierry Reding @ 2012-11-15  9:58 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
	Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA


[-- Attachment #1.1: Type: text/plain, Size: 432 bytes --]

On Thu, Nov 15, 2012 at 10:23:57AM +0100, Steffen Trumtrar wrote:
[...]
> +int of_get_drm_display_mode(struct device_node *np,
> +			    struct drm_display_mode *dmode, unsigned int index)
> +{
> +	struct videomode vm;
> +	int ret;
> +
> +	ret = of_get_videomode(np, &vm, index);
> +	if (ret)
> +		return ret;
> +
> +	display_mode_from_videomode(&vm, dmode);

This function is now called drm_display_mode_from_videomode().

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 6/6] drm_modes: add of_videomode helpers
@ 2012-11-15  9:58       ` Thierry Reding
  0 siblings, 0 replies; 35+ messages in thread
From: Thierry Reding @ 2012-11-15  9:58 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Guennady Liakhovetski, linux-media,
	Tomi Valkeinen, Stephen Warren, kernel

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

On Thu, Nov 15, 2012 at 10:23:57AM +0100, Steffen Trumtrar wrote:
[...]
> +int of_get_drm_display_mode(struct device_node *np,
> +			    struct drm_display_mode *dmode, unsigned int index)
> +{
> +	struct videomode vm;
> +	int ret;
> +
> +	ret = of_get_videomode(np, &vm, index);
> +	if (ret)
> +		return ret;
> +
> +	display_mode_from_videomode(&vm, dmode);

This function is now called drm_display_mode_from_videomode().

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 6/6] drm_modes: add of_videomode helpers
@ 2012-11-15  9:58       ` Thierry Reding
  0 siblings, 0 replies; 35+ messages in thread
From: Thierry Reding @ 2012-11-15  9:58 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
	Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

On Thu, Nov 15, 2012 at 10:23:57AM +0100, Steffen Trumtrar wrote:
[...]
> +int of_get_drm_display_mode(struct device_node *np,
> +			    struct drm_display_mode *dmode, unsigned int index)
> +{
> +	struct videomode vm;
> +	int ret;
> +
> +	ret = of_get_videomode(np, &vm, index);
> +	if (ret)
> +		return ret;
> +
> +	display_mode_from_videomode(&vm, dmode);

This function is now called drm_display_mode_from_videomode().

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 6/6] drm_modes: add of_videomode helpers
  2012-11-15  9:58       ` Thierry Reding
@ 2012-11-15 10:01         ` Steffen Trumtrar
  -1 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15 10:01 UTC (permalink / raw)
  To: Thierry Reding
  Cc: linux-fbdev, devicetree-discuss, dri-devel, Tomi Valkeinen,
	Laurent Pinchart, kernel, Guennady Liakhovetski, linux-media

On Thu, Nov 15, 2012 at 10:58:49AM +0100, Thierry Reding wrote:
> On Thu, Nov 15, 2012 at 10:23:57AM +0100, Steffen Trumtrar wrote:
> [...]
> > +int of_get_drm_display_mode(struct device_node *np,
> > +			    struct drm_display_mode *dmode, unsigned int index)
> > +{
> > +	struct videomode vm;
> > +	int ret;
> > +
> > +	ret = of_get_videomode(np, &vm, index);
> > +	if (ret)
> > +		return ret;
> > +
> > +	display_mode_from_videomode(&vm, dmode);
> 
> This function is now called drm_display_mode_from_videomode().
> 
Well, can't argue with that. You are right.

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 6/6] drm_modes: add of_videomode helpers
@ 2012-11-15 10:01         ` Steffen Trumtrar
  0 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15 10:01 UTC (permalink / raw)
  To: Thierry Reding
  Cc: linux-fbdev, devicetree-discuss, dri-devel, Tomi Valkeinen,
	Laurent Pinchart, kernel, Guennady Liakhovetski, linux-media

On Thu, Nov 15, 2012 at 10:58:49AM +0100, Thierry Reding wrote:
> On Thu, Nov 15, 2012 at 10:23:57AM +0100, Steffen Trumtrar wrote:
> [...]
> > +int of_get_drm_display_mode(struct device_node *np,
> > +			    struct drm_display_mode *dmode, unsigned int index)
> > +{
> > +	struct videomode vm;
> > +	int ret;
> > +
> > +	ret = of_get_videomode(np, &vm, index);
> > +	if (ret)
> > +		return ret;
> > +
> > +	display_mode_from_videomode(&vm, dmode);
> 
> This function is now called drm_display_mode_from_videomode().
> 
Well, can't argue with that. You are right.

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 0/6] of: add display helper
  2012-11-15  9:23 ` Steffen Trumtrar
@ 2012-11-15 10:24   ` Thierry Reding
  -1 siblings, 0 replies; 35+ messages in thread
From: Thierry Reding @ 2012-11-15 10:24 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Guennady Liakhovetski, linux-media,
	Tomi Valkeinen, Stephen Warren, kernel

[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]

On Thu, Nov 15, 2012 at 10:23:51AM +0100, Steffen Trumtrar wrote:
> Hi!
> 
> Changes since v9:
> 	- don't leak memory when previous timings were correct
> 	- CodingStyle fixes
> 	- move blank lines around
> 
> Regards,
> Steffen
> 
> 
> Steffen Trumtrar (6):
>   video: add display_timing and videomode
>   video: add of helper for videomode
>   fbmon: add videomode helpers
>   fbmon: add of_videomode helpers
>   drm_modes: add videomode helpers
>   drm_modes: add of_videomode helpers
> 
>  .../devicetree/bindings/video/display-timings.txt  |  107 ++++++++++
>  drivers/gpu/drm/drm_modes.c                        |   70 +++++++
>  drivers/video/Kconfig                              |   19 ++
>  drivers/video/Makefile                             |    4 +
>  drivers/video/display_timing.c                     |   24 +++
>  drivers/video/fbmon.c                              |   86 ++++++++
>  drivers/video/of_display_timing.c                  |  212 ++++++++++++++++++++
>  drivers/video/of_videomode.c                       |   47 +++++
>  drivers/video/videomode.c                          |   45 +++++
>  include/drm/drmP.h                                 |   12 ++
>  include/linux/display_timing.h                     |   69 +++++++
>  include/linux/fb.h                                 |   12 ++
>  include/linux/of_display_timings.h                 |   20 ++
>  include/linux/of_videomode.h                       |   17 ++
>  include/linux/videomode.h                          |   40 ++++
>  15 files changed, 784 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
>  create mode 100644 drivers/video/display_timing.c
>  create mode 100644 drivers/video/of_display_timing.c
>  create mode 100644 drivers/video/of_videomode.c
>  create mode 100644 drivers/video/videomode.c
>  create mode 100644 include/linux/display_timing.h
>  create mode 100644 include/linux/of_display_timings.h
>  create mode 100644 include/linux/of_videomode.h
>  create mode 100644 include/linux/videomode.h

With the one change that I pointed out, the whole series:

Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 0/6] of: add display helper
@ 2012-11-15 10:24   ` Thierry Reding
  0 siblings, 0 replies; 35+ messages in thread
From: Thierry Reding @ 2012-11-15 10:24 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Guennady Liakhovetski, linux-media,
	Tomi Valkeinen, Stephen Warren, kernel

[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]

On Thu, Nov 15, 2012 at 10:23:51AM +0100, Steffen Trumtrar wrote:
> Hi!
> 
> Changes since v9:
> 	- don't leak memory when previous timings were correct
> 	- CodingStyle fixes
> 	- move blank lines around
> 
> Regards,
> Steffen
> 
> 
> Steffen Trumtrar (6):
>   video: add display_timing and videomode
>   video: add of helper for videomode
>   fbmon: add videomode helpers
>   fbmon: add of_videomode helpers
>   drm_modes: add videomode helpers
>   drm_modes: add of_videomode helpers
> 
>  .../devicetree/bindings/video/display-timings.txt  |  107 ++++++++++
>  drivers/gpu/drm/drm_modes.c                        |   70 +++++++
>  drivers/video/Kconfig                              |   19 ++
>  drivers/video/Makefile                             |    4 +
>  drivers/video/display_timing.c                     |   24 +++
>  drivers/video/fbmon.c                              |   86 ++++++++
>  drivers/video/of_display_timing.c                  |  212 ++++++++++++++++++++
>  drivers/video/of_videomode.c                       |   47 +++++
>  drivers/video/videomode.c                          |   45 +++++
>  include/drm/drmP.h                                 |   12 ++
>  include/linux/display_timing.h                     |   69 +++++++
>  include/linux/fb.h                                 |   12 ++
>  include/linux/of_display_timings.h                 |   20 ++
>  include/linux/of_videomode.h                       |   17 ++
>  include/linux/videomode.h                          |   40 ++++
>  15 files changed, 784 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
>  create mode 100644 drivers/video/display_timing.c
>  create mode 100644 drivers/video/of_display_timing.c
>  create mode 100644 drivers/video/of_videomode.c
>  create mode 100644 drivers/video/videomode.c
>  create mode 100644 include/linux/display_timing.h
>  create mode 100644 include/linux/of_display_timings.h
>  create mode 100644 include/linux/of_videomode.h
>  create mode 100644 include/linux/videomode.h

With the one change that I pointed out, the whole series:

Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 0/6] of: add display helper
  2012-11-15 10:24   ` Thierry Reding
  (?)
@ 2012-11-15 10:27       ` Thierry Reding
  -1 siblings, 0 replies; 35+ messages in thread
From: Thierry Reding @ 2012-11-15 10:27 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
	Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA


[-- Attachment #1.1: Type: text/plain, Size: 2612 bytes --]

On Thu, Nov 15, 2012 at 11:24:11AM +0100, Thierry Reding wrote:
> On Thu, Nov 15, 2012 at 10:23:51AM +0100, Steffen Trumtrar wrote:
> > Hi!
> > 
> > Changes since v9:
> > 	- don't leak memory when previous timings were correct
> > 	- CodingStyle fixes
> > 	- move blank lines around
> > 
> > Regards,
> > Steffen
> > 
> > 
> > Steffen Trumtrar (6):
> >   video: add display_timing and videomode
> >   video: add of helper for videomode
> >   fbmon: add videomode helpers
> >   fbmon: add of_videomode helpers
> >   drm_modes: add videomode helpers
> >   drm_modes: add of_videomode helpers
> > 
> >  .../devicetree/bindings/video/display-timings.txt  |  107 ++++++++++
> >  drivers/gpu/drm/drm_modes.c                        |   70 +++++++
> >  drivers/video/Kconfig                              |   19 ++
> >  drivers/video/Makefile                             |    4 +
> >  drivers/video/display_timing.c                     |   24 +++
> >  drivers/video/fbmon.c                              |   86 ++++++++
> >  drivers/video/of_display_timing.c                  |  212 ++++++++++++++++++++
> >  drivers/video/of_videomode.c                       |   47 +++++
> >  drivers/video/videomode.c                          |   45 +++++
> >  include/drm/drmP.h                                 |   12 ++
> >  include/linux/display_timing.h                     |   69 +++++++
> >  include/linux/fb.h                                 |   12 ++
> >  include/linux/of_display_timings.h                 |   20 ++
> >  include/linux/of_videomode.h                       |   17 ++
> >  include/linux/videomode.h                          |   40 ++++
> >  15 files changed, 784 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
> >  create mode 100644 drivers/video/display_timing.c
> >  create mode 100644 drivers/video/of_display_timing.c
> >  create mode 100644 drivers/video/of_videomode.c
> >  create mode 100644 drivers/video/videomode.c
> >  create mode 100644 include/linux/display_timing.h
> >  create mode 100644 include/linux/of_display_timings.h
> >  create mode 100644 include/linux/of_videomode.h
> >  create mode 100644 include/linux/videomode.h
> 
> With the one change that I pointed out, the whole series:
> 
> Reviewed-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
> Acked-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>

Also:

Tested-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>

=)

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 0/6] of: add display helper
@ 2012-11-15 10:27       ` Thierry Reding
  0 siblings, 0 replies; 35+ messages in thread
From: Thierry Reding @ 2012-11-15 10:27 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Guennady Liakhovetski, linux-media,
	Tomi Valkeinen, Stephen Warren, kernel

[-- Attachment #1: Type: text/plain, Size: 2516 bytes --]

On Thu, Nov 15, 2012 at 11:24:11AM +0100, Thierry Reding wrote:
> On Thu, Nov 15, 2012 at 10:23:51AM +0100, Steffen Trumtrar wrote:
> > Hi!
> > 
> > Changes since v9:
> > 	- don't leak memory when previous timings were correct
> > 	- CodingStyle fixes
> > 	- move blank lines around
> > 
> > Regards,
> > Steffen
> > 
> > 
> > Steffen Trumtrar (6):
> >   video: add display_timing and videomode
> >   video: add of helper for videomode
> >   fbmon: add videomode helpers
> >   fbmon: add of_videomode helpers
> >   drm_modes: add videomode helpers
> >   drm_modes: add of_videomode helpers
> > 
> >  .../devicetree/bindings/video/display-timings.txt  |  107 ++++++++++
> >  drivers/gpu/drm/drm_modes.c                        |   70 +++++++
> >  drivers/video/Kconfig                              |   19 ++
> >  drivers/video/Makefile                             |    4 +
> >  drivers/video/display_timing.c                     |   24 +++
> >  drivers/video/fbmon.c                              |   86 ++++++++
> >  drivers/video/of_display_timing.c                  |  212 ++++++++++++++++++++
> >  drivers/video/of_videomode.c                       |   47 +++++
> >  drivers/video/videomode.c                          |   45 +++++
> >  include/drm/drmP.h                                 |   12 ++
> >  include/linux/display_timing.h                     |   69 +++++++
> >  include/linux/fb.h                                 |   12 ++
> >  include/linux/of_display_timings.h                 |   20 ++
> >  include/linux/of_videomode.h                       |   17 ++
> >  include/linux/videomode.h                          |   40 ++++
> >  15 files changed, 784 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
> >  create mode 100644 drivers/video/display_timing.c
> >  create mode 100644 drivers/video/of_display_timing.c
> >  create mode 100644 drivers/video/of_videomode.c
> >  create mode 100644 drivers/video/videomode.c
> >  create mode 100644 include/linux/display_timing.h
> >  create mode 100644 include/linux/of_display_timings.h
> >  create mode 100644 include/linux/of_videomode.h
> >  create mode 100644 include/linux/videomode.h
> 
> With the one change that I pointed out, the whole series:
> 
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>

Also:

Tested-by: Thierry Reding <thierry.reding@avionic-design.de>

=)

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 0/6] of: add display helper
@ 2012-11-15 10:27       ` Thierry Reding
  0 siblings, 0 replies; 35+ messages in thread
From: Thierry Reding @ 2012-11-15 10:27 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
	Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 2516 bytes --]

On Thu, Nov 15, 2012 at 11:24:11AM +0100, Thierry Reding wrote:
> On Thu, Nov 15, 2012 at 10:23:51AM +0100, Steffen Trumtrar wrote:
> > Hi!
> > 
> > Changes since v9:
> > 	- don't leak memory when previous timings were correct
> > 	- CodingStyle fixes
> > 	- move blank lines around
> > 
> > Regards,
> > Steffen
> > 
> > 
> > Steffen Trumtrar (6):
> >   video: add display_timing and videomode
> >   video: add of helper for videomode
> >   fbmon: add videomode helpers
> >   fbmon: add of_videomode helpers
> >   drm_modes: add videomode helpers
> >   drm_modes: add of_videomode helpers
> > 
> >  .../devicetree/bindings/video/display-timings.txt  |  107 ++++++++++
> >  drivers/gpu/drm/drm_modes.c                        |   70 +++++++
> >  drivers/video/Kconfig                              |   19 ++
> >  drivers/video/Makefile                             |    4 +
> >  drivers/video/display_timing.c                     |   24 +++
> >  drivers/video/fbmon.c                              |   86 ++++++++
> >  drivers/video/of_display_timing.c                  |  212 ++++++++++++++++++++
> >  drivers/video/of_videomode.c                       |   47 +++++
> >  drivers/video/videomode.c                          |   45 +++++
> >  include/drm/drmP.h                                 |   12 ++
> >  include/linux/display_timing.h                     |   69 +++++++
> >  include/linux/fb.h                                 |   12 ++
> >  include/linux/of_display_timings.h                 |   20 ++
> >  include/linux/of_videomode.h                       |   17 ++
> >  include/linux/videomode.h                          |   40 ++++
> >  15 files changed, 784 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
> >  create mode 100644 drivers/video/display_timing.c
> >  create mode 100644 drivers/video/of_display_timing.c
> >  create mode 100644 drivers/video/of_videomode.c
> >  create mode 100644 drivers/video/videomode.c
> >  create mode 100644 include/linux/display_timing.h
> >  create mode 100644 include/linux/of_display_timings.h
> >  create mode 100644 include/linux/of_videomode.h
> >  create mode 100644 include/linux/videomode.h
> 
> With the one change that I pointed out, the whole series:
> 
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>

Also:

Tested-by: Thierry Reding <thierry.reding@avionic-design.de>

=)

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 0/6] of: add display helper
  2012-11-15 10:24   ` Thierry Reding
@ 2012-11-15 10:28     ` Steffen Trumtrar
  -1 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15 10:28 UTC (permalink / raw)
  To: Thierry Reding
  Cc: linux-fbdev, devicetree-discuss, dri-devel, Tomi Valkeinen,
	Laurent Pinchart, kernel, Guennady Liakhovetski, linux-media

On Thu, Nov 15, 2012 at 11:24:11AM +0100, Thierry Reding wrote:
> On Thu, Nov 15, 2012 at 10:23:51AM +0100, Steffen Trumtrar wrote:
> > Hi!
> > 
> > Changes since v9:
> > 	- don't leak memory when previous timings were correct
> > 	- CodingStyle fixes
> > 	- move blank lines around
> > 
> > Regards,
> > Steffen
> > 
> > 
> > Steffen Trumtrar (6):
> >   video: add display_timing and videomode
> >   video: add of helper for videomode
> >   fbmon: add videomode helpers
> >   fbmon: add of_videomode helpers
> >   drm_modes: add videomode helpers
> >   drm_modes: add of_videomode helpers
> > 
> >  .../devicetree/bindings/video/display-timings.txt  |  107 ++++++++++
> >  drivers/gpu/drm/drm_modes.c                        |   70 +++++++
> >  drivers/video/Kconfig                              |   19 ++
> >  drivers/video/Makefile                             |    4 +
> >  drivers/video/display_timing.c                     |   24 +++
> >  drivers/video/fbmon.c                              |   86 ++++++++
> >  drivers/video/of_display_timing.c                  |  212 ++++++++++++++++++++
> >  drivers/video/of_videomode.c                       |   47 +++++
> >  drivers/video/videomode.c                          |   45 +++++
> >  include/drm/drmP.h                                 |   12 ++
> >  include/linux/display_timing.h                     |   69 +++++++
> >  include/linux/fb.h                                 |   12 ++
> >  include/linux/of_display_timings.h                 |   20 ++
> >  include/linux/of_videomode.h                       |   17 ++
> >  include/linux/videomode.h                          |   40 ++++
> >  15 files changed, 784 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
> >  create mode 100644 drivers/video/display_timing.c
> >  create mode 100644 drivers/video/of_display_timing.c
> >  create mode 100644 drivers/video/of_videomode.c
> >  create mode 100644 drivers/video/videomode.c
> >  create mode 100644 include/linux/display_timing.h
> >  create mode 100644 include/linux/of_display_timings.h
> >  create mode 100644 include/linux/of_videomode.h
> >  create mode 100644 include/linux/videomode.h
> 
> With the one change that I pointed out, the whole series:
> 

Already fixed.

> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>

\o/ Thanks


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 0/6] of: add display helper
@ 2012-11-15 10:28     ` Steffen Trumtrar
  0 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-15 10:28 UTC (permalink / raw)
  To: Thierry Reding
  Cc: linux-fbdev, devicetree-discuss, dri-devel, Tomi Valkeinen,
	Laurent Pinchart, kernel, Guennady Liakhovetski, linux-media

On Thu, Nov 15, 2012 at 11:24:11AM +0100, Thierry Reding wrote:
> On Thu, Nov 15, 2012 at 10:23:51AM +0100, Steffen Trumtrar wrote:
> > Hi!
> > 
> > Changes since v9:
> > 	- don't leak memory when previous timings were correct
> > 	- CodingStyle fixes
> > 	- move blank lines around
> > 
> > Regards,
> > Steffen
> > 
> > 
> > Steffen Trumtrar (6):
> >   video: add display_timing and videomode
> >   video: add of helper for videomode
> >   fbmon: add videomode helpers
> >   fbmon: add of_videomode helpers
> >   drm_modes: add videomode helpers
> >   drm_modes: add of_videomode helpers
> > 
> >  .../devicetree/bindings/video/display-timings.txt  |  107 ++++++++++
> >  drivers/gpu/drm/drm_modes.c                        |   70 +++++++
> >  drivers/video/Kconfig                              |   19 ++
> >  drivers/video/Makefile                             |    4 +
> >  drivers/video/display_timing.c                     |   24 +++
> >  drivers/video/fbmon.c                              |   86 ++++++++
> >  drivers/video/of_display_timing.c                  |  212 ++++++++++++++++++++
> >  drivers/video/of_videomode.c                       |   47 +++++
> >  drivers/video/videomode.c                          |   45 +++++
> >  include/drm/drmP.h                                 |   12 ++
> >  include/linux/display_timing.h                     |   69 +++++++
> >  include/linux/fb.h                                 |   12 ++
> >  include/linux/of_display_timings.h                 |   20 ++
> >  include/linux/of_videomode.h                       |   17 ++
> >  include/linux/videomode.h                          |   40 ++++
> >  15 files changed, 784 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
> >  create mode 100644 drivers/video/display_timing.c
> >  create mode 100644 drivers/video/of_display_timing.c
> >  create mode 100644 drivers/video/of_videomode.c
> >  create mode 100644 drivers/video/videomode.c
> >  create mode 100644 include/linux/display_timing.h
> >  create mode 100644 include/linux/of_display_timings.h
> >  create mode 100644 include/linux/of_videomode.h
> >  create mode 100644 include/linux/videomode.h
> 
> With the one change that I pointed out, the whole series:
> 

Already fixed.

> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>

\o/ Thanks


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 1/6] video: add display_timing and videomode
  2012-11-15  9:23   ` Steffen Trumtrar
@ 2012-11-15 15:47     ` Grant Likely
  -1 siblings, 0 replies; 35+ messages in thread
From: Grant Likely @ 2012-11-15 15:47 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: linux-fbdev, dri-devel, Tomi Valkeinen, Laurent Pinchart, kernel,
	Steffen Trumtrar, Guennady Liakhovetski, linux-media

On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote:
> Add display_timing structure and the according helper functions. This allows
> the description of a display via its supported timing parameters.
> 
> Every timing parameter can be specified as a single value or a range
> <min typ max>.
> 
> Also, add helper functions to convert from display timings to a generic videomode
> structure. This videomode can then be converted to the corresponding subsystem
> mode representation (e.g. fb_videomode).
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>

Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm
making an incorrect assumption.

It looks to me that the purpose of this entire series is to decode video
timings from the device tree and (eventually) provide the data in the
form 'struct videomode'. Correct?

If so, then it looks over engineered. Creating new infrastructure to
allocate, maintain, and free a new 'struct display_timings' doesn't make
any sense when it is an intermediary data format that will never be used
by drivers.

Can the DT parsing code instead return a table of struct videomode?

But, wait... struct videomode is also a new structure. So it looks like
this series creates two new intermediary data structures;
display_timings and videomode. And at least as far as I can see in this
series struct fb_videomode is the only user.

g.

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 1/6] video: add display_timing and videomode
@ 2012-11-15 15:47     ` Grant Likely
  0 siblings, 0 replies; 35+ messages in thread
From: Grant Likely @ 2012-11-15 15:47 UTC (permalink / raw)
  To: Steffen Trumtrar, devicetree-discuss
  Cc: linux-fbdev, dri-devel, Tomi Valkeinen, Laurent Pinchart, kernel,
	Steffen Trumtrar, Guennady Liakhovetski, linux-media

On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote:
> Add display_timing structure and the according helper functions. This allows
> the description of a display via its supported timing parameters.
> 
> Every timing parameter can be specified as a single value or a range
> <min typ max>.
> 
> Also, add helper functions to convert from display timings to a generic videomode
> structure. This videomode can then be converted to the corresponding subsystem
> mode representation (e.g. fb_videomode).
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>

Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm
making an incorrect assumption.

It looks to me that the purpose of this entire series is to decode video
timings from the device tree and (eventually) provide the data in the
form 'struct videomode'. Correct?

If so, then it looks over engineered. Creating new infrastructure to
allocate, maintain, and free a new 'struct display_timings' doesn't make
any sense when it is an intermediary data format that will never be used
by drivers.

Can the DT parsing code instead return a table of struct videomode?

But, wait... struct videomode is also a new structure. So it looks like
this series creates two new intermediary data structures;
display_timings and videomode. And at least as far as I can see in this
series struct fb_videomode is the only user.

g.

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 1/6] video: add display_timing and videomode
  2012-11-15 15:47     ` Grant Likely
@ 2012-11-15 16:00       ` Laurent Pinchart
  -1 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2012-11-15 16:00 UTC (permalink / raw)
  To: Grant Likely
  Cc: Steffen Trumtrar, devicetree-discuss, linux-fbdev, dri-devel,
	Tomi Valkeinen, kernel, Guennady Liakhovetski, linux-media

Hi Grant,

On Thursday 15 November 2012 15:47:53 Grant Likely wrote:
> On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar wrote:
> > Add display_timing structure and the according helper functions. This
> > allows the description of a display via its supported timing parameters.
> > 
> > Every timing parameter can be specified as a single value or a range
> > <min typ max>.
> > 
> > Also, add helper functions to convert from display timings to a generic
> > videomode structure. This videomode can then be converted to the
> > corresponding subsystem mode representation (e.g. fb_videomode).
> > 
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> 
> Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm
> making an incorrect assumption.
> 
> It looks to me that the purpose of this entire series is to decode video
> timings from the device tree and (eventually) provide the data in the
> form 'struct videomode'. Correct?
> 
> If so, then it looks over engineered. Creating new infrastructure to
> allocate, maintain, and free a new 'struct display_timings' doesn't make
> any sense when it is an intermediary data format that will never be used
> by drivers.
> 
> Can the DT parsing code instead return a table of struct videomode?
> 
> But, wait... struct videomode is also a new structure. So it looks like
> this series creates two new intermediary data structures;
> display_timings and videomode. And at least as far as I can see in this
> series struct fb_videomode is the only user.

struct videomode is supposed to slowly replace the various video mode 
structures we currently have in the kernel (struct drm_mode_modeinfo, struct 
fb_videomode and struct v4l2_bt_timings), at least where possible (userspace 
APIs can't be broken). This will make it possible to reuse code across the 
DRM, FB and V4L2 subsystems, such as the EDID parser or HDMI encoder drivers. 
This rationale might not be clearly explained in the commit message, but 
having a shared video mode structure is pretty important.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 1/6] video: add display_timing and videomode
@ 2012-11-15 16:00       ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2012-11-15 16:00 UTC (permalink / raw)
  To: Grant Likely
  Cc: Steffen Trumtrar, devicetree-discuss, linux-fbdev, dri-devel,
	Tomi Valkeinen, kernel, Guennady Liakhovetski, linux-media

Hi Grant,

On Thursday 15 November 2012 15:47:53 Grant Likely wrote:
> On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar wrote:
> > Add display_timing structure and the according helper functions. This
> > allows the description of a display via its supported timing parameters.
> > 
> > Every timing parameter can be specified as a single value or a range
> > <min typ max>.
> > 
> > Also, add helper functions to convert from display timings to a generic
> > videomode structure. This videomode can then be converted to the
> > corresponding subsystem mode representation (e.g. fb_videomode).
> > 
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> 
> Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm
> making an incorrect assumption.
> 
> It looks to me that the purpose of this entire series is to decode video
> timings from the device tree and (eventually) provide the data in the
> form 'struct videomode'. Correct?
> 
> If so, then it looks over engineered. Creating new infrastructure to
> allocate, maintain, and free a new 'struct display_timings' doesn't make
> any sense when it is an intermediary data format that will never be used
> by drivers.
> 
> Can the DT parsing code instead return a table of struct videomode?
> 
> But, wait... struct videomode is also a new structure. So it looks like
> this series creates two new intermediary data structures;
> display_timings and videomode. And at least as far as I can see in this
> series struct fb_videomode is the only user.

struct videomode is supposed to slowly replace the various video mode 
structures we currently have in the kernel (struct drm_mode_modeinfo, struct 
fb_videomode and struct v4l2_bt_timings), at least where possible (userspace 
APIs can't be broken). This will make it possible to reuse code across the 
DRM, FB and V4L2 subsystems, such as the EDID parser or HDMI encoder drivers. 
This rationale might not be clearly explained in the commit message, but 
having a shared video mode structure is pretty important.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 1/6] video: add display_timing and videomode
  2012-11-15 16:00       ` Laurent Pinchart
  (?)
@ 2012-11-15 18:03       ` Grant Likely
  2012-11-15 19:00           ` Laurent Pinchart
  2012-11-16  8:53           ` Steffen Trumtrar
  -1 siblings, 2 replies; 35+ messages in thread
From: Grant Likely @ 2012-11-15 18:03 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Steffen Trumtrar, devicetree-discuss, linux-fbdev, dri-devel,
	Tomi Valkeinen, kernel, Guennady Liakhovetski, linux-media

On Thu, 15 Nov 2012 17:00:57 +0100, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> Hi Grant,
> 
> On Thursday 15 November 2012 15:47:53 Grant Likely wrote:
> > On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar wrote:
> > > Add display_timing structure and the according helper functions. This
> > > allows the description of a display via its supported timing parameters.
> > > 
> > > Every timing parameter can be specified as a single value or a range
> > > <min typ max>.
> > > 
> > > Also, add helper functions to convert from display timings to a generic
> > > videomode structure. This videomode can then be converted to the
> > > corresponding subsystem mode representation (e.g. fb_videomode).
> > > 
> > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > 
> > Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm
> > making an incorrect assumption.
> > 
> > It looks to me that the purpose of this entire series is to decode video
> > timings from the device tree and (eventually) provide the data in the
> > form 'struct videomode'. Correct?
> > 
> > If so, then it looks over engineered. Creating new infrastructure to
> > allocate, maintain, and free a new 'struct display_timings' doesn't make
> > any sense when it is an intermediary data format that will never be used
> > by drivers.
> > 
> > Can the DT parsing code instead return a table of struct videomode?
> > 
> > But, wait... struct videomode is also a new structure. So it looks like
> > this series creates two new intermediary data structures;
> > display_timings and videomode. And at least as far as I can see in this
> > series struct fb_videomode is the only user.
> 
> struct videomode is supposed to slowly replace the various video mode 
> structures we currently have in the kernel (struct drm_mode_modeinfo, struct 
> fb_videomode and struct v4l2_bt_timings), at least where possible (userspace 
> APIs can't be broken). This will make it possible to reuse code across the 
> DRM, FB and V4L2 subsystems, such as the EDID parser or HDMI encoder drivers. 
> This rationale might not be clearly explained in the commit message, but 
> having a shared video mode structure is pretty important.

Okay that make sense. What about struct display_timings?

g.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 1/6] video: add display_timing and videomode
  2012-11-15 18:03       ` Grant Likely
@ 2012-11-15 19:00           ` Laurent Pinchart
  2012-11-16  8:53           ` Steffen Trumtrar
  1 sibling, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2012-11-15 18:59 UTC (permalink / raw)
  To: Grant Likely
  Cc: Steffen Trumtrar, devicetree-discuss, linux-fbdev, dri-devel,
	Tomi Valkeinen, kernel, Guennady Liakhovetski, linux-media

On Thursday 15 November 2012 18:03:59 Grant Likely wrote:
> On Thu, 15 Nov 2012 17:00:57 +0100, Laurent Pinchart wrote:
> > On Thursday 15 November 2012 15:47:53 Grant Likely wrote:
> > > On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar wrote:
> > > > Add display_timing structure and the according helper functions. This
> > > > allows the description of a display via its supported timing
> > > > parameters.
> > > > 
> > > > Every timing parameter can be specified as a single value or a range
> > > > <min typ max>.
> > > > 
> > > > Also, add helper functions to convert from display timings to a
> > > > generic videomode structure. This videomode can then be converted to
> > > > the corresponding subsystem mode representation (e.g. fb_videomode).
> > > > 
> > > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > > 
> > > Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm
> > > making an incorrect assumption.
> > > 
> > > It looks to me that the purpose of this entire series is to decode video
> > > timings from the device tree and (eventually) provide the data in the
> > > form 'struct videomode'. Correct?
> > > 
> > > If so, then it looks over engineered. Creating new infrastructure to
> > > allocate, maintain, and free a new 'struct display_timings' doesn't make
> > > any sense when it is an intermediary data format that will never be used
> > > by drivers.
> > > 
> > > Can the DT parsing code instead return a table of struct videomode?
> > > 
> > > But, wait... struct videomode is also a new structure. So it looks like
> > > this series creates two new intermediary data structures;
> > > display_timings and videomode. And at least as far as I can see in this
> > > series struct fb_videomode is the only user.
> > 
> > struct videomode is supposed to slowly replace the various video mode
> > structures we currently have in the kernel (struct drm_mode_modeinfo,
> > struct fb_videomode and struct v4l2_bt_timings), at least where possible
> > (userspace APIs can't be broken). This will make it possible to reuse
> > code across the DRM, FB and V4L2 subsystems, such as the EDID parser or
> > HDMI encoder drivers. This rationale might not be clearly explained in
> > the commit message, but having a shared video mode structure is pretty
> > important.
> 
> Okay that make sense. What about struct display_timings?

I'll let Steffen answer on that one as I (currently) have no strong feeling 
about it. If we were to remove it we would need to carry the number of 
timings, native timing and timings array around. That wouldn't be very 
practical for drivers, but it probably all depends on the usage drivers will 
make of that information.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 1/6] video: add display_timing and videomode
@ 2012-11-15 19:00           ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2012-11-15 19:00 UTC (permalink / raw)
  To: Grant Likely
  Cc: Steffen Trumtrar, devicetree-discuss, linux-fbdev, dri-devel,
	Tomi Valkeinen, kernel, Guennady Liakhovetski, linux-media

On Thursday 15 November 2012 18:03:59 Grant Likely wrote:
> On Thu, 15 Nov 2012 17:00:57 +0100, Laurent Pinchart wrote:
> > On Thursday 15 November 2012 15:47:53 Grant Likely wrote:
> > > On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar wrote:
> > > > Add display_timing structure and the according helper functions. This
> > > > allows the description of a display via its supported timing
> > > > parameters.
> > > > 
> > > > Every timing parameter can be specified as a single value or a range
> > > > <min typ max>.
> > > > 
> > > > Also, add helper functions to convert from display timings to a
> > > > generic videomode structure. This videomode can then be converted to
> > > > the corresponding subsystem mode representation (e.g. fb_videomode).
> > > > 
> > > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > > 
> > > Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm
> > > making an incorrect assumption.
> > > 
> > > It looks to me that the purpose of this entire series is to decode video
> > > timings from the device tree and (eventually) provide the data in the
> > > form 'struct videomode'. Correct?
> > > 
> > > If so, then it looks over engineered. Creating new infrastructure to
> > > allocate, maintain, and free a new 'struct display_timings' doesn't make
> > > any sense when it is an intermediary data format that will never be used
> > > by drivers.
> > > 
> > > Can the DT parsing code instead return a table of struct videomode?
> > > 
> > > But, wait... struct videomode is also a new structure. So it looks like
> > > this series creates two new intermediary data structures;
> > > display_timings and videomode. And at least as far as I can see in this
> > > series struct fb_videomode is the only user.
> > 
> > struct videomode is supposed to slowly replace the various video mode
> > structures we currently have in the kernel (struct drm_mode_modeinfo,
> > struct fb_videomode and struct v4l2_bt_timings), at least where possible
> > (userspace APIs can't be broken). This will make it possible to reuse
> > code across the DRM, FB and V4L2 subsystems, such as the EDID parser or
> > HDMI encoder drivers. This rationale might not be clearly explained in
> > the commit message, but having a shared video mode structure is pretty
> > important.
> 
> Okay that make sense. What about struct display_timings?

I'll let Steffen answer on that one as I (currently) have no strong feeling 
about it. If we were to remove it we would need to carry the number of 
timings, native timing and timings array around. That wouldn't be very 
practical for drivers, but it probably all depends on the usage drivers will 
make of that information.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 1/6] video: add display_timing and videomode
  2012-11-15 18:03       ` Grant Likely
@ 2012-11-16  8:53           ` Steffen Trumtrar
  2012-11-16  8:53           ` Steffen Trumtrar
  1 sibling, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-16  8:53 UTC (permalink / raw)
  To: Grant Likely
  Cc: Laurent Pinchart, devicetree-discuss, linux-fbdev, dri-devel,
	Tomi Valkeinen, kernel, Guennady Liakhovetski, linux-media

Hi Grant,

On Thu, Nov 15, 2012 at 06:03:59PM +0000, Grant Likely wrote:
> On Thu, 15 Nov 2012 17:00:57 +0100, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> > Hi Grant,
> > 
> > On Thursday 15 November 2012 15:47:53 Grant Likely wrote:
> > > On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar wrote:
> > > > Add display_timing structure and the according helper functions. This
> > > > allows the description of a display via its supported timing parameters.
> > > > 
> > > > Every timing parameter can be specified as a single value or a range
> > > > <min typ max>.
> > > > 
> > > > Also, add helper functions to convert from display timings to a generic
> > > > videomode structure. This videomode can then be converted to the
> > > > corresponding subsystem mode representation (e.g. fb_videomode).
> > > > 
> > > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > > 
> > > Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm
> > > making an incorrect assumption.
> > > 
> > > It looks to me that the purpose of this entire series is to decode video
> > > timings from the device tree and (eventually) provide the data in the
> > > form 'struct videomode'. Correct?
> > > 

For the time being it is straight from devicetree via struct videomode
to struct drm_display_mode or fb_videomode. Correct.

> > > If so, then it looks over engineered. Creating new infrastructure to
> > > allocate, maintain, and free a new 'struct display_timings' doesn't make
> > > any sense when it is an intermediary data format that will never be used
> > > by drivers.
> > > 
> > > Can the DT parsing code instead return a table of struct videomode?
> > > 

See below.

> > > But, wait... struct videomode is also a new structure. So it looks like
> > > this series creates two new intermediary data structures;
> > > display_timings and videomode. And at least as far as I can see in this
> > > series struct fb_videomode is the only user.

struct drm_display_mode is also a user in this series see 5/6 and 6/6.

> > 
> > struct videomode is supposed to slowly replace the various video mode 
> > structures we currently have in the kernel (struct drm_mode_modeinfo, struct 
> > fb_videomode and struct v4l2_bt_timings), at least where possible (userspace 
> > APIs can't be broken). This will make it possible to reuse code across the 
> > DRM, FB and V4L2 subsystems, such as the EDID parser or HDMI encoder drivers. 
> > This rationale might not be clearly explained in the commit message, but 
> > having a shared video mode structure is pretty important.
>

That.

> Okay that make sense. What about struct display_timings?
> 

The reason for defining an intermediary step is because of the different things
that are described:
- struct display_timing describes the signal ranges a display supports
- struct display_timings describes all timing settings of a display
- struct videomode describes one single mode generated from that settings

It is possible to generate multiple struct videomodes from one
struct display_timing based on the circumstances. And that is a task for the
driver using the display_timing infos. This means drivers are supposed to use
struct display_timings if they need to generate a struct videomode from the
timing ranges of one entry.
This is just the first step in that direction.
I hope this makes the need for struct display_timings a little clearer.
The other solution would be the one Laurent suggested and pass multiple values
around. Which in my opinion doesn't make it better, more practical or cleaner.


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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v10 1/6] video: add display_timing and videomode
@ 2012-11-16  8:53           ` Steffen Trumtrar
  0 siblings, 0 replies; 35+ messages in thread
From: Steffen Trumtrar @ 2012-11-16  8:53 UTC (permalink / raw)
  To: Grant Likely
  Cc: Laurent Pinchart, devicetree-discuss, linux-fbdev, dri-devel,
	Tomi Valkeinen, kernel, Guennady Liakhovetski, linux-media

Hi Grant,

On Thu, Nov 15, 2012 at 06:03:59PM +0000, Grant Likely wrote:
> On Thu, 15 Nov 2012 17:00:57 +0100, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> > Hi Grant,
> > 
> > On Thursday 15 November 2012 15:47:53 Grant Likely wrote:
> > > On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar wrote:
> > > > Add display_timing structure and the according helper functions. This
> > > > allows the description of a display via its supported timing parameters.
> > > > 
> > > > Every timing parameter can be specified as a single value or a range
> > > > <min typ max>.
> > > > 
> > > > Also, add helper functions to convert from display timings to a generic
> > > > videomode structure. This videomode can then be converted to the
> > > > corresponding subsystem mode representation (e.g. fb_videomode).
> > > > 
> > > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > > 
> > > Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm
> > > making an incorrect assumption.
> > > 
> > > It looks to me that the purpose of this entire series is to decode video
> > > timings from the device tree and (eventually) provide the data in the
> > > form 'struct videomode'. Correct?
> > > 

For the time being it is straight from devicetree via struct videomode
to struct drm_display_mode or fb_videomode. Correct.

> > > If so, then it looks over engineered. Creating new infrastructure to
> > > allocate, maintain, and free a new 'struct display_timings' doesn't make
> > > any sense when it is an intermediary data format that will never be used
> > > by drivers.
> > > 
> > > Can the DT parsing code instead return a table of struct videomode?
> > > 

See below.

> > > But, wait... struct videomode is also a new structure. So it looks like
> > > this series creates two new intermediary data structures;
> > > display_timings and videomode. And at least as far as I can see in this
> > > series struct fb_videomode is the only user.

struct drm_display_mode is also a user in this series see 5/6 and 6/6.

> > 
> > struct videomode is supposed to slowly replace the various video mode 
> > structures we currently have in the kernel (struct drm_mode_modeinfo, struct 
> > fb_videomode and struct v4l2_bt_timings), at least where possible (userspace 
> > APIs can't be broken). This will make it possible to reuse code across the 
> > DRM, FB and V4L2 subsystems, such as the EDID parser or HDMI encoder drivers. 
> > This rationale might not be clearly explained in the commit message, but 
> > having a shared video mode structure is pretty important.
>

That.

> Okay that make sense. What about struct display_timings?
> 

The reason for defining an intermediary step is because of the different things
that are described:
- struct display_timing describes the signal ranges a display supports
- struct display_timings describes all timing settings of a display
- struct videomode describes one single mode generated from that settings

It is possible to generate multiple struct videomodes from one
struct display_timing based on the circumstances. And that is a task for the
driver using the display_timing infos. This means drivers are supposed to use
struct display_timings if they need to generate a struct videomode from the
timing ranges of one entry.
This is just the first step in that direction.
I hope this makes the need for struct display_timings a little clearer.
The other solution would be the one Laurent suggested and pass multiple values
around. Which in my opinion doesn't make it better, more practical or cleaner.


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

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2012-11-16  8:53 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-15  9:23 [PATCH v10 0/6] of: add display helper Steffen Trumtrar
2012-11-15  9:23 ` Steffen Trumtrar
2012-11-15  9:23 ` [PATCH v10 1/6] video: add display_timing and videomode Steffen Trumtrar
2012-11-15  9:23   ` Steffen Trumtrar
2012-11-15 15:47   ` Grant Likely
2012-11-15 15:47     ` Grant Likely
2012-11-15 16:00     ` Laurent Pinchart
2012-11-15 16:00       ` Laurent Pinchart
2012-11-15 18:03       ` Grant Likely
2012-11-15 18:59         ` Laurent Pinchart
2012-11-15 19:00           ` Laurent Pinchart
2012-11-16  8:53         ` Steffen Trumtrar
2012-11-16  8:53           ` Steffen Trumtrar
2012-11-15  9:23 ` [PATCH v10 2/6] video: add of helper for videomode Steffen Trumtrar
2012-11-15  9:23   ` Steffen Trumtrar
2012-11-15  9:23 ` [PATCH v10 3/6] fbmon: add videomode helpers Steffen Trumtrar
2012-11-15  9:23   ` Steffen Trumtrar
2012-11-15  9:23 ` [PATCH v10 4/6] fbmon: add of_videomode helpers Steffen Trumtrar
2012-11-15  9:23   ` Steffen Trumtrar
2012-11-15  9:23 ` [PATCH v10 5/6] drm_modes: add videomode helpers Steffen Trumtrar
2012-11-15  9:23   ` Steffen Trumtrar
2012-11-15  9:23 ` [PATCH v10 6/6] drm_modes: add of_videomode helpers Steffen Trumtrar
2012-11-15  9:23   ` Steffen Trumtrar
     [not found]   ` <1352971437-29877-7-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-11-15  9:58     ` Thierry Reding
2012-11-15  9:58       ` Thierry Reding
2012-11-15  9:58       ` Thierry Reding
2012-11-15 10:01       ` Steffen Trumtrar
2012-11-15 10:01         ` Steffen Trumtrar
2012-11-15 10:24 ` [PATCH v10 0/6] of: add display helper Thierry Reding
2012-11-15 10:24   ` Thierry Reding
     [not found]   ` <20121115102411.GA17272-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
2012-11-15 10:27     ` Thierry Reding
2012-11-15 10:27       ` Thierry Reding
2012-11-15 10:27       ` Thierry Reding
2012-11-15 10:28   ` Steffen Trumtrar
2012-11-15 10:28     ` Steffen Trumtrar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.