[PATCH v9 1/6] video: add display_timing and videomode
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.trumt...@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 | 39 +++
6 files changed, 185 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 000..ac9bbbc
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@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 000..087374a
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,45 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@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 000..caee2a8
--- /dev/null
+++ b/include/linux/display_timing.h
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2012 Steffen Trumtrar s.trumt...@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
[PATCH v9 5/6] drm_modes: add videomode helpers
Add conversion from videomode to drm_display_mode
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/gpu/drm/drm_modes.c | 36
include/drm/drmP.h |6 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 59450f3..42ea099 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,41 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int
vdisplay, int vrefresh,
}
EXPORT_SYMBOL(drm_gtf_mode);
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int 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(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..1e0d846 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 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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 0/6] of: add display helper
Hi! Changes since v8: - fix memory leaks - change API to be more consistent (foo_from_bar(struct bar, struct foo)) - include headers were necessary - misc minor bufixes 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| 69 +++ drivers/video/Kconfig | 19 ++ drivers/video/Makefile |4 + drivers/video/display_timing.c | 24 +++ drivers/video/fbmon.c | 78 drivers/video/of_display_timing.c | 211 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 | 11 + include/linux/of_display_timings.h | 20 ++ include/linux/of_videomode.h | 16 ++ include/linux/videomode.h | 39 15 files changed, 771 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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 3/6] fbmon: add videomode helpers
Add a function to convert from the generic videomode to a fb_videomode.
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/video/fbmon.c | 38 ++
include/linux/fb.h|5 +
2 files changed, 43 insertions(+)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..cccef17 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,43 @@ 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)
+{
+ 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-refresh = (vm-pixelclock*1000) / (vm-hactive * vm-vactive);
+ fbmode-flag = 0;
+
+ 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..6a3a675 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,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_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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 4/6] fbmon: add of_videomode helpers
Add helper to get fb_videomode from devicetree.
Signed-off-by: Steffen Trumtrar s.trumt...@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 cccef17..3a48abd 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
@@ -1410,6 +1410,46 @@ int fb_videomode_from_videomode(struct videomode *vm,
struct fb_videomode *fbmod
EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
#endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static 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 6a3a675..8aeece8 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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 2/6] video: add of helper for videomode
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.trumt...@pengutronix.de
Signed-off-by: Philipp Zabel p.za...@pengutronix.de
Acked-by: Stephen Warren swar...@nvidia.com
---
.../devicetree/bindings/video/display-timings.txt | 107 ++
drivers/video/Kconfig | 13 ++
drivers/video/Makefile |2 +
drivers/video/of_display_timing.c | 211
drivers/video/of_videomode.c | 47 +
include/linux/of_display_timings.h | 20 ++
include/linux/of_videomode.h | 16 ++
7 files changed, 416 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 000..c9d9e1b
--- /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: displayclock 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
Re: [PATCH v9 2/6] video: add of helper for videomode
On Wed, Nov 14, 2012 at 01:00:45PM +0100, Thierry Reding wrote:
On Wed, Nov 14, 2012 at 12:43:19PM +0100, Steffen Trumtrar wrote:
[...]
+display-timings bindings
+
+
+display timings node
I didn't express myself very clearly here =). The way I think this
should be written is display-timings node.
+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: displayclock in Hz
I still think displayclock should be two words: display clock.
+/**
+ * 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)
+{
[...]
+ 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;
I believe you're still potentially leaking memory here. In case you have
5 entries for instance, and the last one fails to parse, then this will
cause the memory allocated for the 4 correct entries to be lost.
Can't you just call display_timings_release() in this case and then jump
to dispfail? That would still leak the native_mode device node. Maybe it
would be better to keep timingfail but modify it to free the display
timings with display_timings_release() instead? See below.
+ }
+
+ 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);
+ kfree(disp-timings);
Call display_timings_release(disp) instead of kfree(disp-timings)?
Yes. That would be the appropriate way to fail here. Done.
diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
new file mode 100644
index 000..4138758
--- /dev/null
+++ b/include/linux/of_videomode.h
@@ -0,0 +1,16 @@
+/*
+ * Copyright 2012 Steffen Trumtrar s.trumt...@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 */
Nit: should have a blank line before #endif.
Thierry
___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
--
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/6] fbmon: add videomode helpers
On Wed, Nov 14, 2012 at 01:12:07PM +0100, Thierry Reding wrote:
On Wed, Nov 14, 2012 at 12:43:20PM +0100, Steffen Trumtrar wrote:
Add a function to convert from the generic videomode to a fb_videomode.
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/video/fbmon.c | 38 ++
include/linux/fb.h|5 +
2 files changed, 43 insertions(+)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..cccef17 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,43 @@ 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)
+{
+ 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-refresh = (vm-pixelclock*1000) / (vm-hactive * vm-vactive);
CodingStyle that you should have spaces around '*'. Also I'm not sure if
that formula is correct. Shouldn't the blanking intervals be counted as
well? So:
unsigned int htotal = vm-hactive + vm-hfront_porch +
vm-hback_porch + vm-hsync_len;
unsigned int vtotal = vm-vactive + vm-vfront_porch +
vm-vback_porch + vm-vsync_len;
fbmode-refresh = (vm-pixelclock * 1000) / (htotal * vtotal);
?
Sounds correct. Done.
+ fbmode-flag = 0;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
+#endif
+
+
Gratuitous blank line.
#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..6a3a675 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,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_VIDEOMODE)
+extern int fb_videomode_from_videomode(struct videomode *vm,
+ struct fb_videomode *fbmode);
+#endif
/* drivers/video/modedb.c */
These in turn could use an extra blank line.
Thierry
___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
--
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 5/6] drm_modes: add videomode helpers
On Wed, Nov 14, 2012 at 01:49:44PM +0100, Thierry Reding wrote: On Wed, Nov 14, 2012 at 12:43:22PM +0100, Steffen Trumtrar wrote: [...] diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c [...] @@ -504,6 +505,41 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh, } EXPORT_SYMBOL(drm_gtf_mode); +#if IS_ENABLED(CONFIG_VIDEOMODE) +int display_mode_from_videomode(struct videomode *vm, struct drm_display_mode *dmode) Given that this is still a DRM core function, maybe it should get a drm_ prefix? Also the line is too long, so you may want to wrap the argument list. Thierry Yes, seems to fit better to the rest of the file. 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- | -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/6] video: add of helper for videomode
Hi!
On Mon, Nov 12, 2012 at 07:58:40PM +0100, Sascha Hauer wrote:
Hi Steffen,
You lose memory in several places:
On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote:
+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__);
+ return NULL;
Here
+ }
+
+ 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)
+ return -ENOMEM;
+
+ 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 (!entry) {
+ pr_err(%s: no timing specifications given\n, __func__);
+ return NULL;
Here
+ }
+
+ pr_info(%s: using %s as default timing\n, __func__, entry-name);
+
+ native_mode = entry;
+
+ disp-num_timings = of_get_child_count(timings_np);
+ disp-timings = kzalloc(sizeof(struct display_timing
*)*disp-num_timings,
+ GFP_KERNEL);
+ if (!disp-timings)
+ return -ENOMEM;
Here
+
+ 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);
+ return NULL;
Here
+ }
+
+ if (native_mode == entry)
+ disp-native_mode = disp-num_timings;
+
+ disp-timings[disp-num_timings] = dt;
+ disp-num_timings++;
+ }
+ of_node_put(timings_np);
+
+ 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__);
+ return NULL;
and here
Well,... 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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/6] video: add of helper for videomode
On Mon, Nov 12, 2012 at 11:00:37PM +0400, Alexey Klimov wrote: Hello Steffen, On Mon, Nov 12, 2012 at 7:37 PM, Steffen Trumtrar s.trumt...@pengutronix.de wrote: 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.trumt...@pengutronix.de Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- .../devicetree/bindings/video/display-timings.txt | 107 +++ drivers/video/Kconfig | 13 ++ drivers/video/Makefile |2 + drivers/video/of_display_timing.c | 186 drivers/video/of_videomode.c | 47 + include/linux/of_display_timings.h | 19 ++ include/linux/of_videomode.h | 15 ++ 7 files changed, 389 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 000..e22e2fd --- /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: displayclock 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
Re: [PATCH v8 2/6] video: add of helper for videomode
On Mon, Nov 12, 2012 at 01:40:12PM -0700, Stephen Warren wrote: On 11/12/2012 08:37 AM, Steffen Trumtrar wrote: 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. diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt The device tree bindings look reasonable to me, so, Acked-by: Stephen Warren swar...@nvidia.com Thanks, 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- | -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 1/6] video: add display_timing and videomode
On Tue, Nov 13, 2012 at 11:41:59AM +0100, Thierry Reding wrote:
On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
[...]
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
DISPLAY_TIMINGS?
#video output switch sysfs driver
obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
display_timings.o?
+obj-$(CONFIG_VIDEOMODE) += videomode.o
diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
display_timings.c?
I originally had that and changed it by request to the singular form.
(Can't find the mail atm). And I think this fits better with all the other
drivers.
+int videomode_from_timing(struct display_timings *disp, struct videomode
*vm,
+ unsigned int index)
I find the indexing API a bit confusing. But that's perhaps just a
matter of personal preference.
Also the ordering of arguments seems a little off. I find it more
natural to have the destination pointer in the first argument, similar
to the memcpy() function, so this would be:
int videomode_from_timing(struct videomode *vm, struct display_timings *disp,
unsigned int index);
Actually, when reading videomode_from_timing() I'd expect the argument
list to be:
int videomode_from_timing(struct videomode *vm, struct display_timing
*timing);
Am I the only one confused by this?
I went with the of_xxx-functions that have fname(from_node, to_property)
and personally prefer it this way. Therefore I'd like to keep it as is.
diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
display_timings.h?
+/* placeholder function until ranges are really needed
The above line has trailing whitespace. Also the block comment should
have the opening /* on a separate line.
Okay.
+ * the index parameter should then be used to select one of [min typ max]
If index is supposed to select min, typ or max, then maybe an enum would
be a better candidate? Or alternatively provide separate accessors, like
display_timing_get_{minimum,typical,maximum}().
Hm, I'm not so sure about this one. I'd prefer the enum.
+ */
+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 timings_release(struct display_timings *disp);
This function no longer exists.
Right.
Steffen
___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
--
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 3/6] fbmon: add videomode helpers
On Tue, Nov 13, 2012 at 12:22:58PM +0100, Thierry Reding wrote:
On Mon, Nov 12, 2012 at 04:37:03PM +0100, Steffen Trumtrar wrote:
[...]
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode
*fbmode)
The other helpers are named destination-type_from_source-type(),
maybe this should follow that example for consistency?
Hm, not a bad idea.
+{
+ 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-refresh = 60;
Can the refresh rate not be computed from the pixel clock and the
horizontal and vertical timings?
Yes. That totally got lost on the way.
+ fbmode-flag = 0;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_to_fb_videomode);
+#endif
+
+
There's a gratuitous blank line here.
#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..46c665b 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -714,6 +714,8 @@ 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);
+extern int videomode_to_fb_videomode(struct videomode *vm, struct
fb_videomode *fbmode);
+
Should you provide a dummy in the !CONFIG_VIDEOMODE case?
Okay
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 6/6] drm_modes: add of_videomode helpers
On Tue, Nov 13, 2012 at 12:35:18PM +0100, Thierry Reding wrote:
On Mon, Nov 12, 2012 at 04:37:06PM +0100, Steffen Trumtrar wrote:
[...]
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static void dump_drm_displaymode(struct drm_display_mode *m)
+{
+ pr_debug(drm_displaymode = %d %d %d %d %d %d %d %d %d\n,
+m-hdisplay, m-hsync_start, m-hsync_end, m-htotal,
+m-vdisplay, m-vsync_start, m-vsync_end, m-vtotal,
+m-clock);
I seem to remember a comment to an earlier version of this patch
requesting better formatting of this string. Alternatively you might
want to consider replacing it using drm_mode_debug_printmodeline().
Ah, yes. I only did that for fb_videomode and forgot about this one.
But the existing function is even better.
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
[...]
@@ -1457,6 +1458,10 @@ drm_mode_create_from_cmdline_mode(struct drm_device
*dev,
extern int videomode_to_display_mode(struct videomode *vm,
struct drm_display_mode *dmode);
+extern int of_get_drm_display_mode(struct device_node *np,
+ struct drm_display_mode *dmode,
+ int index);
Also requires either a dummy or protection.
Thierry
--
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 2/6] video: add of helper for videomode
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.trumt...@pengutronix.de
Signed-off-by: Philipp Zabel p.za...@pengutronix.de
---
.../devicetree/bindings/video/display-timings.txt | 107 +++
drivers/video/Kconfig | 13 ++
drivers/video/Makefile |2 +
drivers/video/of_display_timing.c | 186
drivers/video/of_videomode.c | 47 +
include/linux/of_display_timings.h | 19 ++
include/linux/of_videomode.h | 15 ++
7 files changed, 389 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 000..e22e2fd
--- /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: displayclock 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
Re: [PATCH v7 0/8] of: add display helper
On Thu, Nov 08, 2012 at 03:35:47PM -0600, Rob Herring wrote: On 10/31/2012 04:28 AM, Steffen Trumtrar wrote: Hi! Finally, v7 of the series. Changes since v6: - get rid of some empty lines etc. - move functions to their subsystems - split of_ from non-of_ functions - add at least some kerneldoc to some functions Regards, Steffen Steffen Trumtrar (8): video: add display_timing struct and helpers of: add helper to parse display timings of: add generic videomode description video: add videomode helpers 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 | 139 +++ drivers/gpu/drm/drm_modes.c| 78 + drivers/of/Kconfig | 12 ++ drivers/of/Makefile|2 + drivers/of/of_display_timings.c| 185 drivers/of/of_videomode.c | 47 + Not sure why you moved this, but please move this back to drivers/video. We're trying to move subsystem specific pieces out of drivers/of. Rob Hm, the of_xxx part always was in drivers/of, but I can move that. No problem. 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- | -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 5/8] fbmon: add videomode helpers
Hi!
On Fri, Nov 09, 2012 at 04:54:16PM +, Manjunathappa, Prakash wrote:
Hi Steffen,
On Fri, Nov 09, 2012 at 02:55:45, Steffen Trumtrar wrote:
Hi!
On Wed, Oct 31, 2012 at 03:30:03PM +, Manjunathappa, Prakash wrote:
Hi Steffen,
On Wed, Oct 31, 2012 at 14:58:05, Steffen Trumtrar wrote:
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int videomode_to_fb_videomode(struct videomode *vm, struct
fb_videomode *fbmode)
+{
+ 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;
+
pixelclk-inverted property of the panel is not percolated fb_videomode.
Please let me know if I am missing something.
The next version is almost finished. Only thing I'm missing is this.
And I actually do not know which flag would represent an inverted pixelclock
in fb_videomode. Does anybody have any idea what I have to do here?
if (vm-pixelclk_pol)
fbmode-sync = ???
That's as far as I have come and I don't see a flag that seems right.
Is this even a valid property of fb_videomode?
Thanks for considering it, I see IMX addresses it as proprietary FB_SYNC_
flag.
FB_SYNC_CLK_INVERT: arch/arm/plat-mxc/include/mach/mx3fb.h
No problem. So, it seems this flag has to be set in some imx-specific
videomode_to_fb_videomode function. It is included in the
struct videomode, so that should be no problem. But it will not be
part of this series.
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 5/8] fbmon: add videomode helpers
Hi!
On Wed, Oct 31, 2012 at 03:30:03PM +, Manjunathappa, Prakash wrote:
Hi Steffen,
On Wed, Oct 31, 2012 at 14:58:05, Steffen Trumtrar wrote:
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode
*fbmode)
+{
+ 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;
+
pixelclk-inverted property of the panel is not percolated fb_videomode.
Please let me know if I am missing something.
The next version is almost finished. Only thing I'm missing is this.
And I actually do not know which flag would represent an inverted pixelclock
in fb_videomode. Does anybody have any idea what I have to do here?
if (vm-pixelclk_pol)
fbmode-sync = ???
That's as far as I have come and I don't see a flag that seems right.
Is this even a valid property of fb_videomode?
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/8] video: add display_timing struct and helpers
On Thu, Nov 01, 2012 at 09:08:42PM +0100, Thierry Reding wrote:
On Wed, Oct 31, 2012 at 10:28:01AM +0100, Steffen Trumtrar wrote:
[...]
+void timings_release(struct display_timings *disp)
+{
+ int i;
+
+ for (i = 0; i disp-num_timings; i++)
+ kfree(disp-timings[i]);
+}
+
+void display_timings_release(struct display_timings *disp)
+{
+ timings_release(disp);
+ kfree(disp-timings);
+}
I'm not quite sure I understand how these are supposed to be used. The
only use-case where a struct display_timings is dynamically allocated is
for the OF helpers. In that case, wouldn't it be more useful to have a
function that frees the complete structure, including the struct
display_timings itself? Something like this, which has all of the above
rolled into one:
void display_timings_free(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);
}
Well, you are right. They can be rolled into one function.
The extra function call is useless and as it seems confusing.
Regards,
Steffen
___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
--
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/8] of: add helper to parse display timings
On Thu, Nov 01, 2012 at 09:15:10PM +0100, Thierry Reding wrote: On Wed, Oct 31, 2012 at 10:28:02AM +0100, Steffen Trumtrar wrote: [...] diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt [...] @@ -0,0 +1,139 @@ +display-timings bindings +== + +display-timings-node + Maybe extend the underline to the length of the section and subsection titles respectively? +struct display_timing +=== Same here. +config OF_DISPLAY_TIMINGS + def_bool y + depends on DISPLAY_TIMING Maybe this should be called OF_DISPLAY_TIMING to match DISPLAY_TIMING, or rename DISPLAY_TIMING to DISPLAY_TIMINGS for the sake of consistency? Yes, to all three above. +/** + * of_get_display_timing_list - parse all display_timing entries from a device_node + * @np: device_node with the subnodes + **/ +struct display_timings *of_get_display_timing_list(struct device_node *np) Perhaps this would better be named of_get_display_timings() to match the return type? Hm, I'm not really sure about that. I found it to error prone, to have a function of_get_display_timing and of_get_display_timings. That's why I chose of_get_display_timing_list. But you are correct, that it doesn't match the return value. Maybe I should just make the first function static and change the name as you suggested. + disp = kzalloc(sizeof(*disp), GFP_KERNEL); Shouldn't you be checking this for allocation failures? + disp-timings = kzalloc(sizeof(struct display_timing *)*disp-num_timings, + GFP_KERNEL); Same here. Yes, to both. 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- | -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/8] of: add helper to parse display timings
Hi!
On Fri, Nov 02, 2012 at 10:49:47PM +0530, Leela Krishna Amudala wrote:
Hello Steffen,
On Wed, Oct 31, 2012 at 2:58 PM, Steffen Trumtrar
+static int parse_property(struct device_node *np, char *name,
+ struct timing_entry *result)
+{
+ struct property *prop;
+ int length;
+ int cells;
+ int 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_array(np, name, result-typ,
cells);
As you are reading only one vaue, you can use of_property_read_u32 instead.
Yes, thats copypasta, no need for _array here.
+ result-min = result-typ;
+ result-max = result-typ;
+ } else if (cells == 3) {
+ ret = of_property_read_u32_array(np, name, result-min,
cells);
You are considering only min element, what about typ and max elements?
I start at the address of result-min and read three u32-values, therefore all
three (min,typ,max) are filled with values.
+
+/**
+ * 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;
+ struct device_node *default_np;
+
+ if (!np)
+ return -EINVAL;
+
+ timings_np = of_parse_phandle(np, display-timings, 0);
+ if (!timings_np)
+ return -EINVAL;
+
+ return -EINVAL;
Here it should return success instead of -EINVAL.
Yes.
And one query.. are the binding properties names and display-timings
node structure template finalized..?
I sure hope so. There actually is one error in the examples though.
The property clock is called clock-frequency. I included it correctly
at the top of display-timings.txt, but overlooked it in the examples.
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 0/8] of: add display helper
Hi! Finally, v7 of the series. Changes since v6: - get rid of some empty lines etc. - move functions to their subsystems - split of_ from non-of_ functions - add at least some kerneldoc to some functions Regards, Steffen Steffen Trumtrar (8): video: add display_timing struct and helpers of: add helper to parse display timings of: add generic videomode description video: add videomode helpers 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 | 139 +++ drivers/gpu/drm/drm_modes.c| 78 + drivers/of/Kconfig | 12 ++ drivers/of/Makefile|2 + drivers/of/of_display_timings.c| 185 drivers/of/of_videomode.c | 47 + drivers/video/Kconfig | 11 ++ drivers/video/Makefile |2 + drivers/video/display_timing.c | 24 +++ drivers/video/fbmon.c | 76 drivers/video/videomode.c | 44 + include/drm/drmP.h |8 + include/linux/display_timing.h | 69 include/linux/fb.h |5 + include/linux/of_display_timings.h | 20 +++ include/linux/of_videomode.h | 15 ++ include/linux/videomode.h | 36 17 files changed, 773 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt create mode 100644 drivers/of/of_display_timings.c create mode 100644 drivers/of/of_videomode.c 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/of_display_timings.h create mode 100644 include/linux/of_videomode.h create mode 100644 include/linux/videomode.h -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 1/8] video: add display_timing struct and helpers
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.
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/video/Kconfig |5 +++
drivers/video/Makefile |1 +
drivers/video/display_timing.c | 24 ++
include/linux/display_timing.h | 69
4 files changed, 99 insertions(+)
create mode 100644 drivers/video/display_timing.c
create mode 100644 include/linux/display_timing.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d08d799..1421fc8 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -33,6 +33,11 @@ config VIDEO_OUTPUT_CONTROL
This framework adds support for low-level control of the video
output switch.
+config DISPLAY_TIMING
+ bool Enable display timings helpers
+ help
+ Say Y here, to use the display timing helpers.
+
menuconfig FB
tristate Support for frame buffer devices
---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 23e948e..552c045 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -167,3 +167,4 @@ 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
diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
new file mode 100644
index 000..9ccfdb3
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include linux/slab.h
+#include linux/display_timing.h
+
+void timings_release(struct display_timings *disp)
+{
+ int i;
+
+ for (i = 0; i disp-num_timings; i++)
+ kfree(disp-timings[i]);
+}
+
+void display_timings_release(struct display_timings *disp)
+{
+ timings_release(disp);
+ kfree(disp-timings);
+}
diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
new file mode 100644
index 000..aa02a12
--- /dev/null
+++ b/include/linux/display_timing.h
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2012 Steffen Trumtrar s.trumt...@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 */
+static inline u32 display_timing_get_value(struct timing_entry *te, int index)
+{
+ return te-typ;
+}
+
+static inline struct display_timing *display_timings_get(struct
display_timings *disp,
+int index)
+{
+ struct display_timing *dt;
+
+ if (disp-num_timings index) {
+ dt = disp-timings[index];
+ return dt;
+ } else
+ return NULL;
+}
+
+void timings_release(struct display_timings *disp);
+void display_timings_release(struct display_timings *disp);
+
+#endif
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 2/8] of: add helper to parse display timings
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
.../devicetree/bindings/video/display-timings.txt | 139 +++
drivers/of/Kconfig |6 +
drivers/of/Makefile|1 +
drivers/of/of_display_timings.c| 185
include/linux/of_display_timings.h | 20 +++
5 files changed, 351 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
create mode 100644 drivers/of/of_display_timings.c
create mode 100644 include/linux/of_display_timings.h
diff --git a/Documentation/devicetree/bindings/video/display-timings.txt
b/Documentation/devicetree/bindings/video/display-timings.txt
new file mode 100644
index 000..04c94a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timings.txt
@@ -0,0 +1,139 @@
+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: displayclock 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 = 5200;
+ hactive = 1920;
+ vactive = 1080;
+ hfront-porch = 25;
+ hback-porch = 25;
+ hsync-len = 25;
+ vback-porch = 2;
+ vfront-porch = 2;
+ vsync-len = 2;
+ hsync-active = 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
[PATCH v7 4/8] video: add videomode helpers
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.trumt...@pengutronix.de
---
drivers/video/Kconfig |6 ++
drivers/video/Makefile|1 +
drivers/video/videomode.c | 44
include/linux/videomode.h | 36
4 files changed, 87 insertions(+)
create mode 100644 drivers/video/videomode.c
create mode 100644 include/linux/videomode.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 1421fc8..45dd393 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -38,6 +38,12 @@ config DISPLAY_TIMING
help
Say Y here, to use the display timing helpers.
+config VIDEOMODE
+ bool Enable videomode helpers
+ help
+ Say Y here, to use the generic videomode helpers. This allows
+converting from display timings to fb_videomode and drm_display_mode
+
menuconfig FB
tristate Support for frame buffer devices
---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 552c045..fc30439 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -168,3 +168,4 @@ 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/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 000..a9fe010
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,44 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include linux/kernel.h
+#include linux/export.h
+#include linux/errno.h
+#include linux/display_timing.h
+#include linux/videomode.h
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+ int index)
+{
+ struct display_timing *dt = NULL;
+
+ dt = display_timings_get(disp, index);
+ if (!dt) {
+ pr_err(%s: no signal timings found\n, __func__);
+ 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-interlaced = dt-interlaced;
+ vm-doublescan = dt-doublescan;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_from_timing);
diff --git a/include/linux/videomode.h b/include/linux/videomode.h
new file mode 100644
index 000..f932147
--- /dev/null
+++ b/include/linux/videomode.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2012 Steffen Trumtrar s.trumt...@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;
+ bool interlaced;
+ bool doublescan;
+};
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+ int index);
+#endif
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 5/8] fbmon: add videomode helpers
Add a function to convert from the generic videomode to a fb_videomode.
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/video/fbmon.c | 36
include/linux/fb.h|2 ++
2 files changed, 38 insertions(+)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..b9e6ab3 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1373,6 +1373,42 @@ 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 videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode
*fbmode)
+{
+ 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;
+
+ fbmode-refresh = 60;
+ fbmode-flag = 0;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_to_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 c7a9571..46c665b 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -714,6 +714,8 @@ 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);
+extern int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode
*fbmode);
+
/* drivers/video/modedb.c */
#define VESA_MODEDB_SIZE 34
extern void fb_var_to_videomode(struct fb_videomode *mode,
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 6/8] fbmon: add of_videomode helpers
Add helper to get fb_videomode from devicetree.
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/video/fbmon.c | 40
include/linux/fb.h|3 +++
2 files changed, 43 insertions(+)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index b9e6ab3..871aa76 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1408,6 +1408,46 @@ int videomode_to_fb_videomode(struct videomode *vm,
struct fb_videomode *fbmode)
EXPORT_SYMBOL_GPL(videomode_to_fb_videomode);
#endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static 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_timing_list
ond
+ * work with that instead.
+ */
+int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+ int index)
+{
+ struct videomode vm;
+ int ret;
+
+ ret = of_get_videomode(np, vm, index);
+ if (ret)
+ return ret;
+
+ videomode_to_fb_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 46c665b..9892fd6 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -14,6 +14,8 @@
#include linux/backlight.h
#include linux/slab.h
#include asm/io.h
+#include linux/of.h
+#include linux/of_videomode.h
struct vm_area_struct;
struct fb_info;
@@ -714,6 +716,7 @@ 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);
+extern int of_get_fb_videomode(struct device_node *np, struct fb_videomode
*fb, int index);
extern int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode
*fbmode);
/* drivers/video/modedb.c */
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 7/8] drm_modes: add videomode helpers
Add conversion from videomode to drm_display_mode
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/gpu/drm/drm_modes.c | 36
include/drm/drmP.h |3 +++
2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 59450f3..049624d 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,41 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int
vdisplay, int vrefresh,
}
EXPORT_SYMBOL(drm_gtf_mode);
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int videomode_to_display_mode(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(videomode_to_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 3fd8280..e9fa1e3 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,8 @@ extern struct drm_display_mode *
drm_mode_create_from_cmdline_mode(struct drm_device *dev,
struct drm_cmdline_mode *cmd);
+extern int videomode_to_display_mode(struct videomode *vm,
+struct drm_display_mode *dmode);
/* 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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 3/8] of: add generic videomode description
Get videomode from devicetree in a format appropriate for the
backend. drm_display_mode and fb_videomode are supported atm.
Uses the display signal timings from of_display_timings
Signed-off-by: Philipp Zabel p.za...@pengutronix.de
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/of/Kconfig |6 ++
drivers/of/Makefile |1 +
drivers/of/of_videomode.c| 47 ++
include/linux/of_videomode.h | 15 ++
4 files changed, 69 insertions(+)
create mode 100644 drivers/of/of_videomode.c
create mode 100644 include/linux/of_videomode.h
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 781e773..0575ffe 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -89,4 +89,10 @@ config OF_DISPLAY_TIMINGS
help
helper to parse display timings from the devicetree
+config OF_VIDEOMODE
+ def_bool y
+ depends on VIDEOMODE
+ help
+ helper to get videomodes from the devicetree
+
endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index c8e9603..09d556f 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
obj-$(CONFIG_OF_MTD) += of_mtd.o
obj-$(CONFIG_OF_DISPLAY_TIMINGS) += of_display_timings.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
new file mode 100644
index 000..91a26fc
--- /dev/null
+++ b/drivers/of/of_videomode.c
@@ -0,0 +1,47 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@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_timing_list instead.
+ **/
+int of_get_videomode(struct device_node *np, struct videomode *vm, int index)
+{
+ struct display_timings *disp;
+ int ret;
+
+ disp = of_get_display_timing_list(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_videomode.h b/include/linux/of_videomode.h
new file mode 100644
index 000..01518e6
--- /dev/null
+++ b/include/linux/of_videomode.h
@@ -0,0 +1,15 @@
+/*
+ * Copyright 2012 Steffen Trumtrar s.trumt...@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
+
+int of_get_videomode(struct device_node *np, struct videomode *vm, int index);
+#endif /* __LINUX_OF_VIDEOMODE_H */
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 8/8] drm_modes: add of_videomode helpers
Add helper to get drm_display_mode from devicetree.
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/gpu/drm/drm_modes.c | 42 ++
include/drm/drmP.h |5 +
2 files changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 049624d..d51afe6 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/of.h
#include linux/videomode.h
/**
@@ -540,6 +541,47 @@ int videomode_to_display_mode(struct videomode *vm, struct
drm_display_mode *dmo
EXPORT_SYMBOL_GPL(videomode_to_display_mode);
#endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static void dump_drm_displaymode(struct drm_display_mode *m)
+{
+ pr_debug(drm_displaymode = %d %d %d %d %d %d %d %d %d\n,
+m-hdisplay, m-hsync_start, m-hsync_end, m-htotal,
+m-vdisplay, m-vsync_start, m-vsync_end, m-vtotal,
+m-clock);
+}
+
+/**
+ * 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
+ *
+ * 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_timing_list
ond
+ * work with that instead.
+ */
+int of_get_drm_display_mode(struct device_node *np, struct drm_display_mode
*dmode,
+ int index)
+{
+ struct videomode vm;
+ int ret;
+
+ ret = of_get_videomode(np, vm, index);
+ if (ret)
+ return ret;
+
+ videomode_to_display_mode(vm, dmode);
+
+ pr_info(%s: got %dx%d display mode from %s\n, __func__, vm.hactive,
+ vm.vactive, np-name);
+ dump_drm_displaymode(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 e9fa1e3..4f9c44e 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 */
@@ -1457,6 +1458,10 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
extern int videomode_to_display_mode(struct videomode *vm,
struct drm_display_mode *dmode);
+extern int of_get_drm_display_mode(struct device_node *np,
+ struct drm_display_mode *dmode,
+ int index);
+
/* 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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 5/8] fbmon: add videomode helpers
Hi Prakash!
On Wed, Oct 31, 2012 at 03:30:03PM +, Manjunathappa, Prakash wrote:
Hi Steffen,
On Wed, Oct 31, 2012 at 14:58:05, Steffen Trumtrar wrote:
Add a function to convert from the generic videomode to a fb_videomode.
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/video/fbmon.c | 36
include/linux/fb.h|2 ++
2 files changed, 38 insertions(+)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..b9e6ab3 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1373,6 +1373,42 @@ 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 videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode
*fbmode)
+{
+ 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;
+
pixelclk-inverted property of the panel is not percolated fb_videomode.
Please let me know if I am missing something.
You are right. I forgot that :(
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v6] of: add generic videomode description
On Sat, Oct 20, 2012 at 01:04:12PM +0200, Thierry Reding wrote:
On Thu, Oct 04, 2012 at 07:59:20PM +0200, Steffen Trumtrar wrote:
[...]
diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
[...]
+#if defined(CONFIG_DRM)
This should be:
#if IS_ENABLED(CONFIG_DRM)
or the code below won't be included if DRM is built as a module. But see
my other replies as to how we can probably handle this better by moving
this into the DRM subsystem.
I already started with moving...now I only need some time to finish with it.
+int videomode_to_display_mode(struct videomode *vm, struct
drm_display_mode *dmode)
+{
+ memset(dmode, 0, sizeof(*dmode));
It appears the usual method to obtain a drm_display_mode to allocate it
using drm_mode_create(), which will allocate it and associate it with
the struct drm_device.
Now, if you do a memset() on the structure you'll overwrite a number of
fields that have previously been initialized and are actually required
to get everything cleaned up properly later on.
So I think we should remove the call to memset().
I was not aware of that. The memset has to go than, of course.
+int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+ int index)
+{
[...]
+}
+EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
This should be:
EXPORT_SYMBOL_GPL(of_get_fb_videomode);
Oops.
Regrads,
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v6] of: add helper to parse display timings
On Sat, Oct 20, 2012 at 09:59:51PM +0200, Thierry Reding wrote:
On Thu, Oct 04, 2012 at 07:59:19PM +0200, Steffen Trumtrar wrote:
[...]
diff --git a/include/linux/of_display_timings.h
b/include/linux/of_display_timings.h
[...]
+struct display_timings {
+ unsigned int num_timings;
+ unsigned int default_timing;
+
+ struct signal_timing **timings;
+};
+
+struct timing_entry {
+ u32 min;
+ u32 typ;
+ u32 max;
+};
+
+struct signal_timing {
I'm slightly confused by the naming here. signal_timing seems overly
generic in this context. Is there any reason why this isn't called
display_timing or even display_mode?
You are right. I actually already changed that, for the same reasons.
It will be called display_timing in the next version, as I think that's what it
really
is.
--
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2 v6] of: add display helper
Hi Leela, On Mon, Oct 15, 2012 at 04:24:43PM +0530, Leela Krishna Amudala wrote: Hello Steffen, To which version of the kernel we can expect this patch set to be merged into? Because I'm waiting for this from long time to add DT support for my display controller :) I have no idea, sorry. It seems like we have almost settled with the binding (clock-name needs to be changed), but I'm not responsible for any merging/inclusions in the kernel. 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- | -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v6] of: add helper to parse display timings
On Thu, Oct 11, 2012 at 09:31:18PM +0200, Thierry Reding wrote:
On Mon, Oct 08, 2012 at 09:49:21AM +0200, Steffen Trumtrar wrote:
On Mon, Oct 08, 2012 at 10:07:45AM +0300, Tomi Valkeinen wrote:
On Thu, 2012-10-04 at 19:59 +0200, Steffen Trumtrar wrote:
[...]
+
+ disp-num_timings = 0;
+
+ for_each_child_of_node(timings_np, entry) {
+ disp-num_timings++;
+ }
No need for { }
Okay.
Or you could just use of_get_child_count().
Thierry
Ah, very nice. That's definitely better. Didn't know about that function.
Thanks,
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v6] of: add generic videomode description
Hi Laurent,
On Mon, Oct 08, 2012 at 10:52:04PM +0200, Laurent Pinchart wrote:
Hi Steffen,
On Monday 08 October 2012 14:48:01 Steffen Trumtrar wrote:
On Mon, Oct 08, 2012 at 02:13:50PM +0200, Laurent Pinchart wrote:
On Thursday 04 October 2012 19:59:20 Steffen Trumtrar wrote:
Get videomode from devicetree in a format appropriate for the
backend. drm_display_mode and fb_videomode are supported atm.
Uses the display signal timings from of_display_timings
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/of/Kconfig |5 +
drivers/of/Makefile |1 +
drivers/of/of_videomode.c| 212 +++
include/linux/of_videomode.h | 41
4 files changed, 259 insertions(+)
create mode 100644 drivers/of/of_videomode.c
create mode 100644 include/linux/of_videomode.h
[snip]
diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
new file mode 100644
index 000..76ac16e
--- /dev/null
+++ b/drivers/of/of_videomode.c
[snip]
+int videomode_from_timing(struct display_timings *disp, struct
videomode *vm,
+ int index)
+{
+ struct signal_timing *st = NULL;
+
+ if (!vm)
+ return -EINVAL;
+
What about making vm a mandatory argument ? It looks to me like a caller
bug if vm is NULL.
The caller must provide the struct videomode, yes. Wouldn't the kernel hang
itself with a NULL pointer exception, if I just work with it ?
The kernel would oops, clearly showing the caller that a non-null vm is
needed
:-)
Okay. No error checking it is then.
+ st = display_timings_get(disp, index);
+
You can remove the blank line.
+ if (!st) {
+ pr_err(%s: no signal timings found\n, __func__);
+ return -EINVAL;
+ }
+
+ vm-pixelclock = signal_timing_get_value(st-pixelclock, 0);
+ vm-hactive = signal_timing_get_value(st-hactive, 0);
+ vm-hfront_porch = signal_timing_get_value(st-hfront_porch,
0);
+ vm-hback_porch = signal_timing_get_value(st-hback_porch, 0);
+ vm-hsync_len = signal_timing_get_value(st-hsync_len, 0);
+
+ vm-vactive = signal_timing_get_value(st-vactive, 0);
+ vm-vfront_porch = signal_timing_get_value(st-vfront_porch,
0);
+ vm-vback_porch = signal_timing_get_value(st-vback_porch, 0);
+ vm-vsync_len = signal_timing_get_value(st-vsync_len, 0);
+
+ vm-vah = st-vsync_pol_active_high;
+ vm-hah = st-hsync_pol_active_high;
+ vm-interlaced = st-interlaced;
+ vm-doublescan = st-doublescan;
+
+ return 0;
+}
+
+int of_get_videomode(struct device_node *np, struct videomode *vm, int
index)
I wonder how to avoid abuse of this functions. It's a useful helper for
drivers that need to get a video mode once only, but would result in lower
performances if a driver calls it for every mode. Drivers must call
of_get_display_timing_list instead in that case and case the display
timings. I'm wondering whether we should really expose of_get_videomode.
The intent was to let the driver decide. That way all the other overhead may
be skipped.
My point is that driver writers might just call of_get_videomode() in a loop,
not knowing that it's expensive. I want to avoid that. We need to at least
add
kerneldoc to the function stating that this shouldn't be done.
You're right. That should be made clear in the code.
+{
+ struct display_timings *disp;
+ int ret = 0;
No need to assign ret to 0 here.
Ah, yes. Unneeded in this case.
+
+ disp = of_get_display_timing_list(np);
+
You can remove the blank line.
+ if (!disp) {
+ pr_err(%s: no timings specified\n, __func__);
+ return -EINVAL;
+ }
+
+ if (index == OF_DEFAULT_TIMING)
+ index = disp-default_timing;
+
+ ret = videomode_from_timing(disp, vm, index);
+
No need for a blank line.
+ if (ret)
+ return ret;
+
+ display_timings_release(disp);
+
+ if (!vm) {
+ pr_err(%s: could not get videomode %d\n, __func__,
index);
+ return -EINVAL;
+ }
This can't happen. If vm is NULL the videomode_from_timing call above will
return -EINVAL, and this function will then return immediately without
reaching this code block.
Okay.
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
--
Regards,
Laurent Pinchart
Regards,
Steffen
--
Pengutronix e.K
Re: [PATCH 1/2 v6] of: add helper to parse display timings
On Sun, Oct 07, 2012 at 03:38:25PM +0200, Laurent Pinchart wrote:
Hi Steffen,
On Friday 05 October 2012 18:38:58 Steffen Trumtrar wrote:
On Fri, Oct 05, 2012 at 10:21:37AM -0600, Stephen Warren wrote:
On 10/05/2012 10:16 AM, Steffen Trumtrar wrote:
On Thu, Oct 04, 2012 at 12:47:16PM -0600, Stephen Warren wrote:
On 10/04/2012 11:59 AM, Steffen Trumtrar wrote:
...
+ for_each_child_of_node(timings_np, entry) {
+ struct signal_timing *st;
+
+ st = of_get_display_timing(entry);
+
+ if (!st)
+ continue;
I wonder if that shouldn't be an error?
In the sense of a pr_err not a -EINVAL I presume?! It is a little bit
quiet in case of a faulty spec, that is right.
I did mean return an error; if we try to parse something and can't,
shouldn't we return an error?
I suppose it may be possible to limp on and use whatever subset of modes
could be parsed and drop the others, which is what this code does, but
the code after the loop would definitely return an error if zero timings
were parseable.
If a display supports multiple modes, I think it is better to have a working
mode (even if it is not the preferred one) than have none at all.
If there is no mode at all, that should be an error, right.
If we fail completely in case of an error, DT writers will notice their bugs.
If we ignore errors silently they won't, and we'll end up with buggy DTs (or,
to be accurate, even more buggy DTs :-)). I'd rather fail completely in the
first implementation and add workarounds later only if we need to.
Okay, that is two against one. And if you say it like this, Stephen and you are
right I guess. Fail completely it is then.
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v6] of: add helper to parse display timings
Hi,
On Mon, Oct 08, 2012 at 10:07:45AM +0300, Tomi Valkeinen wrote:
Hi,
On Thu, 2012-10-04 at 19:59 +0200, Steffen Trumtrar wrote:
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
.../devicetree/bindings/video/display-timings.txt | 222
drivers/of/Kconfig |5 +
drivers/of/Makefile|1 +
drivers/of/of_display_timings.c| 183
include/linux/of_display_timings.h | 85
5 files changed, 496 insertions(+)
create mode 100644
Documentation/devicetree/bindings/video/display-timings.txt
create mode 100644 drivers/of/of_display_timings.c
create mode 100644 include/linux/of_display_timings.h
diff --git a/Documentation/devicetree/bindings/video/display-timings.txt
b/Documentation/devicetree/bindings/video/display-timings.txt
new file mode 100644
index 000..45e39bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timings.txt
@@ -0,0 +1,222 @@
+display-timings bindings
+==
+
+display-timings-node
+
+
+required properties:
+ - none
+
+optional properties:
+ - default-timing: the default timing value
+
+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: displayclock in Hz
+
+optional properties:
+ - hsync-active-high (bool): Hsync pulse is active high
+ - vsync-active-high (bool): Vsync pulse is active high
+ - de-active-high (bool): Data-Enable pulse is active high
+ - pixelclk-inverted (bool): pixelclock is inverted
+ - interlaced (bool)
+ - doublescan (bool)
I think bool should be generally used for things that are on/off, like
interlace. For hsync-active-high others I'd rather have 0/1 values as
others already suggested.
+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 default-timing can be
specified.
+
+The parameters are defined as
+
+struct signal_timing
+===
+
+
+--+-+--+---+
+ | |↑| |
|
+ | ||vback_porch | |
|
+ | |↓| |
|
+
+--###--+---+
+ | #↑# |
|
+ | #|# |
|
+ | hback #|# hfront |
hsync |
+ | porch #| hactive # porch |
len |
+
|#---+---#|-|
+ | #|# |
|
+ | #|vactive # |
|
+ | #|# |
|
+ | #↓# |
|
+
+--###--+---+
+ | |↑| |
|
+ | ||vfront_porch| |
|
+ | |↓| |
|
+
+--+-+--+---+
+ | |↑| |
|
+ | ||vsync_len | |
|
+ | |↓| |
|
+
+--+-+--+---+
+
+
+Example:
+
+ display-timings {
+ default-timing = timing0;
+ timing0: 1920p24 {
+ /* 1920x1080p24 */
I think this is commonly called 1080p24.
Oops. Yes, you are right.
+ clock = 5200;
+ hactive = 1920;
+ vactive = 1080;
+ hfront-porch = 25;
+ hback-porch = 25;
+ hsync-len = 25;
+ vback-porch = 2;
+ vfront
Re: [PATCH 2/2 v6] of: add generic videomode description
On Mon, Oct 08, 2012 at 10:21:53AM +0300, Tomi Valkeinen wrote:
On Thu, 2012-10-04 at 19:59 +0200, Steffen Trumtrar wrote:
Get videomode from devicetree in a format appropriate for the
backend. drm_display_mode and fb_videomode are supported atm.
Uses the display signal timings from of_display_timings
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/of/Kconfig |5 +
drivers/of/Makefile |1 +
drivers/of/of_videomode.c| 212
++
include/linux/of_videomode.h | 41
4 files changed, 259 insertions(+)
create mode 100644 drivers/of/of_videomode.c
create mode 100644 include/linux/of_videomode.h
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 646deb0..74282e2 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -88,4 +88,9 @@ config OF_DISPLAY_TIMINGS
help
helper to parse display timings from the devicetree
+config OF_VIDEOMODE
+ def_bool y
+ help
+ helper to get videomodes from the devicetree
+
endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index c8e9603..09d556f 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
obj-$(CONFIG_OF_MTD) += of_mtd.o
obj-$(CONFIG_OF_DISPLAY_TIMINGS) += of_display_timings.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
new file mode 100644
index 000..76ac16e
--- /dev/null
+++ b/drivers/of/of_videomode.c
@@ -0,0 +1,212 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de,
Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include linux/of.h
+#include linux/fb.h
+#include linux/slab.h
+#include drm/drm_mode.h
+#include linux/of_display_timings.h
+#include linux/of_videomode.h
+
+void dump_fb_videomode(struct fb_videomode *m)
+{
+pr_debug(fb_videomode = %d %d %d %d %d %d %d %d %d %d %d %d %d\n,
+m-refresh, m-xres, m-yres, 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);
+}
+
+void dump_drm_displaymode(struct drm_display_mode *m)
+{
+pr_debug(drm_displaymode = %d %d %d %d %d %d %d %d %d\n,
+m-hdisplay, m-hsync_start, m-hsync_end, m-htotal,
+m-vdisplay, m-vsync_start, m-vsync_end, m-vtotal,
+m-clock);
+}
+
+int videomode_from_timing(struct display_timings *disp, struct videomode
*vm,
+ int index)
+{
+ struct signal_timing *st = NULL;
+
+ if (!vm)
+ return -EINVAL;
+
+ st = display_timings_get(disp, index);
+
+ if (!st) {
+ pr_err(%s: no signal timings found\n, __func__);
+ return -EINVAL;
+ }
+
+ vm-pixelclock = signal_timing_get_value(st-pixelclock, 0);
+ vm-hactive = signal_timing_get_value(st-hactive, 0);
+ vm-hfront_porch = signal_timing_get_value(st-hfront_porch, 0);
+ vm-hback_porch = signal_timing_get_value(st-hback_porch, 0);
+ vm-hsync_len = signal_timing_get_value(st-hsync_len, 0);
+
+ vm-vactive = signal_timing_get_value(st-vactive, 0);
+ vm-vfront_porch = signal_timing_get_value(st-vfront_porch, 0);
+ vm-vback_porch = signal_timing_get_value(st-vback_porch, 0);
+ vm-vsync_len = signal_timing_get_value(st-vsync_len, 0);
+
+ vm-vah = st-vsync_pol_active_high;
+ vm-hah = st-hsync_pol_active_high;
+ vm-interlaced = st-interlaced;
+ vm-doublescan = st-doublescan;
+
+ return 0;
+}
+
+int of_get_videomode(struct device_node *np, struct videomode *vm, int
index)
+{
+ struct display_timings *disp;
+ int ret = 0;
+
+ disp = of_get_display_timing_list(np);
+
+ if (!disp) {
+ pr_err(%s: no timings specified\n, __func__);
+ return -EINVAL;
+ }
+
+ if (index == OF_DEFAULT_TIMING)
+ index = disp-default_timing;
+
+ ret = videomode_from_timing(disp, vm, index);
+
+ if (ret)
+ return ret;
+
+ display_timings_release(disp);
+
+ if (!vm) {
+ pr_err(%s: could not get videomode %d\n, __func__, index);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
+
+#if defined(CONFIG_DRM)
+int videomode_to_display_mode(struct videomode *vm, struct
drm_display_mode *dmode)
+{
+ memset(dmode, 0, sizeof(*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
Re: [PATCH 2/2 v6] of: add generic videomode description
On Mon, Oct 08, 2012 at 02:13:50PM +0200, Laurent Pinchart wrote:
Hi Steffen,
Thanks for the patch.
On Thursday 04 October 2012 19:59:20 Steffen Trumtrar wrote:
Get videomode from devicetree in a format appropriate for the
backend. drm_display_mode and fb_videomode are supported atm.
Uses the display signal timings from of_display_timings
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/of/Kconfig |5 +
drivers/of/Makefile |1 +
drivers/of/of_videomode.c| 212 +++
include/linux/of_videomode.h | 41
4 files changed, 259 insertions(+)
create mode 100644 drivers/of/of_videomode.c
create mode 100644 include/linux/of_videomode.h
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 646deb0..74282e2 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -88,4 +88,9 @@ config OF_DISPLAY_TIMINGS
help
helper to parse display timings from the devicetree
+config OF_VIDEOMODE
+ def_bool y
+ help
+ helper to get videomodes from the devicetree
+
endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index c8e9603..09d556f 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
obj-$(CONFIG_OF_MTD) += of_mtd.o
obj-$(CONFIG_OF_DISPLAY_TIMINGS) += of_display_timings.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
new file mode 100644
index 000..76ac16e
--- /dev/null
+++ b/drivers/of/of_videomode.c
@@ -0,0 +1,212 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de,
Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include linux/of.h
+#include linux/fb.h
+#include linux/slab.h
+#include drm/drm_mode.h
+#include linux/of_display_timings.h
+#include linux/of_videomode.h
+
+void dump_fb_videomode(struct fb_videomode *m)
+{
+pr_debug(fb_videomode = %d %d %d %d %d %d %d %d %d %d %d %d %d\n,
That's going to be pretty difficult to read :-) Would it make sense to group
several attributes logically (for instance using %ux%u for m-xres, m-yres) ?
No problem. That can be changed.
+m-refresh, m-xres, m-yres, 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);
+}
Shouldn't this (and the other non exported functions below) be static ?
Yes.
+void dump_drm_displaymode(struct drm_display_mode *m)
+{
+pr_debug(drm_displaymode = %d %d %d %d %d %d %d %d %d\n,
+m-hdisplay, m-hsync_start, m-hsync_end, m-htotal,
+m-vdisplay, m-vsync_start, m-vsync_end, m-vtotal,
+m-clock);
+}
+
+int videomode_from_timing(struct display_timings *disp, struct videomode
*vm,
+ int index)
+{
+ struct signal_timing *st = NULL;
+
+ if (!vm)
+ return -EINVAL;
+
What about making vm a mandatory argument ? It looks to me like a caller bug
if vm is NULL.
The caller must provide the struct videomode, yes. Wouldn't the kernel hang
itself
with a NULL pointer exception, if I just work with it ?
+ st = display_timings_get(disp, index);
+
You can remove the blank line.
+ if (!st) {
+ pr_err(%s: no signal timings found\n, __func__);
+ return -EINVAL;
+ }
+
+ vm-pixelclock = signal_timing_get_value(st-pixelclock, 0);
+ vm-hactive = signal_timing_get_value(st-hactive, 0);
+ vm-hfront_porch = signal_timing_get_value(st-hfront_porch, 0);
+ vm-hback_porch = signal_timing_get_value(st-hback_porch, 0);
+ vm-hsync_len = signal_timing_get_value(st-hsync_len, 0);
+
+ vm-vactive = signal_timing_get_value(st-vactive, 0);
+ vm-vfront_porch = signal_timing_get_value(st-vfront_porch, 0);
+ vm-vback_porch = signal_timing_get_value(st-vback_porch, 0);
+ vm-vsync_len = signal_timing_get_value(st-vsync_len, 0);
+
+ vm-vah = st-vsync_pol_active_high;
+ vm-hah = st-hsync_pol_active_high;
+ vm-interlaced = st-interlaced;
+ vm-doublescan = st-doublescan;
+
+ return 0;
+}
+
+int of_get_videomode(struct device_node *np, struct videomode *vm, int
index)
I wonder how to avoid abuse of this functions. It's a useful helper for
drivers that need to get a video mode once only, but would result in lower
performances if a driver calls it for every mode. Drivers must call
of_get_display_timing_list instead in that case and case the display timings.
I'm wondering whether we should really expose of_get_videomode.
The intent was to let the driver decide. That way all the other overhead may
Re: [PATCH 2/2 v6] of: add generic videomode description
On Thu, Oct 04, 2012 at 12:51:00PM -0600, Stephen Warren wrote:
On 10/04/2012 11:59 AM, Steffen Trumtrar wrote:
Get videomode from devicetree in a format appropriate for the
backend. drm_display_mode and fb_videomode are supported atm.
Uses the display signal timings from of_display_timings
+++ b/drivers/of/of_videomode.c
+int videomode_from_timing(struct display_timings *disp, struct videomode
*vm,
+ st = display_timings_get(disp, index);
+
+ if (!st) {
It's a little odd to leave a blank line between those two lines.
Hm, well okay. That can be remedied
Only half of the code in this file seems OF-related; the routines to
convert a timing to a videomode or drm display mode seem like they'd be
useful outside device tree, so I wonder if putting them into
of_videomode.c is the correct thing to do. Still, it's probably not a
big deal.
I am not sure, what the appropriate way to do this is. I can split it up
(again).
--
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v6] of: add helper to parse display timings
On Thu, Oct 04, 2012 at 12:47:16PM -0600, Stephen Warren wrote: On 10/04/2012 11:59 AM, Steffen Trumtrar wrote: A patch description would be useful for something like this. Yes. Shame on me. diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt new file mode 100644 ... +Usage in backend + Everything before that point in the file looks fine to me. \o/ Everything after this point in the file seems to be Linux-specific implementation details. Does it really belong in the DT binding documentation, rather than some Linux-specific documentation file? I guess you are right about that. +struct drm_display_mode +=== + + +--+-+--+---+ + | | | | | ↑ + | | | | | | + | | | | | | + +--###--+---+ | I suspect the entire horizontal box above (and the entire vertical box all the way down the left-hand side) should be on the bottom/right instead of top/left. The reason I think this is because all of vsync_start, vsync_end, vdisplay have to be referenced to some known point, which is usually zero or the start of the timing definition, /or/ there would be some value indicating the size of the top marging/porch in order to say where those other values are referenced to. + | # ↑ ↑ ↑# | | | + | # | | |# | | | + | # | | |# | | | + | # | | |# | | | + | # | | |# | | | + | # | | | hdisplay # | | | + | #--++---# | | | + | # | | |# | | | + | # |vsync_start |# | | | + | # | | |# | | | + | # | |vsync_end |# | | | + | # | | |vdisplay# | | | + | # | | |# | | | + | # | | |# | | | + | # | | |# | | | vtotal + | # | | |# | | | + | # | | | hsync_start# | | | + | #--+-+--+--| | | + | # | | |# | | | + | # | | | hsync_end # | | | + | #--+-+--+--| | + | # | | ↓# | | | + +--|#|--+---+ | + | | | | | | | | + | | | | | | | | + | | ↓ | | | | | + +--+-+---+--+---+ | + | | | | | | | + | | | | | | | + | | ↓ | | | ↓ + +--+-+--+---+ + htotal + - diff --git a/drivers/of/of_display_timings.c b/drivers/of/of_display_timings.c +static int parse_property(struct device_node *np, char *name, + struct timing_entry *result) + if (cells == 1) + ret = of_property_read_u32_array(np, name, result-typ, cells); Should that branch not just set result-min/max to typ as well? Presumably it'd prevent
Re: [PATCH 1/2 v6] of: add helper to parse display timings
On Thu, Oct 04, 2012 at 11:35:35PM +0200, Guennadi Liakhovetski wrote: Hi Steffen Sorry for chiming in so late in the game, but I've long been wanting to have a look at this and compare with what we do for V4L2, so, this seems a great opportunity to me:-) On Thu, 4 Oct 2012, Steffen Trumtrar wrote: Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de --- .../devicetree/bindings/video/display-timings.txt | 222 drivers/of/Kconfig |5 + drivers/of/Makefile|1 + drivers/of/of_display_timings.c| 183 include/linux/of_display_timings.h | 85 5 files changed, 496 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt create mode 100644 drivers/of/of_display_timings.c create mode 100644 include/linux/of_display_timings.h diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt new file mode 100644 index 000..45e39bd --- /dev/null +++ b/Documentation/devicetree/bindings/video/display-timings.txt @@ -0,0 +1,222 @@ +display-timings bindings +== + +display-timings-node + + +required properties: + - none + +optional properties: + - default-timing: the default timing value + +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: displayclock in Hz You're going to hate me for this, but eventually we want to actually reference clock objects in our DT bindings. For now, even if you don't want to actually add clock phandles and stuff here, I think, using the standard clock-frequency property would be much better! Well, that shouldn't be a big deal, the clock-frequency property I mean :-) + +optional properties: + - hsync-active-high (bool): Hsync pulse is active high + - vsync-active-high (bool): Vsync pulse is active high For the above two we also considered using bool properties but eventually settled down with integer ones: - hsync-active = 1 for active-high and 0 for active low. This has the added advantage of being able to omit this property in the .dts, which then doesn't mean, that the polarity is active low, but rather, that the hsync line is not used on this hardware. So, maybe it would be good to use the same binding here too? Never really thought about it that way. But the argument sounds convincing. + - de-active-high (bool): Data-Enable pulse is active high + - pixelclk-inverted (bool): pixelclock is inverted We don't (yet) have a de-active property in V4L, don't know whether we'll ever have to distingsuish between what some datasheets call HREF and HSYNC in DT, but maybe similarly to the above an integer would be preferred. As for pixclk, we call the property pclk-sample and it's also an integer. + - interlaced (bool) Is interlaced a property of the hardware, i.e. of the board? Can the same display controller on one board require interlaced data and on another board - progressive? BTW, I'm not very familiar with display interfaces, but for interlaced you probably sometimes use a field signal, whose polarity you also want to specify here? We use a field-even-active integer property for it. I don't really know about that; have to collect some info first. Thanks Guennadi Thank you. 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- | -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v6] of: add helper to parse display timings
On Fri, Oct 05, 2012 at 10:21:37AM -0600, Stephen Warren wrote:
On 10/05/2012 10:16 AM, Steffen Trumtrar wrote:
On Thu, Oct 04, 2012 at 12:47:16PM -0600, Stephen Warren wrote:
On 10/04/2012 11:59 AM, Steffen Trumtrar wrote:
...
+ for_each_child_of_node(timings_np, entry) {
+ struct signal_timing *st;
+
+ st = of_get_display_timing(entry);
+
+ if (!st)
+ continue;
I wonder if that shouldn't be an error?
In the sense of a pr_err not a -EINVAL I presume?! It is a little bit quiet
in
case of a faulty spec, that is right.
I did mean return an error; if we try to parse something and can't,
shouldn't we return an error?
I suppose it may be possible to limp on and use whatever subset of modes
could be parsed and drop the others, which is what this code does, but
the code after the loop would definitely return an error if zero timings
were parseable.
If a display supports multiple modes, I think it is better to have a working
mode (even if it is not the preferred one) than have none at all.
If there is no mode at all, that should be an error, 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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2 v6] of: add display helper
Hi! In accordance with Stepehn Warren, I downsized the binding. Now, just the display-timing is described, as I think, it is way easier to agree on those and have a complete binding. Regards, Steffen Steffen Trumtrar (2): of: add helper to parse display timings of: add generic videomode description .../devicetree/bindings/video/display-timings.txt | 222 drivers/of/Kconfig | 10 + drivers/of/Makefile|2 + drivers/of/of_display_timings.c| 183 drivers/of/of_videomode.c | 212 +++ include/linux/of_display_timings.h | 85 include/linux/of_videomode.h | 41 7 files changed, 755 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt create mode 100644 drivers/of/of_display_timings.c create mode 100644 drivers/of/of_videomode.c create mode 100644 include/linux/of_display_timings.h create mode 100644 include/linux/of_videomode.h -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2 v6] of: add helper to parse display timings
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
.../devicetree/bindings/video/display-timings.txt | 222
drivers/of/Kconfig |5 +
drivers/of/Makefile|1 +
drivers/of/of_display_timings.c| 183
include/linux/of_display_timings.h | 85
5 files changed, 496 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
create mode 100644 drivers/of/of_display_timings.c
create mode 100644 include/linux/of_display_timings.h
diff --git a/Documentation/devicetree/bindings/video/display-timings.txt
b/Documentation/devicetree/bindings/video/display-timings.txt
new file mode 100644
index 000..45e39bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timings.txt
@@ -0,0 +1,222 @@
+display-timings bindings
+==
+
+display-timings-node
+
+
+required properties:
+ - none
+
+optional properties:
+ - default-timing: the default timing value
+
+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: displayclock in Hz
+
+optional properties:
+ - hsync-active-high (bool): Hsync pulse is active high
+ - vsync-active-high (bool): Vsync pulse is active high
+ - de-active-high (bool): Data-Enable pulse is active high
+ - pixelclk-inverted (bool): pixelclock is inverted
+ - interlaced (bool)
+ - doublescan (bool)
+
+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 default-timing can be
specified.
+
+The parameters are defined as
+
+struct signal_timing
+===
+
+ +--+-+--+---+
+ | |↑| | |
+ | ||vback_porch | | |
+ | |↓| | |
+ +--###--+---+
+ | #↑# | |
+ | #|# | |
+ | hback #|# hfront | hsync |
+ | porch #| hactive # porch | len |
+ |#---+---#|-|
+ | #|# | |
+ | #|vactive # | |
+ | #|# | |
+ | #↓# | |
+ +--###--+---+
+ | |↑| | |
+ | ||vfront_porch| | |
+ | |↓| | |
+ +--+-+--+---+
+ | |↑| | |
+ | ||vsync_len | | |
+ | |↓| | |
+ +--+-+--+---+
+
+
+Example:
+
+ display-timings {
+ default-timing = timing0;
+ timing0: 1920p24 {
+ /* 1920x1080p24 */
+ clock = 5200;
+ hactive = 1920;
+ vactive = 1080;
+ hfront-porch = 25;
+ hback-porch = 25;
+ hsync-len = 25;
+ vback-porch = 2;
+ vfront-porch = 2;
+ vsync-len = 2;
+ hsync-active-high;
+ };
+ };
+
+Every property also supports the use of ranges, so the commonly used datasheet
+description with min typ max-tuples can be used.
+
+Example:
+
+ timing1: timing {
+ /* 1920x1080p24 */
+ clock = 14850;
+ hactive = 1920;
+ vactive = 1080;
+ hsync-len = 0 44 60;
+ hfront-porch = 80 88 95;
+ hback-porch = 100 148 160;
+ vfront-porch = 0 4 6;
+ vback-porch = 0
[PATCH 2/2 v6] of: add generic videomode description
Get videomode from devicetree in a format appropriate for the
backend. drm_display_mode and fb_videomode are supported atm.
Uses the display signal timings from of_display_timings
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/of/Kconfig |5 +
drivers/of/Makefile |1 +
drivers/of/of_videomode.c| 212 ++
include/linux/of_videomode.h | 41
4 files changed, 259 insertions(+)
create mode 100644 drivers/of/of_videomode.c
create mode 100644 include/linux/of_videomode.h
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 646deb0..74282e2 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -88,4 +88,9 @@ config OF_DISPLAY_TIMINGS
help
helper to parse display timings from the devicetree
+config OF_VIDEOMODE
+ def_bool y
+ help
+ helper to get videomodes from the devicetree
+
endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index c8e9603..09d556f 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
obj-$(CONFIG_OF_MTD) += of_mtd.o
obj-$(CONFIG_OF_DISPLAY_TIMINGS) += of_display_timings.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
new file mode 100644
index 000..76ac16e
--- /dev/null
+++ b/drivers/of/of_videomode.c
@@ -0,0 +1,212 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include linux/of.h
+#include linux/fb.h
+#include linux/slab.h
+#include drm/drm_mode.h
+#include linux/of_display_timings.h
+#include linux/of_videomode.h
+
+void dump_fb_videomode(struct fb_videomode *m)
+{
+pr_debug(fb_videomode = %d %d %d %d %d %d %d %d %d %d %d %d %d\n,
+m-refresh, m-xres, m-yres, 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);
+}
+
+void dump_drm_displaymode(struct drm_display_mode *m)
+{
+pr_debug(drm_displaymode = %d %d %d %d %d %d %d %d %d\n,
+m-hdisplay, m-hsync_start, m-hsync_end, m-htotal,
+m-vdisplay, m-vsync_start, m-vsync_end, m-vtotal,
+m-clock);
+}
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+ int index)
+{
+ struct signal_timing *st = NULL;
+
+ if (!vm)
+ return -EINVAL;
+
+ st = display_timings_get(disp, index);
+
+ if (!st) {
+ pr_err(%s: no signal timings found\n, __func__);
+ return -EINVAL;
+ }
+
+ vm-pixelclock = signal_timing_get_value(st-pixelclock, 0);
+ vm-hactive = signal_timing_get_value(st-hactive, 0);
+ vm-hfront_porch = signal_timing_get_value(st-hfront_porch, 0);
+ vm-hback_porch = signal_timing_get_value(st-hback_porch, 0);
+ vm-hsync_len = signal_timing_get_value(st-hsync_len, 0);
+
+ vm-vactive = signal_timing_get_value(st-vactive, 0);
+ vm-vfront_porch = signal_timing_get_value(st-vfront_porch, 0);
+ vm-vback_porch = signal_timing_get_value(st-vback_porch, 0);
+ vm-vsync_len = signal_timing_get_value(st-vsync_len, 0);
+
+ vm-vah = st-vsync_pol_active_high;
+ vm-hah = st-hsync_pol_active_high;
+ vm-interlaced = st-interlaced;
+ vm-doublescan = st-doublescan;
+
+ return 0;
+}
+
+int of_get_videomode(struct device_node *np, struct videomode *vm, int index)
+{
+ struct display_timings *disp;
+ int ret = 0;
+
+ disp = of_get_display_timing_list(np);
+
+ if (!disp) {
+ pr_err(%s: no timings specified\n, __func__);
+ return -EINVAL;
+ }
+
+ if (index == OF_DEFAULT_TIMING)
+ index = disp-default_timing;
+
+ ret = videomode_from_timing(disp, vm, index);
+
+ if (ret)
+ return ret;
+
+ display_timings_release(disp);
+
+ if (!vm) {
+ pr_err(%s: could not get videomode %d\n, __func__, index);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
+
+#if defined(CONFIG_DRM)
+int videomode_to_display_mode(struct videomode *vm, struct drm_display_mode
*dmode)
+{
+ memset(dmode, 0, sizeof(*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
Re: [PATCH 1/2] of: add helper to parse display specs
On Mon, Oct 01, 2012 at 10:53:08AM -0600, Stephen Warren wrote:
On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
Parse a display-node with timings and hardware-specs from devictree.
diff --git a/Documentation/devicetree/bindings/video/display
b/Documentation/devicetree/bindings/video/display
new file mode 100644
index 000..722766a
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display
This should be display.txt.
Okay
@@ -0,0 +1,208 @@
+display bindings
+==
+
+display-node
+
I'm not personally convinced about the direction this is going. While I
think it's reasonable to define DT bindings for displays, and DT
bindings for display modes, I'm not sure that it's reasonable to couple
them together into a single binding.
I think creating a well-defined timing binding first will be much
simpler than doing so within the context of a display binding; the
scope/content of a general display binding seems much less well-defined
to me at least, for reasons I mentioned before.
Yes, you are right. I'm in the middle of moving things around a little.
It seems best, to have bindings only for the timings at the moment and
get people to agree on those and use them, instead of all the adhoc solutions
based on of_videomode v2.
Then, the of_get_display_timings and the conversion via videomode to
fb_videomode
etc can be combined with Laurent Pincharts panel proposal.
+required properties:
+ - none
+
+optional properties:
+ - default-timing: the default timing value
+ - width-mm, height-mm: Display dimensions in mm
+ - hsync-active-high (bool): Hsync pulse is active high
+ - vsync-active-high (bool): Vsync pulse is active high
At least those two properties should exist in the display timing instead
(or perhaps as well). There are certainly cases where different similar
display modes are differentiated by hsync/vsync polarity more than
anything else. This is probably more likely with analog display
connectors than digital, but I see no reason why a DT binding for
display timing shouldn't cover both.
Yes. Will do.
+ - de-active-high (bool): Data-Enable pulse is active high
+ - pixelclk-inverted (bool): pixelclock is inverted
+ - pixel-per-clk
pixel-per-clk is probably something that should either be part of the
timing definition, or something computed internally to the display
driver based on rules for the signal type, rather than something
represented in DT.
The above comment assumes this property is intended to represent DVI's
requirement for pixel clock doubling for low-pixel-clock-rate modes. If
it's something to do with e.g. a single-data-rate vs. double-data-rate
property of the underlying physical connection, that's most likely
something that should be defined in a binding specific to e.g. LVDS,
rather than something generic.
+ - link-width: number of channels (e.g. LVDS)
+ - bpp: bits-per-pixel
+
+timings-subnode
+---
+
+required properties:
+subnodes that specify
+ - hactive, vactive: Display resolution
+ - hfront-porch, hback-porch, hsync-len: Horizontal Display timing
parameters
+ in pixels
+ vfront-porch, vback-porch, vsync-len: Vertical display timing
parameters in
+ lines
+ - clock: displayclock in Hz
+
+There are different ways of describing a display and its capabilities. The
devicetree
+representation corresponds to the one commonly found in datasheets for
displays.
+The description of the display and its timing is split in two parts: first
the display
+properties like size in mm and (optionally) multiple subnodes with the
supported timings.
+If a display supports multiple signal timings, the default-timing can be
specified.
+
+Example:
+
+ display@0 {
+ width-mm = 800;
+ height-mm = 480;
+ default-timing = timing0;
+ timings {
+ timing0: timing@0 {
If you're going to use a unit address (@0) to ensure that node names
are unique (which is not mandatory), then each node also needs a reg
property with matching value, and #address-cells/#size-cells in the
parent. Instead, you could name the nodes something unique based on the
mode name to avoid this, e.g. 1080p24 { ... }.
Ah, okay. Wasn't sure that was valid. I prefer to not use unit addresses.
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5] of: add display helper (was: of: add videomode helper)
Hi! After the feedback I got on v4, I thought about the current state of the videomode helper and decided to reengineer it. The most confusion seems to stem from the fact, that a videomode does not have ranges. And that is correct. Therefore the description of the display now uses a list of timings. These timings support ranges, as they do in datasheets (min/typ/max). A device driver may choose to grep a display-description from the devicetree and directly work with that (matching parameters according to their range etc.). Or one can use the former struct videomode to convert from the timings to a videomode (at the moment it just grabs the typical-value from every timing-parameter). This videomode on the other hand, can then be converted to a mode the backend wants (drm_mode_info, fb_videomode,...). As of now, this intermediate step is a bit, well, unnecessary. But it provides a way to have a generic videomode and functions to possibly convert back-and-forth. In the end, this version does the same as of_videomode, but I hope makes the whole thing a little clearer. Thanks to everybody who reviewed the previous versions. Feedback is always welcome. Regards, Steffen Steffen Trumtrar (2): of: add helper to parse display specs video: add generic videomode description Documentation/devicetree/bindings/video/display | 208 ++ drivers/of/Kconfig |5 +++ drivers/of/Makefile |1 + drivers/of/of_display.c | 157 + drivers/video/Makefile |1 + drivers/video/videomode.c | 146 +++ include/linux/display.h | 85 ++ include/linux/of_display.h | 15 include/linux/videomode.h | 38 +++ 9 files changed, 656 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/display create mode 100644 drivers/of/of_display.c create mode 100644 drivers/video/videomode.c create mode 100644 include/linux/display.h create mode 100644 include/linux/of_display.h create mode 100644 include/linux/videomode.h -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] of: add helper to parse display specs
Parse a display-node with timings and hardware-specs from devictree.
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
Documentation/devicetree/bindings/video/display | 208 +++
drivers/of/Kconfig |5 +
drivers/of/Makefile |1 +
drivers/of/of_display.c | 157 +
include/linux/display.h | 85 +
include/linux/of_display.h | 15 ++
6 files changed, 471 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/display
create mode 100644 drivers/of/of_display.c
create mode 100644 include/linux/display.h
create mode 100644 include/linux/of_display.h
diff --git a/Documentation/devicetree/bindings/video/display
b/Documentation/devicetree/bindings/video/display
new file mode 100644
index 000..722766a
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display
@@ -0,0 +1,208 @@
+display bindings
+==
+
+display-node
+
+
+required properties:
+ - none
+
+optional properties:
+ - default-timing: the default timing value
+ - width-mm, height-mm: Display dimensions in mm
+ - hsync-active-high (bool): Hsync pulse is active high
+ - vsync-active-high (bool): Vsync pulse is active high
+ - de-active-high (bool): Data-Enable pulse is active high
+ - pixelclk-inverted (bool): pixelclock is inverted
+ - pixel-per-clk
+ - link-width: number of channels (e.g. LVDS)
+ - bpp: bits-per-pixel
+
+timings-subnode
+---
+
+required properties:
+subnodes that specify
+ - hactive, vactive: Display resolution
+ - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
+ in pixels
+ vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
+ lines
+ - clock: displayclock in Hz
+
+There are different ways of describing a display and its capabilities. The
devicetree
+representation corresponds to the one commonly found in datasheets for
displays.
+The description of the display and its timing is split in two parts: first the
display
+properties like size in mm and (optionally) multiple subnodes with the
supported timings.
+If a display supports multiple signal timings, the default-timing can be
specified.
+
+Example:
+
+ display@0 {
+ width-mm = 800;
+ height-mm = 480;
+ default-timing = timing0;
+ timings {
+ timing0: timing@0 {
+ /* 1920x1080p24 */
+ clock = 5200;
+ hactive = 1920;
+ vactive = 1080;
+ hfront-porch = 25;
+ hback-porch = 25;
+ hsync-len = 25;
+ vback-porch = 2;
+ vfront-porch = 2;
+ vsync-len = 2;
+ hsync-active-high;
+ };
+ };
+ };
+
+Every property also supports the use of ranges, so the commonly used datasheet
+description with min typ max-tuples can be used.
+
+Example:
+
+ timing1: timing@1 {
+ /* 1920x1080p24 */
+ clock = 14850;
+ hactive = 1920;
+ vactive = 1080;
+ hsync-len = 0 44 60;
+ hfront-porch = 80 88 95;
+ hback-porch = 100 148 160;
+ vfront-porch = 0 4 6;
+ vback-porch = 0 36 50;
+ vsync-len = 0 5 6;
+ };
+
+The display-property in a connector-node (e.g. hdmi, ldb,...) is used to
connect
+the display to that driver.
+of_display expects a phandle, that specifies the display-node, in that
property.
+
+Example:
+
+ hdmi@0012 {
+ status = okay;
+ display = acme;
+ };
+
+Usage in backend
+
+
+A backend driver may choose to use the display directly and convert the timing
+ranges to a suitable mode. Or it may just use the conversion of the display
timings
+to the required mode via the generic videomode struct.
+
+ dtb
+|
+| of_get_display
+↓
+ struct display
+|
+| videomode_from_timings
+↓
+ --- struct videomode ---
+ | |
+ videomode_to_displaymode | |
videomode_to_fb_videomode
+ ↓ ↓
+drm_display_mode fb_videomode
+
+
+Conversion to videomode
[PATCH 2/2] video: add generic videomode description
Backend-independent videomode. At the moment this is not very different
from fb_mode or drm_mode.
It is supposed to be a generic description of videomodes and conversion
step to the desired backend video mode representation.
At the moment, only really makes sense when used with of_display
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
drivers/video/Makefile|1 +
drivers/video/videomode.c | 146 +
include/linux/videomode.h | 38
3 files changed, 185 insertions(+)
create mode 100644 drivers/video/videomode.c
create mode 100644 include/linux/videomode.h
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index ee8dafb..6a02fe0 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -170,3 +170,4 @@ obj-$(CONFIG_FB_VIRTUAL) += vfb.o
#video output switch sysfs driver
obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+obj-y += videomode.o
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 000..b118d00
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,146 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include linux/of.h
+#include linux/fb.h
+#include linux/slab.h
+#include drm/drm_mode.h
+#include linux/display.h
+#include linux/videomode.h
+
+struct videomode *videomode_from_display(struct display *disp, int index)
+{
+ struct videomode *vm;
+ struct signal_timing *st;
+
+ vm = kmalloc(sizeof(struct videomode *), GFP_KERNEL);
+
+ if (!vm)
+ return NULL;
+
+ st = display_get_timing(disp, index);
+
+ vm-pixelclock = signal_timing_get_value(st-pixelclock, 0);
+ vm-hactive = signal_timing_get_value(st-hactive, 0);
+ vm-hfront_porch = signal_timing_get_value(st-hfront_porch, 0);
+ vm-hback_porch = signal_timing_get_value(st-hback_porch, 0);
+ vm-hsync_len = signal_timing_get_value(st-hsync_len, 0);
+
+ vm-vactive = signal_timing_get_value(st-vactive, 0);
+ vm-vfront_porch = signal_timing_get_value(st-vfront_porch, 0);
+ vm-vback_porch = signal_timing_get_value(st-vback_porch, 0);
+ vm-vsync_len = signal_timing_get_value(st-vsync_len, 0);
+
+ vm-vah = disp-vsync_pol_active_high;
+ vm-hah = disp-hsync_pol_active_high;
+
+ return vm;
+}
+
+#if defined(CONFIG_DRM)
+int videomode_to_display_mode(struct videomode *vm, struct drm_display_mode
*dmode)
+{
+ memset(dmode, 0, sizeof(*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-vtotal + vm-vback_porch;
+
+ dmode-clock = vm-pixelclock / 1000;
+
+ 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(videomode_to_display_mode);
+#else
+int videomode_to_displaymode(struct videomode *vm, struct drm_display_mode
*dmode)
+{
+ return 0;
+}
+#endif
+
+int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode
*fbmode)
+{
+ memset(fbmode, 0, sizeof(*fbmode));
+
+ 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;
+
+ 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;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_to_fb_videomode);
+
+int of_get_display_mode(struct device_node *np, struct drm_display_mode
*dmode, int index)
+{
+ struct videomode *vm;
+ struct display *disp;
+
+ disp = of_get_display(np);
+
+ if (!disp) {
+ pr_err(%s: no display specified\n, __func__
Re: [PATCH v4] of: Add videomode helper
On Thu, Sep 20, 2012 at 11:29:42PM +0200, Laurent Pinchart wrote:
Hi,
(CC'ing the linux-media mailing list, as video modes are of interest there as
well)
On Wednesday 19 September 2012 12:19:22 Tomi Valkeinen wrote:
On Wed, 2012-09-19 at 10:20 +0200, Steffen Trumtrar wrote:
This patch adds a helper function for parsing videomodes from the
devicetree. The videomode can be either converted to a struct
drm_display_mode or a struct fb_videomode.
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
Hi!
changes since v3:
- print error messages
- free alloced memory
- general cleanup
Regards
Steffen
.../devicetree/bindings/video/displaymode | 74 +
drivers/of/Kconfig |5 +
drivers/of/Makefile|1 +
drivers/of/of_videomode.c | 283 +++
include/linux/of_videomode.h | 56
5 files changed, 419 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/displaymode
create mode 100644 drivers/of/of_videomode.c
create mode 100644 include/linux/of_videomode.h
diff --git a/Documentation/devicetree/bindings/video/displaymode
b/Documentation/devicetree/bindings/video/displaymode new file mode
100644
index 000..990ca52
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/displaymode
@@ -0,0 +1,74 @@
+videomode bindings
+==
+
+Required properties:
+ - hactive, vactive: Display resolution
+ - hfront-porch, hback-porch, hsync-len: Horizontal Display timing
parameters
+ in pixels
+ vfront-porch, vback-porch, vsync-len: Vertical display timing
parameters in
+ lines
+ - clock: displayclock in Hz
+
+Optional properties:
+ - width-mm, height-mm: Display dimensions in mm
+ - hsync-active-high (bool): Hsync pulse is active high
+ - vsync-active-high (bool): Vsync pulse is active high
+ - interlaced (bool): This is an interlaced mode
+ - doublescan (bool): This is a doublescan mode
+
+There are different ways of describing a display mode. The devicetree
representation
+corresponds to the one commonly found in datasheets for displays.
+The description of the display and its mode is split in two parts: first
the display
+properties like size in mm and (optionally) multiple subnodes with the
supported modes.
+
+Example:
+
+ display@0 {
+ width-mm = 800;
+ height-mm = 480;
+ modes {
+ mode0: mode@0 {
+ /* 1920x1080p24 */
+ clock = 5200;
+ hactive = 1920;
+ vactive = 1080;
+ hfront-porch = 25;
+ hback-porch = 25;
+ hsync-len = 25;
+ vback-porch = 2;
+ vfront-porch = 2;
+ vsync-len = 2;
+ hsync-active-high;
+ };
+ };
+ };
+
+Every property also supports the use of ranges, so the commonly used
datasheet +description with min typ max-tuples can be used.
+
+Example:
+
+ mode1: mode@1 {
+ /* 1920x1080p24 */
+ clock = 14850;
+ hactive = 1920;
+ vactive = 1080;
+ hsync-len = 0 44 60;
+ hfront-porch = 80 88 95;
+ hback-porch = 100 148 160;
+ vfront-porch = 0 4 6;
+ vback-porch = 0 36 50;
+ vsync-len = 0 5 6;
+ };
+
+The videomode can be linked to a connector via phandles. The connector
has to
+support the display- and default-mode-property to link to and select a
mode.
+
+Example:
+
+ hdmi@0012 {
+ status = okay;
+ display = benq;
+ default-mode = mode1;
+ };
+
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index dfba3e6..a3acaa3 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -83,4 +83,9 @@ config OF_MTD
depends on MTD
def_bool y
+config OF_VIDEOMODE
+ def_bool y
+ help
+ helper to parse videomodes from the devicetree
+
endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index e027f44..80e6db3 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
obj-$(CONFIG_OF_MTD) += of_mtd.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
new file mode
