* [PATCH v7 2/8] of: add helper to parse display timings
From: Steffen Trumtrar @ 2012-10-31 9:28 UTC (permalink / raw)
To: devicetree-discuss
Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-1-git-send-email-s.trumtrar@pengutronix.de>
Signed-off-by: Steffen Trumtrar <s.trumtrar@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 0000000..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 = <52000000>;
+ hactive = <1920>;
+ vactive = <1080>;
+ hfront-porch = <25>;
+ hback-porch = <25>;
+ hsync-len = <25>;
+ vback-porch = <2>;
+ vfront-porch = <2>;
+ vsync-len = <2>;
+ hsync-active = <1>;
+ };
+ };
+
+Every required property also supports the use of ranges, so the commonly used
+datasheet description with <min typ max>-tuples can be used.
+
+Example:
+
+ timing1: timing {
+ /* 1920x1080p24 */
+ clock = <148500000>;
+ hactive = <1920>;
+ vactive = <1080>;
+ hsync-len = <0 44 60>;
+ hfront-porch = <80 88 95>;
+ hback-porch = <100 148 160>;
+ vfront-porch = <0 4 6>;
+ vback-porch = <0 36 50>;
+ vsync-len = <0 5 6>;
+ };
+
+
+Usage in backend
+========
+
+A backend driver may choose to use the display-timings 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_timing_list
+ ↓
+ struct display_timings
+ |
+ | videomode_from_timing
+ ↓
+ --- struct videomode ---
+ | |
+ videomode_to_displaymode | | videomode_to_fb_videomode
+ ↓ ↓
+ drm_display_mode fb_videomode
+
+The functions of_get_fb_videomode and of_get_display_mode are provided
+to conveniently get the respective mode representation from the devicetree.
+
+Conversion to videomode
+===========+
+As device drivers normally work with some kind of video mode, the timings can be
+converted (may be just a simple copying of the typical value) to a generic videomode
+structure which then can be converted to the according mode used by the backend.
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index dfba3e6..781e773 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -83,4 +83,10 @@ config OF_MTD
depends on MTD
def_bool y
+config OF_DISPLAY_TIMINGS
+ def_bool y
+ depends on DISPLAY_TIMING
+ help
+ helper to parse display timings from the devicetree
+
endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index e027f44..c8e9603 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_DISPLAY_TIMINGS) += of_display_timings.o
diff --git a/drivers/of/of_display_timings.c b/drivers/of/of_display_timings.c
new file mode 100644
index 0000000..388fe4c
--- /dev/null
+++ b/drivers/of/of_display_timings.c
@@ -0,0 +1,185 @@
+/*
+ * OF helpers for parsing display timings
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * based on of_videomode.c by Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/export.h>
+#include <linux/of_display_timings.h>
+
+/**
+ * parse_property - parse timing_entry from device_node
+ * @np: device_node with the property
+ * @name: name of the property
+ * @result: will be set to the return value
+ *
+ * DESCRIPTION:
+ * Every display_timing can be specified with either just the typical value or
+ * a range consisting of min/typ/max. This function helps handling this
+ **/
+static int parse_property(struct device_node *np, char *name,
+ struct timing_entry *result)
+{
+ struct property *prop;
+ int length;
+ 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);
+ result->min = result->typ;
+ result->max = result->typ;
+ } else if (cells = 3) {
+ ret = of_property_read_u32_array(np, name, &result->min, cells);
+ } else {
+ pr_err("%s: illegal timing specification in %s\n", __func__, name);
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+/**
+ * of_get_display_timing - parse display_timing entry from device_node
+ * @np: device_node with the properties
+ **/
+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;
+ }
+
+ return dt;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timing);
+
+/**
+ * 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)
+{
+ 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);
+
+ 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;
+ }
+
+ 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);
+ 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;
+ }
+
+ 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;
+ }
+ return disp;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timing_list);
+
+/**
+ * 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;
+}
+EXPORT_SYMBOL_GPL(of_display_timings_exists);
diff --git a/include/linux/of_display_timings.h b/include/linux/of_display_timings.h
new file mode 100644
index 0000000..e4e1f22
--- /dev/null
+++ b/include/linux/of_display_timings.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * display timings of helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_DISPLAY_TIMINGS_H
+#define __LINUX_OF_DISPLAY_TIMINGS_H
+
+#include <linux/display_timing.h>
+
+#define OF_USE_NATIVE_MODE -1
+
+struct display_timings *of_get_display_timing_list(struct device_node *np);
+struct display_timing *of_get_display_timing(struct device_node *np);
+int of_display_timings_exists(struct device_node *np);
+
+#endif
--
1.7.10.4
^ permalink raw reply related
* [PATCH v7 3/8] of: add generic videomode description
From: Steffen Trumtrar @ 2012-10-31 9:28 UTC (permalink / raw)
To: devicetree-discuss
Cc: Steffen Trumtrar, Philipp Zabel, Rob Herring, linux-fbdev,
dri-devel, Laurent Pinchart, Thierry Reding,
Guennady Liakhovetski, linux-media, Tomi Valkeinen,
Stephen Warren, kernel
In-Reply-To: <1351675689-26814-1-git-send-email-s.trumtrar@pengutronix.de>
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.zabel@pengutronix.de>
Signed-off-by: Steffen Trumtrar <s.trumtrar@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 0000000..91a26fc
--- /dev/null
+++ b/drivers/of/of_videomode.c
@@ -0,0 +1,47 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/of.h>
+#include <linux/of_display_timings.h>
+#include <linux/of_videomode.h>
+#include <linux/export.h>
+
+/**
+ * of_get_videomode - get the videomode #<index> from devicetree
+ * @np - devicenode with the display_timings
+ * @vm - set to return value
+ * @index - index into list of display_timings
+ * DESCRIPTION:
+ * Get a list of all display timings and put the one
+ * specified by index into *vm. This function should only be used, if
+ * only one videomode is to be retrieved. A driver that needs to work
+ * with multiple/all videomodes should work with
+ * of_get_display_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 0000000..01518e6
--- /dev/null
+++ b/include/linux/of_videomode.h
@@ -0,0 +1,15 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * videomode of-helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_VIDEOMODE_H
+#define __LINUX_OF_VIDEOMODE_H
+
+#include <linux/videomode.h>
+
+int of_get_videomode(struct device_node *np, struct videomode *vm, int index);
+#endif /* __LINUX_OF_VIDEOMODE_H */
--
1.7.10.4
^ permalink raw reply related
* [PATCH v7 4/8] video: add videomode helpers
From: Steffen Trumtrar @ 2012-10-31 9:28 UTC (permalink / raw)
To: devicetree-discuss
Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-1-git-send-email-s.trumtrar@pengutronix.de>
Add helper functions to convert from display timings to a generic videomode
structure. This videomode can then be converted to the corresponding subsystem
mode representation (e.g. fb_videomode).
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
drivers/video/Kconfig | 6 ++++++
drivers/video/Makefile | 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 0000000..a9fe010
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,44 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@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 0000000..f932147
--- /dev/null
+++ b/include/linux/videomode.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * generic videomode description
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_VIDEOMODE_H
+#define __LINUX_VIDEOMODE_H
+
+#include <linux/display_timing.h>
+
+struct videomode {
+ u32 pixelclock;
+ u32 refreshrate;
+
+ u32 hactive;
+ u32 hfront_porch;
+ u32 hback_porch;
+ u32 hsync_len;
+
+ u32 vactive;
+ u32 vfront_porch;
+ u32 vback_porch;
+ u32 vsync_len;
+
+ u32 hah;
+ u32 vah;
+ bool interlaced;
+ bool doublescan;
+};
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+ int index);
+#endif
--
1.7.10.4
^ permalink raw reply related
* [PATCH v7 5/8] fbmon: add videomode helpers
From: Steffen Trumtrar @ 2012-10-31 9:28 UTC (permalink / raw)
To: devicetree-discuss
Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-1-git-send-email-s.trumtrar@pengutronix.de>
Add a function to convert from the generic videomode to a fb_videomode.
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
drivers/video/fbmon.c | 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
^ permalink raw reply related
* [PATCH v7 6/8] fbmon: add of_videomode helpers
From: Steffen Trumtrar @ 2012-10-31 9:28 UTC (permalink / raw)
To: devicetree-discuss
Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-1-git-send-email-s.trumtrar@pengutronix.de>
Add helper to get fb_videomode from devicetree.
Signed-off-by: Steffen Trumtrar <s.trumtrar@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
^ permalink raw reply related
* [PATCH v7 7/8] drm_modes: add videomode helpers
From: Steffen Trumtrar @ 2012-10-31 9:28 UTC (permalink / raw)
To: devicetree-discuss
Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-1-git-send-email-s.trumtrar@pengutronix.de>
Add conversion from videomode to drm_display_mode
Signed-off-by: Steffen Trumtrar <s.trumtrar@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
^ permalink raw reply related
* [PATCH v7 8/8] drm_modes: add of_videomode helpers
From: Steffen Trumtrar @ 2012-10-31 9:28 UTC (permalink / raw)
To: devicetree-discuss
Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-1-git-send-email-s.trumtrar@pengutronix.de>
Add helper to get drm_display_mode from devicetree.
Signed-off-by: Steffen Trumtrar <s.trumtrar@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
^ permalink raw reply related
* Re: [RFC 0/5] Generic panel framework
From: Laurent Pinchart @ 2012-10-31 13:13 UTC (permalink / raw)
To: Tomi Valkeinen, Richard Purdie
Cc: linux-fbdev, dri-devel, linux-leds, linux-media, Marcus Lorentzon,
Sumit Semwal, Archit Taneja, Sebastien Guiriec, Inki Dae,
Kyungmin Park, Sascha Hauer, Alexandre Courbot
In-Reply-To: <1348055129.2565.54.camel@deskari>
Hi Tomi,
On Wednesday 19 September 2012 14:45:29 Tomi Valkeinen wrote:
> On Fri, 2012-08-17 at 02:49 +0200, Laurent Pinchart wrote:
> > Hi everybody,
> >
> > While working on DT bindings for the Renesas Mobile SoC display controller
> > (a.k.a. LCDC) I quickly realized that display panel implementation based
> > on board code callbacks would need to be replaced by a driver-based panel
> > framework.
>
> I thought I'd try to approach the common panel framework by creating
> device tree examples that OMAP would need. I only thought about the
> connections and organization, not individual properties, so I have not
> filled any "compatible" or such properties.
>
> What I have below is first DT data for OMAP SoC (more or less what omap4
> has), then display examples of different board setups. The hardware
> examples I have here are all real, so no imaginary use cases =).
>
> We don't need to implement support for all these at once, but I think
> the DT data structure should be right from the start, so I'm posting
> this to get discussion about the format.
Thank you for working on this proposal.
> OMAP SoC
> ====
>
> So here's first the SoC specific display nodes. OMAP has a DSS (display
> subsystem) block, which contains the following elements:
>
> - DISPC (display controller) reads the pixels from memory and outputs
> them using specified video timings. DISPC has three outputs, LCD0, LCD1
> and TV. These are SoC internal outputs, they do not go outside the SoC.
>
> - DPI gets its data from DISPC's LCD0, and outputs MIPI DPI (parallel
> RBG)
>
> - Two independent DSI modules, which get their data from LCD0 or LCD1,
> and output MIPI DSI (a serial two-way video bus)
>
> - HDMI, gets data from DISPC's TV output and outputs HDMI
>
> / {
> ocp {
> dss {
> dispc {
> dss-lcd0: output@0 {
> };
>
> dss-lcd1: output@1 {
> };
>
> dss-tv: output@2 {
> };
> };
>
> dpi: dpi {
> video-source = <&dss-lcd0>;
> };
>
> dsi0: dsi@0 {
> video-source = <&dss-lcd0>;
> };
>
> dsi1: dsi@1 {
> video-source = <&dss-lcd1>;
> };
>
> hdmi: hdmi {
> video-source = <&dss-tv>;
> };
> };
> };
> };
>
> I have defined all the relevant nodes, and video-source property is used
> to point to the source for video data. I also define aliases for the SoC
> outputs so that panels can use them.
>
> One thing to note is that the video sources for some of the blocks, like
> DSI, are not hardcoded in the HW, so dsi0 could get its data from LCD0
> or LCD1.
What about the source that are hardwired in hardware ? Shouldn't those be
hardcoded in the driver instead ?
> However, I don't think they are usually changed during runtime, and the dss
> driver cannot change them independently for sure (meaning that some upper
> layer should tell it how to change the config). Thus I specify sane defaults
> here, but the board dts files can of course override the video sources.
I'm not sure whether default settings like those really belong to the DT. I'm
no expert on that topic though.
> Another thing to note is that we have more outputs from OMAP than we have
> outputs from DISPC. This means that the same video source is used by
> multiple sinks (LCD0 used by DPI and DSI0). DPI and DSI0 cannot be used at
> the same time, obviously.
It might not be really obvious, as I don't see what prevents DPI and DSI0 to
be used at the same time :-) Do they share physical pins ?
> And third thing to note, DISPC node defines outputs explicitly, as it has
> multiple outputs, whereas the external outputs do not as they have only one
> output. Thus the node's output is implicitly the node itself. So, instead of
> having:
>
> ds0: dsi@0 {
> video-source = <&dss-lcd0>;
> dsi0-out0: output@0 {
> };
> };
>
> I have:
>
> dsi0: dsi@0 {
> video-source = <&dss-lcd0>;
> };
What about defining the data sinks instead of the data sources ? I find it
more logical for the DSS to get the panel it's connected to than the other way
around.
> Of this I'm a bit unsure. I believe in most cases there's only one output,
> so it'd be nice to have a shorter representation, but I'm not sure if it's
> good to handle the cases for single and multiple outputs differently. Or if
> it's good to mix control and data busses, as, for example, dsi0 can be used
> as both control and data bus. Having the output defined explicitly would
> separate the control and data bus nodes.
>
>
> Simple DPI panel
> ========
>
> Here a board has a DPI panel, which is controlled via i2c. Panel nodes
> are children of the control bus, so in this case we define the panel
> under i2c2.
>
> &i2c2 {
> dpi-lcd-panel {
> video-source = <&dpi>;
>
> };
> };
>
>
> HDMI
> ==
>
> OMAP has a HDMI output, but it cannot be connected directly to an HDMI
> cable. TI uses tpd12s015 chip in its board, which provides ESD,
> level-shifting and whatnot (I'm an SW guy, google for the chip to read
> the details =). tpd12s015 has a few GPIOs and powers that need to be
> controlled, so we need a driver for it.
>
> There's no control bus for the tpd12s015, so it's platform device. Then
> there's the device for the HDMI monitor, and the DDC lines are connected
> to OMAP's i2c4, thus the hdmi monitor device is a child of i2c.
>
> / {
> hdmi-connector: tpd12s015 {
> video-source = <&hdmi>;
> };
> };
>
> &i2c4 {
> hdmi-monitor {
> video-source = <&hdmi-connector>;
> };
> };
So this implied we would have the following chain ?
DISPC (on SoC) -> HDMI (on SoC) -> TPD12S015 (on board) -> HDMI monitor (off
board)
Should we then have a driver for the HDMI monitor ?
> DSI bridge chip
> =======>
> In this board we have a DSI bridge chip that is controlled via DSI, and
> gets the pixel data via DSI video mode stream. The chip converts this to
> LVDS. We then have an LVDS panel connected to this bridge chip, which is
> controlled via SPI.
>
> &dsi0 {
> dsi2lvds: dsi2lvds {
> video-source = <&dsi0>;
>
> };
> };
>
> &spi2 {
> lvds-lcd-panel {
> video-source = <&dsi2lvds>;
> };
> };
>
>
> High res dual-DSI panel
> ===========>
> Here we have a DSI video mode panel that gets its data from two DSI
> buses. This allows it to get the combined bandwidth of the DSI buses,
> achieving higher resolution than with single DSI bus. The panel is
> controlled via the first DSI bus.
>
> &dsi0 {
> dual-dsi-panel {
> video-source-1 = <&dsi0>;
> video-source-2 = <&dsi1>;
>
> };
> };
>
>
> DSI buffer chip with two outputs
> ================
>
> This board has a DSI command mode buffer chip, that contains its own
> framebuffer. The buffer chip has two DPI outputs, which get the pixels
> from the framebuffer. Similar to OMAP's DISPC this chip has multiple
> outputs so they need to be defined explicitly. And we also have two
> dummy DPI panels connected to this buffer chip.
>
> &dsi0 {
> dsibuf {
> video-source = <&dsi0>;
>
> dsibuf-dpi0: output@0 {
> };
>
> dsibuf-dpi1: output@1 {
> };
> };
> };
>
> / {
> dpi-lcd-panel@0 {
> video-source = <&dsibuf-dpi0>;
>
> };
>
> dpi-lcd-panel@1 {
> video-source = <&dsibuf-dpi1>;
>
> };
> };
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC 0/5] Generic panel framework
From: Laurent Pinchart @ 2012-10-31 13:28 UTC (permalink / raw)
To: Inki Dae
Cc: linux-fbdev, dri-devel, linux-leds, linux-media, Bryan Wu,
Richard Purdie, Tomi Valkeinen, Marcus Lorentzon, Sumit Semwal,
Archit Taneja, Sebastien Guiriec, Kyungmin Park
In-Reply-To: <CAAQKjZOZ9+NSQbNkG3qyWh+oLAE1e44DQ_bQCEr4Wvg2WLiGtA@mail.gmail.com>
Hi Inki,
On Saturday 20 October 2012 22:10:17 Inki Dae wrote:
> Hi Laurent. sorry for being late.
No worries.
> 2012/8/17 Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> > Hi everybody,
> >
> > While working on DT bindings for the Renesas Mobile SoC display controller
> > (a.k.a. LCDC) I quickly realized that display panel implementation based
> > on board code callbacks would need to be replaced by a driver-based panel
> > framework.
> >
> > Several driver-based panel support solution already exist in the kernel.
> >
> > - The LCD device class is implemented in drivers/video/backlight/lcd.c and
> > exposes a kernel API in include/linux/lcd.h. That API is tied to the
> > FBDEV API for historical reason, uses board code callback for reset and
> > power management, and doesn't include support for standard features
> > available in today's "smart panels".
> >
> > - OMAP2+ based systems use custom panel drivers available in
> >
> > drivers/video/omap2/displays. Those drivers are based on OMAP DSS
> > (display controller) specific APIs.
> >
> > - Similarly, Exynos based systems use custom panel drivers available in
> >
> > drivers/video/exynos. Only a single driver (s6e8ax0) is currently
> > available. That driver is based on Exynos display controller specific
> > APIs and on the LCD device class API.
> >
> > I've brought up the issue with Tomi Valkeinen (OMAP DSS maintainer) and
> > Marcus Lorentzon (working on panel support for ST/Linaro), and we agreed
> > that a generic panel framework for display devices is needed. These
> > patches implement a first proof of concept.
> >
> > One of the main reasons for creating a new panel framework instead of
> > adding missing features to the LCD framework is to avoid being tied to
> > the FBDEV framework. Panels will be used by DRM drivers as well, and
> > their API should thus be subsystem-agnostic. Note that the panel
> > framework used the fb_videomode structure in its API, this will be
> > replaced by a common video mode structure shared across subsystems
> > (there's only so many hours per day).
> >
> > Panels, as used in these patches, are defined as physical devices
> > combining a matrix of pixels and a controller capable of driving that
> > matrix.
> >
> > Panel physical devices are registered as children of the control bus the
> > panel controller is connected to (depending on the panel type, we can
> > find platform devices for dummy panels with no control bus, or I2C, SPI,
> > DBI, DSI, ... devices). The generic panel framework matches registered
> > panel devices with panel drivers and call the panel drivers probe method,
> > as done by other device classes in the kernel. The driver probe() method
> > is responsible for instantiating a struct panel instance and registering
> > it with the generic panel framework.
> >
> > Display drivers are panel consumers. They register a panel notifier with
> > the framework, which then calls the notifier when a matching panel is
> > registered. The reason for this asynchronous mode of operation, compared
> > to how drivers acquire regulator or clock resources, is that the panel
> > can use resources provided by the display driver. For instance a panel
> > can be a child of the DBI or DSI bus controlled by the display device, or
> > use a clock provided by that device. We can't defer the display device
> > probe until the panel is registered and also defer the panel device probe
> > until the display is registered. As most display drivers need to handle
> > output devices hotplug (HDMI monitors for instance), handling panel
> > through a notification system seemed to be the easiest solution.
> >
> > Note that this brings a different issue after registration, as display and
> > panel drivers would take a reference to each other. Those circular
> > references would make driver unloading impossible. I haven't found a good
> > solution for that problem yet (hence the RFC state of those patches), and
> > I would appreciate your input here. This might also be a hint that the
> > framework design is wrong to start with. I guess I can't get everything
> > right on the first try ;-)
> >
> > Getting hold of the panel is the most complex part. Once done, display
> > drivers can call abstract operations provided by panel drivers to control
> > the panel operation. These patches implement three of those operations
> > (enable, start transfer and get modes). More operations will be needed,
> > and those three operations will likely get modified during review. Most
> > of the panels on devices I own are dumb panels with no control bus, and
> > are thus not the best candidates to design a framework that needs to take
> > complex panels' needs into account.
> >
> > In addition to the generic panel core, I've implemented MIPI DBI (Display
> > Bus Interface, a parallel bus for panels that supports read/write
> > transfers of commands and data) bus support, as well as three panel
> > drivers (dummy panels with no control bus, and Renesas R61505- and
> > R61517-based panels, both using DBI as their control bus). Only the dummy
> > panel driver has been tested as I lack hardware for the two other
> > drivers.
> >
> > I will appreciate all reviews, comments, criticisms, ideas, remarks, ...
> > If you can find a clever way to solve the cyclic references issue
> > described above I'll buy you a beer at the next conference we will both
> > attend. If you think the proposed solution is too complex, or too simple,
> > I'm all ears. I personally already feel that we might need something even
> > more generic to support other kinds of external devices connected to
> > display controllers, such as external DSI to HDMI converters for instance.
> > Some kind of video entity exposing abstract operations like the panels do
> > would make sense, in which case panels would "inherit" from that video
> > entity.
> >
> > Speaking of conferences, I will attend the KS/LPC in San Diego in a bit
> > more than a week, and would be happy to discuss this topic face to face
> > there.
> >
> > Laurent Pinchart (5):
> > video: Add generic display panel core
> > video: panel: Add dummy panel support
> > video: panel: Add MIPI DBI bus support
> > video: panel: Add R61505 panel support
> > video: panel: Add R61517 panel support
>
> how about using 'buses' directory instead of 'panel' and adding
> 'panel' under that like below?
> video/buess: display bus frameworks such as MIPI-DBI/DSI and eDP are placed.
> video/buess/panel: panel drivers based on display bus-based drivers are
> placed.
>
> I think MIPI-DBI(Display Bus Interface)/DSI(Display Serial Interface)
> and eDP are the bus interfaces for display controllers such as
> DISC(OMAP SoC) and FIMC(Exynos SoC).
After discussing the generic panel framework at Linaro Connect, we came to the
conclusion that "panel" is too limiting a name. I will send an RFC v2 titled
"common display framework", with a drivers/video/display/ directory. I'm
unsure whether panels should go to drivers/video/panels/ or
drivers/video/display/panels/.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* fbdev: on-screen character cell corruption
From: Jan Engelhardt @ 2012-10-31 13:47 UTC (permalink / raw)
To: linux-fbdev
Hi,
I have here a 9x16 font (for use with e.g. videor0x400 on KMS to get
80x25 tty size) that fbcon seems to have a problem correctly rendering.
When backspacing, the pixels from the character under the cursor not
only gets moved to one character cell to the left (as it should), but
the leftmost pixel column of the char also leaks to the rightmost pixel
column previous glyph.
LKML being a text/plain medium, I have taken pictures of the screen for
better understanding on how to reproduce, and have placed them at
http://inai.de/keyb/ . The font file is also provided there.
The kernel I run is 3.4.11 on x86_64. There is no known good version to
me (IOW, it is not a regression), as I just started using a 9x16 font
on fbcon.
^ permalink raw reply
* Re: [RFC 0/5] Generic panel framework
From: Tomi Valkeinen @ 2012-10-31 14:20 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Richard Purdie, linux-fbdev, dri-devel, linux-leds, linux-media,
Marcus Lorentzon, Sumit Semwal, Archit Taneja, Sebastien Guiriec,
Inki Dae, Kyungmin Park, Sascha Hauer, Alexandre Courbot
In-Reply-To: <3150258.gEx8mFWcM9@avalon>
[-- Attachment #1: Type: text/plain, Size: 11000 bytes --]
On 2012-10-31 15:13, Laurent Pinchart wrote:
>> OMAP SoC
>> ========
>>
>> So here's first the SoC specific display nodes. OMAP has a DSS (display
>> subsystem) block, which contains the following elements:
>>
>> - DISPC (display controller) reads the pixels from memory and outputs
>> them using specified video timings. DISPC has three outputs, LCD0, LCD1
>> and TV. These are SoC internal outputs, they do not go outside the SoC.
>>
>> - DPI gets its data from DISPC's LCD0, and outputs MIPI DPI (parallel
>> RBG)
>>
>> - Two independent DSI modules, which get their data from LCD0 or LCD1,
>> and output MIPI DSI (a serial two-way video bus)
>>
>> - HDMI, gets data from DISPC's TV output and outputs HDMI
>>
>> / {
>> ocp {
>> dss {
>> dispc {
>> dss-lcd0: output@0 {
>> };
>>
>> dss-lcd1: output@1 {
>> };
>>
>> dss-tv: output@2 {
>> };
>> };
>>
>> dpi: dpi {
>> video-source = <&dss-lcd0>;
>> };
>>
>> dsi0: dsi@0 {
>> video-source = <&dss-lcd0>;
>> };
>>
>> dsi1: dsi@1 {
>> video-source = <&dss-lcd1>;
>> };
>>
>> hdmi: hdmi {
>> video-source = <&dss-tv>;
>> };
>> };
>> };
>> };
>>
>> I have defined all the relevant nodes, and video-source property is used
>> to point to the source for video data. I also define aliases for the SoC
>> outputs so that panels can use them.
>>
>> One thing to note is that the video sources for some of the blocks, like
>> DSI, are not hardcoded in the HW, so dsi0 could get its data from LCD0
>> or LCD1.
>
> What about the source that are hardwired in hardware ? Shouldn't those be
> hardcoded in the driver instead ?
Even if both the DSI and the DISPC are parts of OMAP DSS, and part of
the SoC, they are separate IPs. We should look at them the same way we'd
consider chips that are outside the SoC.
So things that are internal to a device can (and I think should) be
hardcoded in the driver, but integration details, the connections
between the IPs, etc, should be described in the DT data.
Then again, we do have (and need) a driver for the "dss" node in the
above DT data. This dss represents dss_core, a "glue" IP that contains
the rest of the DSS blocks and muxes and such. It could be argued that
this dss_core driver does indeed know the integration details, and thus
there's no need to represent them in the DT data.
However, I do think that we should represent the DISPC outputs with
generic display entities inside CPF, just like DSI and the panels. And
we do need to set the connections between these entities. So the
question is, do we get those connections from the DT data, or are they
hardcoded in the dss_core driver.
I don't currently have strong opinions on either direction. Both make
sense to me. But I think this is SoC driver implementation specific, and
the common panel framework doesn't need to force this to either
direction. Both should be possible from CPF's point of view.
>> However, I don't think they are usually changed during runtime, and the dss
>> driver cannot change them independently for sure (meaning that some upper
>> layer should tell it how to change the config). Thus I specify sane defaults
>> here, but the board dts files can of course override the video sources.
>
> I'm not sure whether default settings like those really belong to the DT. I'm
> no expert on that topic though.
I agree. But I also don't have a good solution how the driver would find
good choices for these settings...
>> Another thing to note is that we have more outputs from OMAP than we have
>> outputs from DISPC. This means that the same video source is used by
>> multiple sinks (LCD0 used by DPI and DSI0). DPI and DSI0 cannot be used at
>> the same time, obviously.
>
> It might not be really obvious, as I don't see what prevents DPI and DSI0 to
> be used at the same time :-) Do they share physical pins ?
I think they do, but even if they didn't, there's just one source for
two outputs. So if the SoC design is such that the only video source for
DPI is LCD0, and the only video source for DSI0 is LCD0, and presuming
you can't send the video data to both destinations, then only one of DPI
and DSI0 can be enabled at the same time.
Even if the LCD0 could send the pixel stream to both DPI and DSI0, it'd
be "interesting", because the original video timings and pixel clock are
programmed in the LCD0 output, and thus both DPI and DSI0 get the same
timings. If DPI would want to use some other mode, DSI would most likely
go nuts.
So my opinion is that we should only allow 1:1 connections between
sources and sinks. If a component has multiple outputs, and even if
those outputs give the exact same data, it should define multiple sources.
>> And third thing to note, DISPC node defines outputs explicitly, as it has
>> multiple outputs, whereas the external outputs do not as they have only one
>> output. Thus the node's output is implicitly the node itself. So, instead of
>> having:
>>
>> ds0: dsi@0 {
>> video-source = <&dss-lcd0>;
>> dsi0-out0: output@0 {
>> };
>> };
>>
>> I have:
>>
>> dsi0: dsi@0 {
>> video-source = <&dss-lcd0>;
>> };
>
> What about defining the data sinks instead of the data sources ? I find it
> more logical for the DSS to get the panel it's connected to than the other way
> around.
Good question. We look at this from different angles. Is the DSS
connected to the panel, or is the panel connected to the DSS? ;)
I see this the same way than, say, regulators. We have a device and a
driver for it, and the driver wants to use a hw resource. If the
resource is a regulator, the DT data for the device will contain a
reference to the regulator. The driver will then get the regulator, and
use it to operate the device.
Similarly for the display devices. We have a driver for a, say, i2c
device. The DT data for that device will contain a reference to the
source for the video data. The driver for the device will then use this
reference to enable/disable/etc the video stream (i.e. operate the device).
The regulators don't even really know anything about the users of the
regulators. Somebody just "gets" a regulator and enables it. The same
should work for display entities also. There's no need for the video
source to know anything about the sink.
So looking at a chain like:
DISPC -> DPI -> Panel
I see the component on the right side using the component on its left.
And thus it makes sense to me to have references from right to left,
i.e. panel has reference to DPI, etc.
Also, it makes sense with DT data files. For example, for omap4 we'll
have omap4.dtsi, which describes common omap4 details. This file would
contain the omap dss nodes. Then we have omap4-panda.dts, which includes
omap4.dtsi. omap4-panda.dts contains data for the displays on this
board. Thus we have something like:
omap4.dtsi:
dss {
dsi0: dsi@0 {
...
};
};
omap4-panda.dts:
/ {
panel {
video-source = <&dsi0>;
};
};
So it comes out quite nicely. If we had the references the other way
around, omap4-panda.dts would need to have the panel definition, and
also override the dsi0 node from omap4.dtsi, and set the sink to the panel.
I have to say we've had this mind-set with omapdss for a long time, so I
may be a bit blind to other alternative. Do you have any practical
examples how linking the other way around would be better?
>> Of this I'm a bit unsure. I believe in most cases there's only one output,
>> so it'd be nice to have a shorter representation, but I'm not sure if it's
>> good to handle the cases for single and multiple outputs differently. Or if
>> it's good to mix control and data busses, as, for example, dsi0 can be used
>> as both control and data bus. Having the output defined explicitly would
>> separate the control and data bus nodes.
>>
>>
>> Simple DPI panel
>> ================
>>
>> Here a board has a DPI panel, which is controlled via i2c. Panel nodes
>> are children of the control bus, so in this case we define the panel
>> under i2c2.
>>
>> &i2c2 {
>> dpi-lcd-panel {
>> video-source = <&dpi>;
>>
>> };
>> };
>>
>>
>> HDMI
>> ====
>>
>> OMAP has a HDMI output, but it cannot be connected directly to an HDMI
>> cable. TI uses tpd12s015 chip in its board, which provides ESD,
>> level-shifting and whatnot (I'm an SW guy, google for the chip to read
>> the details =). tpd12s015 has a few GPIOs and powers that need to be
>> controlled, so we need a driver for it.
>>
>> There's no control bus for the tpd12s015, so it's platform device. Then
>> there's the device for the HDMI monitor, and the DDC lines are connected
>> to OMAP's i2c4, thus the hdmi monitor device is a child of i2c.
>>
>> / {
>> hdmi-connector: tpd12s015 {
>> video-source = <&hdmi>;
>> };
>> };
>>
>> &i2c4 {
>> hdmi-monitor {
>> video-source = <&hdmi-connector>;
>> };
>> };
>
> So this implied we would have the following chain ?
>
> DISPC (on SoC) -> HDMI (on SoC) -> TPD12S015 (on board) -> HDMI monitor (off
> board)
Yes.
Although to be honest, I'm not sure if the TPD12S015 should be part of
the chain, or somehow separate. It's so simple, kinda pass-through IP,
that it would be nicer to have the HDMI monitor connected to the OMAP
HDMI. But this is omap specific question, not really CPF thing.
> Should we then have a driver for the HDMI monitor ?
Yes, that is my opinion:
- It would be strange if for some video chains we would have panel
devices in the end of the chain, and for some we wouldn't. I guess we'd
need some trickery to make them both work the same way if there's no
panel devices for some cases.
- While in 99% of the cases we can use a common, simple HDMI monitor
driver, there could be HDMI monitors with special features. We could
detect this monitor by reading the EDID and if it's this special case,
use a specific driver for it instead of the common HDMI driver. This is
perhaps not very likely with HDMI, but I could imagine eDP panels with
special features.
So I imagine that we could use hot-plug here. The HDMI monitor device
would not exist until a HDMI cable is connected. The SoC's HDMI driver
(or whatever is before the HDMI monitor in the chain) gets a hotplug
interrupt, and it would then add the device, which would then trigger
probe of the corresponding HDMI monitor driver.
Actually, thinking about this, what I said in the above paragraph
wouldn't work. The SoC's HDMI driver can't know what kind of device to
create, unless we have a HDMI bus and HDMI devices. Which, I think, we
shouldn't have, as HDMI monitors are usually controlled via i2c, and
thus the HDMI monitors should be i2c devices.
So I don't really know how this hotplug would work =). It's just an
idea, not a scenario I have at hand.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* [PATCH] video: exynos_dp: Check DPCD return codes
From: Sean Paul @ 2012-10-31 14:21 UTC (permalink / raw)
To: linux-fbdev
Add return code checks to the DPCD transactions in the SW link training
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/video/exynos/exynos_dp_core.c | 86 +++++++++++++++++++++-----------
1 files changed, 56 insertions(+), 30 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index d55470e..44820f2 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -261,11 +261,10 @@ static void exynos_dp_set_lane_lane_pre_emphasis(struct exynos_dp_device *dp,
}
}
-static void exynos_dp_link_start(struct exynos_dp_device *dp)
+static int exynos_dp_link_start(struct exynos_dp_device *dp)
{
u8 buf[4];
- int lane;
- int lane_count;
+ int lane, lane_count, retval;
lane_count = dp->link_train.lane_count;
@@ -276,8 +275,10 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
dp->link_train.cr_loop[lane] = 0;
/* Set sink to D0 (Sink Not Ready) mode. */
- exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
+ retval = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
DPCD_SET_POWER_STATE_D0);
+ if (retval)
+ return retval;
/* Set link rate and count as you want to establish*/
exynos_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
@@ -286,8 +287,10 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
/* Setup RX configuration */
buf[0] = dp->link_train.link_rate;
buf[1] = dp->link_train.lane_count;
- exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_LINK_BW_SET,
+ retval = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_LINK_BW_SET,
2, buf);
+ if (retval)
+ return retval;
/* Set TX pre-emphasis to minimum */
for (lane = 0; lane < lane_count; lane++)
@@ -306,9 +309,11 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
for (lane = 0; lane < lane_count; lane++)
buf[lane] = DPCD_PRE_EMPHASIS_PATTERN2_LEVEL0 |
DPCD_VOLTAGE_SWING_PATTERN1_LEVEL0;
- exynos_dp_write_bytes_to_dpcd(dp,
+ retval = exynos_dp_write_bytes_to_dpcd(dp,
DPCD_ADDR_TRAINING_LANE0_SET,
lane_count, buf);
+
+ return retval;
}
static unsigned char exynos_dp_get_lane_status(u8 link_status[2], int lane)
@@ -430,8 +435,7 @@ static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp)
static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
{
u8 link_status[2];
- int lane;
- int lane_count;
+ int lane, lane_count, retval;
u8 adjust_request[2];
u8 voltage_swing;
@@ -442,17 +446,22 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
lane_count = dp->link_train.lane_count;
- exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
+ retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
2, link_status);
+ if (retval)
+ return retval;
if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
/* set training pattern 2 for EQ */
exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
for (lane = 0; lane < lane_count; lane++) {
- exynos_dp_read_bytes_from_dpcd(dp,
+ retval = exynos_dp_read_bytes_from_dpcd(dp,
DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
2, adjust_request);
+ if (retval)
+ return retval;
+
voltage_swing = exynos_dp_get_adjust_request_voltage(
adjust_request, lane);
pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
@@ -472,15 +481,19 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
lane);
}
- exynos_dp_write_byte_to_dpcd(dp,
+ retval = exynos_dp_write_byte_to_dpcd(dp,
DPCD_ADDR_TRAINING_PATTERN_SET,
DPCD_SCRAMBLING_DISABLED |
DPCD_TRAINING_PATTERN_2);
+ if (retval)
+ return retval;
- exynos_dp_write_bytes_to_dpcd(dp,
+ retval = exynos_dp_write_bytes_to_dpcd(dp,
DPCD_ADDR_TRAINING_LANE0_SET,
lane_count,
dp->link_train.training_lane);
+ if (retval)
+ return retval;
dev_info(dp->dev, "Link Training Clock Recovery success\n");
dp->link_train.lt_state = EQUALIZER_TRAINING;
@@ -488,9 +501,12 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
for (lane = 0; lane < lane_count; lane++) {
training_lane = exynos_dp_get_lane_link_training(
dp, lane);
- exynos_dp_read_bytes_from_dpcd(dp,
+ retval = exynos_dp_read_bytes_from_dpcd(dp,
DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
2, adjust_request);
+ if (retval)
+ return retval;
+
voltage_swing = exynos_dp_get_adjust_request_voltage(
adjust_request, lane);
pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
@@ -527,13 +543,14 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
dp->link_train.training_lane[lane], lane);
}
- exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET,
- lane_count,
- dp->link_train.training_lane);
+ retval = exynos_dp_write_bytes_to_dpcd(dp,
+ DPCD_ADDR_TRAINING_LANE0_SET, lane_count,
+ dp->link_train.training_lane);
+ if (retval)
+ return retval;
}
- return 0;
+ return retval;
reduce_link_rate:
exynos_dp_reduce_link_rate(dp);
@@ -544,8 +561,7 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
{
u8 link_status[2];
u8 link_align[3];
- int lane;
- int lane_count;
+ int lane, lane_count, retval;
u32 reg;
u8 adjust_request[2];
@@ -557,8 +573,10 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
lane_count = dp->link_train.lane_count;
- exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
+ retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
2, link_status);
+ if (retval)
+ return retval;
if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
link_align[0] = link_status[0];
@@ -569,9 +587,12 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
&link_align[2]);
for (lane = 0; lane < lane_count; lane++) {
- exynos_dp_read_bytes_from_dpcd(dp,
+ retval = exynos_dp_read_bytes_from_dpcd(dp,
DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
2, adjust_request);
+ if (retval)
+ return retval;
+
voltage_swing = exynos_dp_get_adjust_request_voltage(
adjust_request, lane);
pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
@@ -620,10 +641,12 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
dp->link_train.training_lane[lane],
lane);
- exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET,
- lane_count,
- dp->link_train.training_lane);
+ retval = exynos_dp_write_bytes_to_dpcd(dp,
+ DPCD_ADDR_TRAINING_LANE0_SET,
+ lane_count,
+ dp->link_train.training_lane);
+ if (retval)
+ return retval;
}
} else {
goto reduce_link_rate;
@@ -701,16 +724,17 @@ static void exynos_dp_init_training(struct exynos_dp_device *dp,
static int exynos_dp_sw_link_training(struct exynos_dp_device *dp)
{
- int retval = 0;
- int training_finished = 0;
+ int retval = 0, training_finished = 0;
dp->link_train.lt_state = START;
/* Process here */
- while (!training_finished) {
+ while (!retval && !training_finished) {
switch (dp->link_train.lt_state) {
case START:
- exynos_dp_link_start(dp);
+ retval = exynos_dp_link_start(dp);
+ if (retval)
+ dev_err(dp->dev, "LT link start failed!\n");
break;
case CLOCK_RECOVERY:
retval = exynos_dp_process_clock_recovery(dp);
@@ -729,6 +753,8 @@ static int exynos_dp_sw_link_training(struct exynos_dp_device *dp)
return -EREMOTEIO;
}
}
+ if (retval)
+ dev_err(dp->dev, "eDP link training failed (%d)\n", retval);
return retval;
}
--
1.7.7.3
^ permalink raw reply related
* RE: [PATCH v7 5/8] fbmon: add videomode helpers
From: Manjunathappa, Prakash @ 2012-10-31 15:30 UTC (permalink / raw)
To: Steffen Trumtrar, devicetree-discuss@lists.ozlabs.org
Cc: Rob Herring, linux-fbdev@vger.kernel.org,
dri-devel@lists.freedesktop.org, Laurent Pinchart, Thierry Reding,
Guennady Liakhovetski, linux-media@vger.kernel.org,
Valkeinen, Tomi, Stephen Warren, kernel@pengutronix.de
In-Reply-To: <1351675689-26814-6-git-send-email-s.trumtrar@pengutronix.de>
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.trumtrar@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.
Thanks,
Prakash
> + 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-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v7 5/8] fbmon: add videomode helpers
From: Steffen Trumtrar @ 2012-10-31 16:49 UTC (permalink / raw)
To: Manjunathappa, Prakash
Cc: devicetree-discuss@lists.ozlabs.org, Rob Herring,
linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media@vger.kernel.org, Valkeinen, Tomi, Stephen Warren,
kernel@pengutronix.de
In-Reply-To: <A73F36158E33644199EB82C5EC81C7BC3E9E1B39@DBDE01.ent.ti.com>
Hi Prakash!
On Wed, Oct 31, 2012 at 03:30:03PM +0000, 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.trumtrar@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-5555 |
^ permalink raw reply
* [PATCH] video: exynos_dp: Clean up SW link training
From: Sean Paul @ 2012-10-31 16:54 UTC (permalink / raw)
To: linux-fbdev
Clean up some of the SW training code to make it more clear and reduce
duplicate code.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/video/exynos/exynos_dp_core.c | 279 +++++++++++++--------------------
1 files changed, 112 insertions(+), 167 deletions(-)
Thanks for the pointer. There are still places where the code can be either
simplified, or duplication removed.
Below is a rebased patch for your review.
Sean
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 44820f2..b126e8a 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -276,7 +276,7 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
/* Set sink to D0 (Sink Not Ready) mode. */
retval = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
- DPCD_SET_POWER_STATE_D0);
+ DPCD_SET_POWER_STATE_D0);
if (retval)
return retval;
@@ -301,17 +301,18 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
exynos_dp_set_training_pattern(dp, TRAINING_PTN1);
/* Set RX training pattern */
- exynos_dp_write_byte_to_dpcd(dp,
- DPCD_ADDR_TRAINING_PATTERN_SET,
- DPCD_SCRAMBLING_DISABLED |
- DPCD_TRAINING_PATTERN_1);
+ retval = exynos_dp_write_byte_to_dpcd(dp,
+ DPCD_ADDR_TRAINING_PATTERN_SET,
+ DPCD_SCRAMBLING_DISABLED | DPCD_TRAINING_PATTERN_1);
+ if (retval)
+ return retval;
for (lane = 0; lane < lane_count; lane++)
buf[lane] = DPCD_PRE_EMPHASIS_PATTERN2_LEVEL0 |
DPCD_VOLTAGE_SWING_PATTERN1_LEVEL0;
- retval = exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET,
- lane_count, buf);
+
+ retval = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_TRAINING_LANE0_SET,
+ lane_count, buf);
return retval;
}
@@ -337,18 +338,17 @@ static int exynos_dp_clock_recovery_ok(u8 link_status[2], int lane_count)
return 0;
}
-static int exynos_dp_channel_eq_ok(u8 link_align[3], int lane_count)
+static int exynos_dp_channel_eq_ok(u8 link_status[2], u8 link_align,
+ int lane_count)
{
int lane;
- u8 lane_align;
u8 lane_status;
- lane_align = link_align[2];
- if ((lane_align & DPCD_INTERLANE_ALIGN_DONE) = 0)
+ if ((link_align & DPCD_INTERLANE_ALIGN_DONE) = 0)
return -EINVAL;
for (lane = 0; lane < lane_count; lane++) {
- lane_status = exynos_dp_get_lane_status(link_align, lane);
+ lane_status = exynos_dp_get_lane_status(link_status, lane);
lane_status &= DPCD_CHANNEL_EQ_BITS;
if (lane_status != DPCD_CHANNEL_EQ_BITS)
return -EINVAL;
@@ -432,22 +432,47 @@ static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp)
dp->link_train.lt_state = FAILED;
}
+static void exynos_dp_get_adjust_training_lane(struct exynos_dp_device *dp,
+ u8 adjust_request[2])
+{
+ int lane, lane_count;
+ u8 voltage_swing, pre_emphasis, training_lane;
+
+ lane_count = dp->link_train.lane_count;
+ for (lane = 0; lane < lane_count; lane++) {
+ voltage_swing = exynos_dp_get_adjust_request_voltage(
+ adjust_request, lane);
+ pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
+ adjust_request, lane);
+ training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
+ DPCD_PRE_EMPHASIS_SET(pre_emphasis);
+
+ if (voltage_swing = VOLTAGE_LEVEL_3)
+ training_lane |= DPCD_MAX_SWING_REACHED;
+ if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
+ training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
+
+ dp->link_train.training_lane[lane] = training_lane;
+ }
+}
+
static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
{
- u8 link_status[2];
int lane, lane_count, retval;
-
- u8 adjust_request[2];
- u8 voltage_swing;
- u8 pre_emphasis;
- u8 training_lane;
+ u8 voltage_swing, pre_emphasis, training_lane;
+ u8 link_status[2], adjust_request[2];
usleep_range(100, 101);
lane_count = dp->link_train.lane_count;
retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_LANE0_1_STATUS,
- 2, link_status);
+ 2, link_status);
+ if (retval)
+ return retval;
+
+ retval = exynos_dp_read_bytes_from_dpcd(dp,
+ DPCD_ADDR_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
if (retval)
return retval;
@@ -455,43 +480,9 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
/* set training pattern 2 for EQ */
exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
- for (lane = 0; lane < lane_count; lane++) {
- retval = exynos_dp_read_bytes_from_dpcd(dp,
- DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
- 2, adjust_request);
- if (retval)
- return retval;
-
- voltage_swing = exynos_dp_get_adjust_request_voltage(
- adjust_request, lane);
- pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
- adjust_request, lane);
- training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
- DPCD_PRE_EMPHASIS_SET(pre_emphasis);
-
- if (voltage_swing = VOLTAGE_LEVEL_3)
- training_lane |= DPCD_MAX_SWING_REACHED;
- if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
- training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
-
- dp->link_train.training_lane[lane] = training_lane;
-
- exynos_dp_set_lane_link_training(dp,
- dp->link_train.training_lane[lane],
- lane);
- }
-
retval = exynos_dp_write_byte_to_dpcd(dp,
DPCD_ADDR_TRAINING_PATTERN_SET,
- DPCD_SCRAMBLING_DISABLED |
- DPCD_TRAINING_PATTERN_2);
- if (retval)
- return retval;
-
- retval = exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET,
- lane_count,
- dp->link_train.training_lane);
+ DPCD_SCRAMBLING_DISABLED | DPCD_TRAINING_PATTERN_2);
if (retval)
return retval;
@@ -501,73 +492,49 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
for (lane = 0; lane < lane_count; lane++) {
training_lane = exynos_dp_get_lane_link_training(
dp, lane);
- retval = exynos_dp_read_bytes_from_dpcd(dp,
- DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
- 2, adjust_request);
- if (retval)
- return retval;
-
voltage_swing = exynos_dp_get_adjust_request_voltage(
adjust_request, lane);
pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
adjust_request, lane);
- if (voltage_swing = VOLTAGE_LEVEL_3 ||
- pre_emphasis = PRE_EMPHASIS_LEVEL_3) {
- dev_err(dp->dev, "voltage or pre emphasis reached max level\n");
- goto reduce_link_rate;
- }
-
- if ((DPCD_VOLTAGE_SWING_GET(training_lane) =
- voltage_swing) &&
- (DPCD_PRE_EMPHASIS_GET(training_lane) =
- pre_emphasis)) {
+ if (DPCD_VOLTAGE_SWING_GET(training_lane) =
+ voltage_swing &&
+ DPCD_PRE_EMPHASIS_GET(training_lane) =
+ pre_emphasis)
dp->link_train.cr_loop[lane]++;
- if (dp->link_train.cr_loop[lane] = MAX_CR_LOOP) {
- dev_err(dp->dev, "CR Max loop\n");
- goto reduce_link_rate;
- }
- }
- training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
- DPCD_PRE_EMPHASIS_SET(pre_emphasis);
-
- if (voltage_swing = VOLTAGE_LEVEL_3)
- training_lane |= DPCD_MAX_SWING_REACHED;
- if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
- training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
-
- dp->link_train.training_lane[lane] = training_lane;
-
- exynos_dp_set_lane_link_training(dp,
- dp->link_train.training_lane[lane], lane);
+ if (dp->link_train.cr_loop[lane] = MAX_CR_LOOP ||
+ voltage_swing = VOLTAGE_LEVEL_3 ||
+ pre_emphasis = PRE_EMPHASIS_LEVEL_3) {
+ dev_err(dp->dev, "CR Max reached (%d,%d,%d)\n",
+ dp->link_train.cr_loop[lane],
+ voltage_swing, pre_emphasis);
+ exynos_dp_reduce_link_rate(dp);
+ return -EIO;
+ }
}
+ }
+
+ exynos_dp_get_adjust_training_lane(dp, adjust_request);
- retval = exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET, lane_count,
- dp->link_train.training_lane);
+ for (lane = 0; lane < lane_count; lane++) {
+ exynos_dp_set_lane_link_training(dp,
+ dp->link_train.training_lane[lane], lane);
+ retval = exynos_dp_write_byte_to_dpcd(dp,
+ DPCD_ADDR_TRAINING_LANE0_SET + lane,
+ dp->link_train.training_lane[lane]);
if (retval)
return retval;
}
return retval;
-
-reduce_link_rate:
- exynos_dp_reduce_link_rate(dp);
- return -EIO;
}
static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
{
- u8 link_status[2];
- u8 link_align[3];
int lane, lane_count, retval;
u32 reg;
-
- u8 adjust_request[2];
- u8 voltage_swing;
- u8 pre_emphasis;
- u8 training_lane;
+ u8 link_align, link_status[2], adjust_request[2];
usleep_range(400, 401);
@@ -578,85 +545,63 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
if (retval)
return retval;
- if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
- link_align[0] = link_status[0];
- link_align[1] = link_status[1];
-
- exynos_dp_read_byte_from_dpcd(dp,
- DPCD_ADDR_LANE_ALIGN_STATUS_UPDATED,
- &link_align[2]);
-
- for (lane = 0; lane < lane_count; lane++) {
- retval = exynos_dp_read_bytes_from_dpcd(dp,
- DPCD_ADDR_ADJUST_REQUEST_LANE0_1,
- 2, adjust_request);
- if (retval)
- return retval;
+ if (exynos_dp_clock_recovery_ok(link_status, lane_count)) {
+ exynos_dp_reduce_link_rate(dp);
+ return -EIO;
+ }
- voltage_swing = exynos_dp_get_adjust_request_voltage(
- adjust_request, lane);
- pre_emphasis = exynos_dp_get_adjust_request_pre_emphasis(
- adjust_request, lane);
- training_lane = DPCD_VOLTAGE_SWING_SET(voltage_swing) |
- DPCD_PRE_EMPHASIS_SET(pre_emphasis);
+ retval = exynos_dp_read_bytes_from_dpcd(dp,
+ DPCD_ADDR_ADJUST_REQUEST_LANE0_1, 2, adjust_request);
+ if (retval)
+ return retval;
- if (voltage_swing = VOLTAGE_LEVEL_3)
- training_lane |= DPCD_MAX_SWING_REACHED;
- if (pre_emphasis = PRE_EMPHASIS_LEVEL_3)
- training_lane |= DPCD_MAX_PRE_EMPHASIS_REACHED;
+ retval = exynos_dp_read_byte_from_dpcd(dp,
+ DPCD_ADDR_LANE_ALIGN_STATUS_UPDATED, &link_align);
+ if (retval)
+ return retval;
- dp->link_train.training_lane[lane] = training_lane;
- }
+ exynos_dp_get_adjust_training_lane(dp, adjust_request);
- if (exynos_dp_channel_eq_ok(link_align, lane_count) = 0) {
- /* traing pattern Set to Normal */
- exynos_dp_training_pattern_dis(dp);
+ if (!exynos_dp_channel_eq_ok(link_status, link_align, lane_count)) {
+ /* traing pattern Set to Normal */
+ exynos_dp_training_pattern_dis(dp);
- dev_info(dp->dev, "Link Training success!\n");
+ dev_info(dp->dev, "Link Training success!\n");
- exynos_dp_get_link_bandwidth(dp, ®);
- dp->link_train.link_rate = reg;
- dev_dbg(dp->dev, "final bandwidth = %.2x\n",
- dp->link_train.link_rate);
+ exynos_dp_get_link_bandwidth(dp, ®);
+ dp->link_train.link_rate = reg;
+ dev_dbg(dp->dev, "final bandwidth = %.2x\n",
+ dp->link_train.link_rate);
- exynos_dp_get_lane_count(dp, ®);
- dp->link_train.lane_count = reg;
- dev_dbg(dp->dev, "final lane count = %.2x\n",
- dp->link_train.lane_count);
+ exynos_dp_get_lane_count(dp, ®);
+ dp->link_train.lane_count = reg;
+ dev_dbg(dp->dev, "final lane count = %.2x\n",
+ dp->link_train.lane_count);
- /* set enhanced mode if available */
- exynos_dp_set_enhanced_mode(dp);
- dp->link_train.lt_state = FINISHED;
- } else {
- /* not all locked */
- dp->link_train.eq_loop++;
+ /* set enhanced mode if available */
+ exynos_dp_set_enhanced_mode(dp);
+ dp->link_train.lt_state = FINISHED;
- if (dp->link_train.eq_loop > MAX_EQ_LOOP) {
- dev_err(dp->dev, "EQ Max loop\n");
- goto reduce_link_rate;
- }
+ return 0;
+ }
- for (lane = 0; lane < lane_count; lane++)
- exynos_dp_set_lane_link_training(dp,
- dp->link_train.training_lane[lane],
- lane);
+ /* not all locked */
+ dp->link_train.eq_loop++;
- retval = exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET,
- lane_count,
- dp->link_train.training_lane);
- if (retval)
- return retval;
- }
- } else {
- goto reduce_link_rate;
+ if (dp->link_train.eq_loop > MAX_EQ_LOOP) {
+ dev_err(dp->dev, "EQ Max loop\n");
+ exynos_dp_reduce_link_rate(dp);
+ return -EIO;
}
- return 0;
+ for (lane = 0; lane < lane_count; lane++)
+ exynos_dp_set_lane_link_training(dp,
+ dp->link_train.training_lane[lane], lane);
-reduce_link_rate:
- exynos_dp_reduce_link_rate(dp);
- return -EIO;
+ retval = exynos_dp_write_bytes_to_dpcd(dp, DPCD_ADDR_TRAINING_LANE0_SET,
+ lane_count, dp->link_train.training_lane);
+
+ return retval;
}
static void exynos_dp_get_max_rx_bandwidth(struct exynos_dp_device *dp,
--
1.7.7.3
^ permalink raw reply related
* [PATCH] video: exynos_dp: Get pll lock before pattern set
From: Sean Paul @ 2012-10-31 17:01 UTC (permalink / raw)
To: linux-fbdev
According to the exynos datasheet (Figure 49-10), we should wait for PLL
lock before programming the training pattern when doing software eDP
link training.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/video/exynos/exynos_dp_core.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index b126e8a..ef9b003 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -264,7 +264,7 @@ static void exynos_dp_set_lane_lane_pre_emphasis(struct exynos_dp_device *dp,
static int exynos_dp_link_start(struct exynos_dp_device *dp)
{
u8 buf[4];
- int lane, lane_count, retval;
+ int lane, lane_count, pll_tries, retval;
lane_count = dp->link_train.lane_count;
@@ -297,6 +297,18 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
exynos_dp_set_lane_lane_pre_emphasis(dp,
PRE_EMPHASIS_LEVEL_0, lane);
+ /* Wait for PLL lock */
+ pll_tries = 0;
+ while (exynos_dp_get_pll_lock_status(dp) = PLL_UNLOCKED) {
+ if (pll_tries = DP_TIMEOUT_LOOP_COUNT) {
+ dev_err(dp->dev, "Wait for PLL lock timed out\n");
+ return -ETIMEDOUT;
+ }
+
+ pll_tries++;
+ usleep_range(90, 120);
+ }
+
/* Set training pattern 1 */
exynos_dp_set_training_pattern(dp, TRAINING_PTN1);
--
1.7.7.3
^ permalink raw reply related
* Re: [PATCH v7 1/8] video: add display_timing struct and helpers
From: Laurent Pinchart @ 2012-10-31 17:04 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Thierry Reding, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-2-git-send-email-s.trumtrar@pengutronix.de>
Hi Steffen,
Thanks for the patch.
As we'll need a v8 anyway due to the comment on patch 5/8, here are a couple
of other small comments.
On Wednesday 31 October 2012 10:28:01 Steffen Trumtrar wrote:
> Add display_timing structure and the according helper functions. This allows
> the description of a display via its supported timing parameters.
>
> Every timing parameter can be specified as a single value or a range
> <min typ max>.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@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 0000000..9ccfdb3
> --- /dev/null
> +++ b/drivers/video/display_timing.c
> @@ -0,0 +1,24 @@
> +/*
> + * generic display timing functions
> + *
> + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>,
> Pengutronix + *
> + * This file is released under the GPLv2
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/display_timing.h>
I try to keep #include's sorted alphabetically, but I won't push for that.
> +void timings_release(struct display_timings *disp)
> +{
> + int i;
> +
> + for (i = 0; i < disp->num_timings; i++)
disp->num_timings is an unsigned int, i should be an unsigned int as well to
avoid signed vs. unsigned comparisons.
> + 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 0000000..aa02a12
> --- /dev/null
> +++ b/include/linux/display_timing.h
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * description of display timings
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_DISPLAY_TIMINGS_H
> +#define __LINUX_DISPLAY_TIMINGS_H
> +
> +#include <linux/types.h>
> +
> +struct timing_entry {
> + u32 min;
> + u32 typ;
> + u32 max;
> +};
> +
> +struct display_timing {
> + struct timing_entry pixelclock;
> +
> + struct timing_entry hactive;
> + struct timing_entry hfront_porch;
> + struct timing_entry hback_porch;
> + struct timing_entry hsync_len;
> +
> + struct timing_entry vactive;
> + struct timing_entry vfront_porch;
> + struct timing_entry vback_porch;
> + struct timing_entry vsync_len;
> +
> + unsigned int vsync_pol_active;
> + unsigned int hsync_pol_active;
> + unsigned int de_pol_active;
> + unsigned int pixelclk_pol;
> + bool interlaced;
> + bool doublescan;
> +};
> +
> +struct display_timings {
> + unsigned int num_timings;
> + unsigned int native_mode;
> +
> + struct display_timing **timings;
> +};
> +
> +/* placeholder function until ranges are really needed */
> +static inline u32 display_timing_get_value(struct timing_entry *te, int
> index)
What is the index parameter for ?
> +{
> + 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) {
index should be an unsigned int for the same reason as above.
> + dt = disp->timings[index];
> + return dt;
Maybe just
return disp->timings[index];
?
> + } else
> + return NULL;
> +}
> +
> +void timings_release(struct display_timings *disp);
> +void display_timings_release(struct display_timings *disp);
> +
> +#endif
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 05/10] video: exynos_dp: Remove sink control to D0
From: Sean Paul @ 2012-10-31 17:05 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344398064-13563-6-git-send-email-seanpaul@chromium.org>
On Mon, Aug 20, 2012 at 5:19 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:
>>
>> Don't reset the sink power to D0. Removing this for three reasons:
>>
>> 1) It's not required in the SW link training documentation
> OK.
>> 2) The comment is incorrect, D0 is normal operation, not "power down"
> OK. As you mentioned, D0 is not 'power down', 'normal operation'.
> However, if comment is wrong, usually we fix comment, not remove it. :)
>
The datasheet doesn't mention this step at all. Furthermore, the
comment explains how it *should* work, so I think it's the code that
is in error. Do you know why this code is needed?
>> 3) It seems to change things in the link training that causes glitches
>
> Um, it seems that this problem depends on LCD panel.
> Other LCDs that I have tested do not have this kind of problem.
> Please, modify this comment.
>
That was a red herring, it didn't cause the glitches, just made them
less obvious.
Sean
>>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> ---
>> drivers/video/exynos/exynos_dp_core.c | 6 ------
>> 1 files changed, 0 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
>> index 207bd7e..1c998d9 100644
>> --- a/drivers/video/exynos/exynos_dp_core.c
>> +++ b/drivers/video/exynos/exynos_dp_core.c
>> @@ -273,12 +273,6 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
>> for (lane = 0; lane < lane_count; lane++)
>> dp->link_train.cr_loop[lane] = 0;
>>
>> - /* Set sink to D0 (Sink Not Ready) mode. */
>> - ret = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
>> - DPCD_SET_POWER_STATE_D0);
>> - if (ret)
>> - return ret;
>> -
>> /* Set link rate and count as you want to establish*/
>> exynos_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
>> exynos_dp_set_lane_count(dp, dp->link_train.lane_count);
>> --
>> 1.7.7.3
>
^ permalink raw reply
* Re: [PATCH v7 1/8] video: add display_timing struct and helpers
From: Laurent Pinchart @ 2012-10-31 17:09 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Thierry Reding, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-2-git-send-email-s.trumtrar@pengutronix.de>
Hi Steffen,
One more comment.
On Wednesday 31 October 2012 10:28:01 Steffen Trumtrar wrote:
> Add display_timing structure and the according helper functions. This allows
> the description of a display via its supported timing parameters.
>
> Every timing parameter can be specified as a single value or a range
> <min typ max>.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@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 0000000..9ccfdb3
> --- /dev/null
> +++ b/drivers/video/display_timing.c
> @@ -0,0 +1,24 @@
> +/*
> + * generic display timing functions
> + *
> + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>,
> Pengutronix + *
> + * This file is released under the GPLv2
> + */
> +
> +#include <linux/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]);
> +}
This function doesn't seem to be called externally, you can make it static.
> +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 0000000..aa02a12
> --- /dev/null
> +++ b/include/linux/display_timing.h
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * description of display timings
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_DISPLAY_TIMINGS_H
> +#define __LINUX_DISPLAY_TIMINGS_H
> +
> +#include <linux/types.h>
> +
> +struct timing_entry {
> + u32 min;
> + u32 typ;
> + u32 max;
> +};
> +
> +struct display_timing {
> + struct timing_entry pixelclock;
> +
> + struct timing_entry hactive;
> + struct timing_entry hfront_porch;
> + struct timing_entry hback_porch;
> + struct timing_entry hsync_len;
> +
> + struct timing_entry vactive;
> + struct timing_entry vfront_porch;
> + struct timing_entry vback_porch;
> + struct timing_entry vsync_len;
> +
> + unsigned int vsync_pol_active;
> + unsigned int hsync_pol_active;
> + unsigned int de_pol_active;
> + unsigned int pixelclk_pol;
> + bool interlaced;
> + bool doublescan;
> +};
> +
> +struct display_timings {
> + unsigned int num_timings;
> + unsigned int native_mode;
> +
> + struct display_timing **timings;
> +};
> +
> +/* placeholder function until ranges are really needed */
> +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
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH] video: exynos_dp: Improve EDID error handling
From: Sean Paul @ 2012-10-31 17:13 UTC (permalink / raw)
To: linux-fbdev
EDID error handling has 2 problems:
- It doesn't fail as early as it can
- The retry counts for i2c and aux transactions are huge
This patch fails if the initial i2c transaction fails, and reduces the
aux and i2c retry counts down to 3.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/video/exynos/exynos_dp_core.c | 13 ++++++++-----
drivers/video/exynos/exynos_dp_reg.c | 9 ++++-----
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index ef9b003..f761ec2 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -90,9 +90,11 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
*/
/* Read Extension Flag, Number of 128-byte EDID extension blocks */
- exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
+ retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
EDID_EXTENSION_FLAG,
&extend_block);
+ if (retval)
+ return retval;
if (extend_block > 0) {
dev_dbg(dp->dev, "EDID data includes a single extension!\n");
@@ -181,14 +183,15 @@ static int exynos_dp_handle_edid(struct exynos_dp_device *dp)
int retval;
/* Read DPCD DPCD_ADDR_DPCD_REV~RECEIVE_PORT1_CAP_1 */
- exynos_dp_read_bytes_from_dpcd(dp,
- DPCD_ADDR_DPCD_REV,
- 12, buf);
+ retval = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_DPCD_REV, 12,
+ buf);
+ if (retval)
+ return retval;
/* Read EDID */
for (i = 0; i < 3; i++) {
retval = exynos_dp_read_edid(dp);
- if (retval = 0)
+ if (!retval)
break;
}
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 3f5ca8a..9a939aa 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -552,7 +552,7 @@ int exynos_dp_write_bytes_to_dpcd(struct exynos_dp_device *dp,
else
cur_data_count = count - start_offset;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Select DPCD device address */
reg = AUX_ADDR_7_0(reg_addr + start_offset);
writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
@@ -617,7 +617,7 @@ int exynos_dp_read_bytes_from_dpcd(struct exynos_dp_device *dp,
cur_data_count = count - start_offset;
/* AUX CH Request Transaction process */
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Select DPCD device address */
reg = AUX_ADDR_7_0(reg_addr + start_offset);
writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
@@ -700,7 +700,7 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
int i;
int retval;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
@@ -708,7 +708,6 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
/* Select EDID device */
retval = exynos_dp_select_i2c_device(dp, device_addr, reg_addr);
if (retval != 0) {
- dev_err(dp->dev, "Select EDID device fail!\n");
continue;
}
@@ -750,7 +749,7 @@ int exynos_dp_read_bytes_from_i2c(struct exynos_dp_device *dp,
int retval = 0;
for (i = 0; i < count; i += 16) {
- for (j = 0; j < 100; j++) {
+ for (j = 0; j < 3; j++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
--
1.7.7.3
^ permalink raw reply related
* [PATCH] video: exynos_dp: Fix bug when checking dp->irq
From: Sean Paul @ 2012-10-31 17:15 UTC (permalink / raw)
To: linux-fbdev
Fix a bug where we check !dp->irq instead of the correct check for
-ENXIO.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
---
drivers/video/exynos/exynos_dp_core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index f761ec2..39271eb 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -882,7 +882,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
}
dp->irq = platform_get_irq(pdev, 0);
- if (!dp->irq) {
+ if (dp->irq = -ENXIO) {
dev_err(&pdev->dev, "failed to get irq\n");
return -ENODEV;
}
--
1.7.7.3
^ permalink raw reply related
* Re: [PATCH v7 4/8] video: add videomode helpers
From: Laurent Pinchart @ 2012-10-31 17:18 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Thierry Reding, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1351675689-26814-5-git-send-email-s.trumtrar@pengutronix.de>
Hi Steffen,
Thanks for the patch.
On Wednesday 31 October 2012 10:28:04 Steffen Trumtrar wrote:
> Add helper functions to convert from display timings to a generic videomode
> structure. This videomode can then be converted to the corresponding
> subsystem mode representation (e.g. fb_videomode).
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
> drivers/video/Kconfig | 6 ++++++
> drivers/video/Makefile | 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"
Shouldn't this option should be automatically selected through a select
statement in other options that depend on it instead of manually selected ?
Same for the DISPLAY_TIMING option in 1/8.
There's so little code here, do you think it would be a good idea to merge
patches 1/8 and 4/8 and have a single Kconfig option ?
> + 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 0000000..a9fe010
> --- /dev/null
> +++ b/drivers/video/videomode.c
> @@ -0,0 +1,44 @@
> +/*
> + * generic display timing functions
> + *
> + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@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>
As in 1/8, I try to keep #include's sorted alphabetically, but I won't push
for it here either :-)
> +
> +int videomode_from_timing(struct display_timings *disp, struct videomode
> *vm,
> + int index)
unsigned int for index ?
> +{
> + struct display_timing *dt = NULL;
No need to initialize dt to NULL.
> + dt = display_timings_get(disp, index);
> + if (!dt) {
> + pr_err("%s: no signal timings found\n", __func__);
I wonder whether this really deserves a pr_err() here. Would this be a caller
bug, or can there be valid use cases where this function would return an error
?
> + 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 0000000..f932147
> --- /dev/null
> +++ b/include/linux/videomode.h
> @@ -0,0 +1,36 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * generic videomode description
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_VIDEOMODE_H
> +#define __LINUX_VIDEOMODE_H
> +
> +#include <linux/display_timing.h>
> +
> +struct videomode {
> + u32 pixelclock;
> + u32 refreshrate;
> +
> + u32 hactive;
> + u32 hfront_porch;
> + u32 hback_porch;
> + u32 hsync_len;
> +
> + u32 vactive;
> + u32 vfront_porch;
> + u32 vback_porch;
> + u32 vsync_len;
> +
> + u32 hah;
> + u32 vah;
> + bool interlaced;
> + bool doublescan;
> +};
> +
> +int videomode_from_timing(struct display_timings *disp, struct videomode
> *vm,
> + int index);
> +#endif
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 08/10] video: exynos_dp: Move hotplug into a workqueue
From: Sean Paul @ 2012-10-31 17:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344398064-13563-9-git-send-email-seanpaul@chromium.org>
On Tue, Aug 7, 2012 at 11:54 PM, Sean Paul <seanpaul@chromium.org> wrote:
> Move the hotplug related code from probe and resume into a workqueue.
> This allows us to initialize the DP driver (and resume it) when there
> is no monitor connected.
>
Comments?
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Olof Johansson <olofj@chromium.org>
> ---
> drivers/video/exynos/exynos_dp_core.c | 94 +++++++++++++++++----------------
> drivers/video/exynos/exynos_dp_core.h | 1 +
> 2 files changed, 50 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 2882362..68ad494 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -834,6 +834,45 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> +static void exynos_dp_hotplug(struct work_struct *work)
> +{
> + struct exynos_dp_device *dp;
> + int ret;
> +
> + dp = container_of(work, struct exynos_dp_device, hotplug_work);
> +
> + ret = exynos_dp_detect_hpd(dp);
> + if (ret) {
> + dev_err(dp->dev, "unable to detect hpd\n");
> + return;
> + }
> +
> + ret = exynos_dp_handle_edid(dp);
> + if (ret) {
> + dev_err(dp->dev, "unable to handle edid\n");
> + return;
> + }
> +
> + ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> + dp->video_info->link_rate);
> + if (ret) {
> + dev_err(dp->dev, "unable to do link train\n");
> + return;
> + }
> +
> + exynos_dp_enable_scramble(dp, 1);
> + exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> + exynos_dp_enable_enhanced_mode(dp, 1);
> +
> + exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> + exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> +
> + exynos_dp_init_video(dp);
> + ret = exynos_dp_config_video(dp, dp->video_info);
> + if (ret)
> + dev_err(&dp->dev, "unable to config video\n");
> +}
> +
> static int __devinit exynos_dp_probe(struct platform_device *pdev)
> {
> struct resource *res;
> @@ -886,6 +925,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> goto err_clock;
> }
>
> + INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
> +
> ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
> "exynos-dp", dp);
> if (ret) {
> @@ -899,36 +940,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>
> exynos_dp_init_dp(dp);
>
> - ret = exynos_dp_detect_hpd(dp);
> - if (ret) {
> - dev_err(&pdev->dev, "unable to detect hpd\n");
> - goto err_clock;
> - }
> -
> - exynos_dp_handle_edid(dp);
> -
> - ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> - dp->video_info->link_rate);
> - if (ret) {
> - dev_err(&pdev->dev, "unable to do link train\n");
> - goto err_clock;
> - }
> -
> - exynos_dp_enable_scramble(dp, 1);
> - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> - exynos_dp_enable_enhanced_mode(dp, 1);
> -
> - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> -
> - exynos_dp_init_video(dp);
> - ret = exynos_dp_config_video(dp, dp->video_info);
> - if (ret) {
> - dev_err(&pdev->dev, "unable to config video\n");
> - goto err_clock;
> - }
> -
> platform_set_drvdata(pdev, dp);
> + schedule_work(&dp->hotplug_work);
>
> return 0;
>
> @@ -943,6 +956,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
> struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
> struct exynos_dp_device *dp = platform_get_drvdata(pdev);
>
> + if (work_pending(&dp->hotplug_work))
> + flush_work_sync(&dp->hotplug_work);
> +
> if (pdata && pdata->phy_exit)
> pdata->phy_exit();
>
> @@ -959,6 +975,9 @@ static int exynos_dp_suspend(struct device *dev)
> struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
> struct exynos_dp_device *dp = platform_get_drvdata(pdev);
>
> + if (work_pending(&dp->hotplug_work))
> + flush_work_sync(&dp->hotplug_work);
> +
> if (pdata && pdata->phy_exit)
> pdata->phy_exit();
>
> @@ -979,22 +998,7 @@ static int exynos_dp_resume(struct device *dev)
> clk_enable(dp->clock);
>
> exynos_dp_init_dp(dp);
> -
> - exynos_dp_detect_hpd(dp);
> - exynos_dp_handle_edid(dp);
> -
> - exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> - dp->video_info->link_rate);
> -
> - exynos_dp_enable_scramble(dp, 1);
> - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> - exynos_dp_enable_enhanced_mode(dp, 1);
> -
> - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> -
> - exynos_dp_init_video(dp);
> - exynos_dp_config_video(dp, dp->video_info);
> + schedule_work(&dp->hotplug_work);
>
> return 0;
> }
> diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
> index 6431c65..cf1010b 100644
> --- a/drivers/video/exynos/exynos_dp_core.h
> +++ b/drivers/video/exynos/exynos_dp_core.h
> @@ -32,6 +32,7 @@ struct exynos_dp_device {
>
> struct video_info *video_info;
> struct link_train link_train;
> + struct work_struct hotplug_work;
> };
>
> /* exynos_dp_reg.c */
> --
> 1.7.7.3
>
^ permalink raw reply
* Re: [PATCH 08/10] video: exynos_dp: Move hotplug into a workqueue
From: Jingoo Han @ 2012-11-01 0:36 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344398064-13563-9-git-send-email-seanpaul@chromium.org>
On Thursday, November 01, 2012 2:20 AM Sean Paul wrote
>
> On Tue, Aug 7, 2012 at 11:54 PM, Sean Paul <seanpaul@chromium.org> wrote:
> > Move the hotplug related code from probe and resume into a workqueue.
> > This allows us to initialize the DP driver (and resume it) when there
> > is no monitor connected.
> >
>
> Comments?
It looks good, but, I have to test this.
>
>
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > Reviewed-by: Olof Johansson <olofj@chromium.org>
> > ---
> > drivers/video/exynos/exynos_dp_core.c | 94 +++++++++++++++++----------------
> > drivers/video/exynos/exynos_dp_core.h | 1 +
> > 2 files changed, 50 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> > index 2882362..68ad494 100644
> > --- a/drivers/video/exynos/exynos_dp_core.c
> > +++ b/drivers/video/exynos/exynos_dp_core.c
> > @@ -834,6 +834,45 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
> > return IRQ_HANDLED;
> > }
> >
> > +static void exynos_dp_hotplug(struct work_struct *work)
> > +{
> > + struct exynos_dp_device *dp;
> > + int ret;
> > +
> > + dp = container_of(work, struct exynos_dp_device, hotplug_work);
> > +
> > + ret = exynos_dp_detect_hpd(dp);
> > + if (ret) {
> > + dev_err(dp->dev, "unable to detect hpd\n");
> > + return;
> > + }
> > +
> > + ret = exynos_dp_handle_edid(dp);
> > + if (ret) {
> > + dev_err(dp->dev, "unable to handle edid\n");
> > + return;
> > + }
> > +
> > + ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> > + dp->video_info->link_rate);
> > + if (ret) {
> > + dev_err(dp->dev, "unable to do link train\n");
> > + return;
> > + }
> > +
> > + exynos_dp_enable_scramble(dp, 1);
> > + exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> > + exynos_dp_enable_enhanced_mode(dp, 1);
> > +
> > + exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> > + exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> > +
> > + exynos_dp_init_video(dp);
> > + ret = exynos_dp_config_video(dp, dp->video_info);
> > + if (ret)
> > + dev_err(&dp->dev, "unable to config video\n");
> > +}
> > +
> > static int __devinit exynos_dp_probe(struct platform_device *pdev)
> > {
> > struct resource *res;
> > @@ -886,6 +925,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> > goto err_clock;
> > }
> >
> > + INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
> > +
> > ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
> > "exynos-dp", dp);
> > if (ret) {
> > @@ -899,36 +940,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >
> > exynos_dp_init_dp(dp);
> >
> > - ret = exynos_dp_detect_hpd(dp);
> > - if (ret) {
> > - dev_err(&pdev->dev, "unable to detect hpd\n");
> > - goto err_clock;
> > - }
> > -
> > - exynos_dp_handle_edid(dp);
> > -
> > - ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> > - dp->video_info->link_rate);
> > - if (ret) {
> > - dev_err(&pdev->dev, "unable to do link train\n");
> > - goto err_clock;
> > - }
> > -
> > - exynos_dp_enable_scramble(dp, 1);
> > - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> > - exynos_dp_enable_enhanced_mode(dp, 1);
> > -
> > - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> > - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> > -
> > - exynos_dp_init_video(dp);
> > - ret = exynos_dp_config_video(dp, dp->video_info);
> > - if (ret) {
> > - dev_err(&pdev->dev, "unable to config video\n");
> > - goto err_clock;
> > - }
> > -
> > platform_set_drvdata(pdev, dp);
> > + schedule_work(&dp->hotplug_work);
> >
> > return 0;
> >
> > @@ -943,6 +956,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
> > struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
> > struct exynos_dp_device *dp = platform_get_drvdata(pdev);
> >
> > + if (work_pending(&dp->hotplug_work))
> > + flush_work_sync(&dp->hotplug_work);
> > +
> > if (pdata && pdata->phy_exit)
> > pdata->phy_exit();
> >
> > @@ -959,6 +975,9 @@ static int exynos_dp_suspend(struct device *dev)
> > struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
> > struct exynos_dp_device *dp = platform_get_drvdata(pdev);
> >
> > + if (work_pending(&dp->hotplug_work))
> > + flush_work_sync(&dp->hotplug_work);
> > +
> > if (pdata && pdata->phy_exit)
> > pdata->phy_exit();
> >
> > @@ -979,22 +998,7 @@ static int exynos_dp_resume(struct device *dev)
> > clk_enable(dp->clock);
> >
> > exynos_dp_init_dp(dp);
> > -
> > - exynos_dp_detect_hpd(dp);
> > - exynos_dp_handle_edid(dp);
> > -
> > - exynos_dp_set_link_train(dp, dp->video_info->lane_count,
> > - dp->video_info->link_rate);
> > -
> > - exynos_dp_enable_scramble(dp, 1);
> > - exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
> > - exynos_dp_enable_enhanced_mode(dp, 1);
> > -
> > - exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
> > - exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
> > -
> > - exynos_dp_init_video(dp);
> > - exynos_dp_config_video(dp, dp->video_info);
> > + schedule_work(&dp->hotplug_work);
> >
> > return 0;
> > }
> > diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
> > index 6431c65..cf1010b 100644
> > --- a/drivers/video/exynos/exynos_dp_core.h
> > +++ b/drivers/video/exynos/exynos_dp_core.h
> > @@ -32,6 +32,7 @@ struct exynos_dp_device {
> >
> > struct video_info *video_info;
> > struct link_train link_train;
> > + struct work_struct hotplug_work;
> > };
> >
> > /* exynos_dp_reg.c */
> > --
> > 1.7.7.3
> >
^ permalink raw reply
* RE: [PATCH 05/10] video: exynos_dp: Remove sink control to D0
From: Jingoo Han @ 2012-11-01 0:45 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344398064-13563-6-git-send-email-seanpaul@chromium.org>
On Thursday, November 01, 2012 2:05 AM Sean Paul wrote
>
> On Mon, Aug 20, 2012 at 5:19 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Wednesday, August 08, 2012 12:54 PM Sean Paul wrote:
> >>
> >> Don't reset the sink power to D0. Removing this for three reasons:
> >>
> >> 1) It's not required in the SW link training documentation
> > OK.
> >> 2) The comment is incorrect, D0 is normal operation, not "power down"
> > OK. As you mentioned, D0 is not 'power down', 'normal operation'.
> > However, if comment is wrong, usually we fix comment, not remove it. :)
> >
>
> The datasheet doesn't mention this step at all. Furthermore, the
> comment explains how it *should* work, so I think it's the code that
> is in error. Do you know why this code is needed?
This is not requirement of the datasheet.
Also, D0 is not necessary for Link training.
Um, this is work-around for initial development.
Newly developed panel did not work, due to pane problem,
so this work-around was added.
However, the panel problem was fixed, so this code is
not necessary.
Please replace comment as below:
According to DP spec, it is not required in the Link Training procedure.
Best regards,
Jingoo Han
>
> >> 3) It seems to change things in the link training that causes glitches
> >
> > Um, it seems that this problem depends on LCD panel.
> > Other LCDs that I have tested do not have this kind of problem.
> > Please, modify this comment.
> >
>
> That was a red herring, it didn't cause the glitches, just made them
> less obvious.
>
> Sean
>
> >>
> >> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> >> ---
> >> drivers/video/exynos/exynos_dp_core.c | 6 ------
> >> 1 files changed, 0 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> >> index 207bd7e..1c998d9 100644
> >> --- a/drivers/video/exynos/exynos_dp_core.c
> >> +++ b/drivers/video/exynos/exynos_dp_core.c
> >> @@ -273,12 +273,6 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp)
> >> for (lane = 0; lane < lane_count; lane++)
> >> dp->link_train.cr_loop[lane] = 0;
> >>
> >> - /* Set sink to D0 (Sink Not Ready) mode. */
> >> - ret = exynos_dp_write_byte_to_dpcd(dp, DPCD_ADDR_SINK_POWER_STATE,
> >> - DPCD_SET_POWER_STATE_D0);
> >> - if (ret)
> >> - return ret;
> >> -
> >> /* Set link rate and count as you want to establish*/
> >> exynos_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
> >> exynos_dp_set_lane_count(dp, dp->link_train.lane_count);
> >> --
> >> 1.7.7.3
> >
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox