linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 1/2 v6] of: add helper to parse display timings
From: Robert Schwebel @ 2012-10-05  7:17 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Steffen Trumtrar, devicetree-discuss, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, linux-media, Tomi Valkeinen,
	Philipp Zabel
In-Reply-To: <Pine.LNX.4.64.1210042307300.3744@axis700.grange>

On Thu, Oct 04, 2012 at 11:35:35PM +0200, Guennadi Liakhovetski wrote:
> > +optional properties:
> > + - hsync-active-high (bool): Hsync pulse is active high
> > + - vsync-active-high (bool): Vsync pulse is active high
>
> For the above two we also considered using bool properties but eventually
> settled down with integer ones:
>
> - hsync-active = <1>
>
> for active-high and 0 for active low. This has the added advantage of
> being able to omit this property in the .dts, which then doesn't mean,
> that the polarity is active low, but rather, that the hsync line is not
> used on this hardware. So, maybe it would be good to use the same binding
> here too?

Philipp, this is the same argumentation as we discussed yesterday for
the dual-link LVDS option, so that one could be modelled in a similar
way.

rsc
-- 
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 04/16] ARM: OMAP: Make plat/fpga.h local to arch/arm/plat-omap
From: Tony Lindgren @ 2012-10-04 22:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121004213950.26676.21898.stgit@muffinssi.local>

There's no need to have this file in plat/fpga.h. We can
make it local to plat-omap replacing fpga_read/write
functions directly with readb/writeb as that's how
they are already defined in fpga.h.

Note that 2420 based H4 is also using the fpga, so let's
keep the led support around in plat-omap until we flip
over mach-omap2 to device tree.

Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: linux-fbdev@vger.kernel.org
Cc: Felipe Balbi <balbi@ti.com>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap1/board-fsample.c         |    6 -
 arch/arm/mach-omap1/board-innovator.c       |   26 ++--
 arch/arm/mach-omap1/board-perseus2.c        |    6 -
 arch/arm/mach-omap1/common.h                |    1 
 arch/arm/mach-omap1/fpga.c                  |    2 
 arch/arm/mach-omap1/include/mach/omap1510.h |  113 ++++++++++++++++
 arch/arm/mach-omap1/serial.c                |    1 
 arch/arm/plat-omap/debug-leds.c             |    2 
 arch/arm/plat-omap/fpga.h                   |   74 ++++++++++
 arch/arm/plat-omap/include/plat/fpga.h      |  193 ---------------------------
 drivers/usb/host/ohci-omap.c                |    5 -
 drivers/video/omap/lcd_inn1510.c            |    7 +
 12 files changed, 216 insertions(+), 220 deletions(-)
 create mode 100644 arch/arm/plat-omap/fpga.h
 delete mode 100644 arch/arm/plat-omap/include/plat/fpga.h

diff --git a/arch/arm/mach-omap1/board-fsample.c b/arch/arm/mach-omap1/board-fsample.c
index 4b6de70..6f496df 100644
--- a/arch/arm/mach-omap1/board-fsample.c
+++ b/arch/arm/mach-omap1/board-fsample.c
@@ -30,7 +30,7 @@
 #include <plat/tc.h>
 #include <mach/mux.h>
 #include <mach/flash.h>
-#include <plat/fpga.h>
+#include <../plat-omap/fpga.h>
 #include <linux/platform_data/keypad-omap.h>
 
 #include <mach/hardware.h>
@@ -123,9 +123,9 @@ static struct resource smc91x_resources[] = {
 
 static void __init fsample_init_smc91x(void)
 {
-	fpga_write(1, H2P2_DBG_FPGA_LAN_RESET);
+	__raw_writeb(1, H2P2_DBG_FPGA_LAN_RESET);
 	mdelay(50);
-	fpga_write(fpga_read(H2P2_DBG_FPGA_LAN_RESET) & ~1,
+	__raw_writeb(__raw_readb(H2P2_DBG_FPGA_LAN_RESET) & ~1,
 		   H2P2_DBG_FPGA_LAN_RESET);
 	mdelay(50);
 }
diff --git a/arch/arm/mach-omap1/board-innovator.c b/arch/arm/mach-omap1/board-innovator.c
index db5f7d2..5804da7 100644
--- a/arch/arm/mach-omap1/board-innovator.c
+++ b/arch/arm/mach-omap1/board-innovator.c
@@ -33,7 +33,7 @@
 
 #include <mach/mux.h>
 #include <mach/flash.h>
-#include <plat/fpga.h>
+#include <../plat-omap/fpga.h>
 #include <plat/tc.h>
 #include <linux/platform_data/keypad-omap.h>
 #include <plat/mmc.h>
@@ -215,7 +215,7 @@ static struct platform_device *innovator1510_devices[] __initdata = {
 
 static int innovator_get_pendown_state(void)
 {
-	return !(fpga_read(OMAP1510_FPGA_TOUCHSCREEN) & (1 << 5));
+	return !(__raw_readb(OMAP1510_FPGA_TOUCHSCREEN) & (1 << 5));
 }
 
 static const struct ads7846_platform_data innovator1510_ts_info = {
@@ -279,7 +279,7 @@ static struct platform_device *innovator1610_devices[] __initdata = {
 static void __init innovator_init_smc91x(void)
 {
 	if (cpu_is_omap1510()) {
-		fpga_write(fpga_read(OMAP1510_FPGA_RST) & ~1,
+		__raw_writeb(__raw_readb(OMAP1510_FPGA_RST) & ~1,
 			   OMAP1510_FPGA_RST);
 		udelay(750);
 	} else {
@@ -335,10 +335,10 @@ static int mmc_set_power(struct device *dev, int slot, int power_on,
 				int vdd)
 {
 	if (power_on)
-		fpga_write(fpga_read(OMAP1510_FPGA_POWER) | (1 << 3),
+		__raw_writeb(__raw_readb(OMAP1510_FPGA_POWER) | (1 << 3),
 				OMAP1510_FPGA_POWER);
 	else
-		fpga_write(fpga_read(OMAP1510_FPGA_POWER) & ~(1 << 3),
+		__raw_writeb(__raw_readb(OMAP1510_FPGA_POWER) & ~(1 << 3),
 				OMAP1510_FPGA_POWER);
 
 	return 0;
@@ -390,14 +390,14 @@ static void __init innovator_init(void)
 		omap_cfg_reg(UART3_TX);
 		omap_cfg_reg(UART3_RX);
 
-		reg = fpga_read(OMAP1510_FPGA_POWER);
+		reg = __raw_readb(OMAP1510_FPGA_POWER);
 		reg |= OMAP1510_FPGA_PCR_COM1_EN;
-		fpga_write(reg, OMAP1510_FPGA_POWER);
+		__raw_writeb(reg, OMAP1510_FPGA_POWER);
 		udelay(10);
 
-		reg = fpga_read(OMAP1510_FPGA_POWER);
+		reg = __raw_readb(OMAP1510_FPGA_POWER);
 		reg |= OMAP1510_FPGA_PCR_COM2_EN;
-		fpga_write(reg, OMAP1510_FPGA_POWER);
+		__raw_writeb(reg, OMAP1510_FPGA_POWER);
 		udelay(10);
 
 		platform_add_devices(innovator1510_devices, ARRAY_SIZE(innovator1510_devices));
@@ -437,6 +437,7 @@ static void __init innovator_init(void)
  */
 static void __init innovator_map_io(void)
 {
+#ifdef CONFIG_ARCH_OMAP15XX
 	omap15xx_map_io();
 
 	iotable_init(innovator1510_io_desc, ARRAY_SIZE(innovator1510_io_desc));
@@ -444,9 +445,10 @@ static void __init innovator_map_io(void)
 
 	/* Dump the Innovator FPGA rev early - useful info for support. */
 	pr_debug("Innovator FPGA Rev %d.%d Board Rev %d\n",
-			fpga_read(OMAP1510_FPGA_REV_HIGH),
-			fpga_read(OMAP1510_FPGA_REV_LOW),
-			fpga_read(OMAP1510_FPGA_BOARD_REV));
+			__raw_readb(OMAP1510_FPGA_REV_HIGH),
+			__raw_readb(OMAP1510_FPGA_REV_LOW),
+			__raw_readb(OMAP1510_FPGA_BOARD_REV));
+#endif
 }
 
 MACHINE_START(OMAP_INNOVATOR, "TI-Innovator")
diff --git a/arch/arm/mach-omap1/board-perseus2.c b/arch/arm/mach-omap1/board-perseus2.c
index 198b054..a1cdeeb 100644
--- a/arch/arm/mach-omap1/board-perseus2.c
+++ b/arch/arm/mach-omap1/board-perseus2.c
@@ -30,7 +30,7 @@
 
 #include <plat/tc.h>
 #include <mach/mux.h>
-#include <plat/fpga.h>
+#include <../plat-omap/fpga.h>
 #include <mach/flash.h>
 
 #include <mach/hardware.h>
@@ -231,9 +231,9 @@ static struct omap_lcd_config perseus2_lcd_config __initdata = {
 
 static void __init perseus2_init_smc91x(void)
 {
-	fpga_write(1, H2P2_DBG_FPGA_LAN_RESET);
+	__raw_writeb(1, H2P2_DBG_FPGA_LAN_RESET);
 	mdelay(50);
-	fpga_write(fpga_read(H2P2_DBG_FPGA_LAN_RESET) & ~1,
+	__raw_writeb(__raw_readb(H2P2_DBG_FPGA_LAN_RESET) & ~1,
 		   H2P2_DBG_FPGA_LAN_RESET);
 	mdelay(50);
 }
diff --git a/arch/arm/mach-omap1/common.h b/arch/arm/mach-omap1/common.h
index 9f2d8b6..26e19d3 100644
--- a/arch/arm/mach-omap1/common.h
+++ b/arch/arm/mach-omap1/common.h
@@ -41,6 +41,7 @@ static inline void omap7xx_map_io(void)
 #endif
 
 #ifdef CONFIG_ARCH_OMAP15XX
+void omap1510_fpga_init_irq(void);
 void omap15xx_map_io(void);
 #else
 static inline void omap15xx_map_io(void)
diff --git a/arch/arm/mach-omap1/fpga.c b/arch/arm/mach-omap1/fpga.c
index 29ec50f..4ec220d 100644
--- a/arch/arm/mach-omap1/fpga.c
+++ b/arch/arm/mach-omap1/fpga.c
@@ -27,7 +27,7 @@
 #include <asm/irq.h>
 #include <asm/mach/irq.h>
 
-#include <plat/fpga.h>
+#include <../plat-omap/fpga.h>
 
 #include <mach/hardware.h>
 
diff --git a/arch/arm/mach-omap1/include/mach/omap1510.h b/arch/arm/mach-omap1/include/mach/omap1510.h
index 8fe05d6..3d23524 100644
--- a/arch/arm/mach-omap1/include/mach/omap1510.h
+++ b/arch/arm/mach-omap1/include/mach/omap1510.h
@@ -45,5 +45,118 @@
 
 #define OMAP1510_DSP_MMU_BASE	(0xfffed200)
 
+/*
+ * ---------------------------------------------------------------------------
+ *  OMAP-1510 FPGA
+ * ---------------------------------------------------------------------------
+ */
+#define OMAP1510_FPGA_BASE		0xE8000000		/* VA */
+#define OMAP1510_FPGA_SIZE		SZ_4K
+#define OMAP1510_FPGA_START		0x08000000		/* PA */
+
+/* Revision */
+#define OMAP1510_FPGA_REV_LOW			IOMEM(OMAP1510_FPGA_BASE + 0x0)
+#define OMAP1510_FPGA_REV_HIGH			IOMEM(OMAP1510_FPGA_BASE + 0x1)
+#define OMAP1510_FPGA_LCD_PANEL_CONTROL		IOMEM(OMAP1510_FPGA_BASE + 0x2)
+#define OMAP1510_FPGA_LED_DIGIT			IOMEM(OMAP1510_FPGA_BASE + 0x3)
+#define INNOVATOR_FPGA_HID_SPI			IOMEM(OMAP1510_FPGA_BASE + 0x4)
+#define OMAP1510_FPGA_POWER			IOMEM(OMAP1510_FPGA_BASE + 0x5)
+
+/* Interrupt status */
+#define OMAP1510_FPGA_ISR_LO			IOMEM(OMAP1510_FPGA_BASE + 0x6)
+#define OMAP1510_FPGA_ISR_HI			IOMEM(OMAP1510_FPGA_BASE + 0x7)
+
+/* Interrupt mask */
+#define OMAP1510_FPGA_IMR_LO			IOMEM(OMAP1510_FPGA_BASE + 0x8)
+#define OMAP1510_FPGA_IMR_HI			IOMEM(OMAP1510_FPGA_BASE + 0x9)
+
+/* Reset registers */
+#define OMAP1510_FPGA_HOST_RESET		IOMEM(OMAP1510_FPGA_BASE + 0xa)
+#define OMAP1510_FPGA_RST			IOMEM(OMAP1510_FPGA_BASE + 0xb)
+
+#define OMAP1510_FPGA_AUDIO			IOMEM(OMAP1510_FPGA_BASE + 0xc)
+#define OMAP1510_FPGA_DIP			IOMEM(OMAP1510_FPGA_BASE + 0xe)
+#define OMAP1510_FPGA_FPGA_IO			IOMEM(OMAP1510_FPGA_BASE + 0xf)
+#define OMAP1510_FPGA_UART1			IOMEM(OMAP1510_FPGA_BASE + 0x14)
+#define OMAP1510_FPGA_UART2			IOMEM(OMAP1510_FPGA_BASE + 0x15)
+#define OMAP1510_FPGA_OMAP1510_STATUS		IOMEM(OMAP1510_FPGA_BASE + 0x16)
+#define OMAP1510_FPGA_BOARD_REV			IOMEM(OMAP1510_FPGA_BASE + 0x18)
+#define INNOVATOR_FPGA_CAM_USB_CONTROL		IOMEM(OMAP1510_FPGA_BASE + 0x20c)
+#define OMAP1510P1_PPT_DATA			IOMEM(OMAP1510_FPGA_BASE + 0x100)
+#define OMAP1510P1_PPT_STATUS			IOMEM(OMAP1510_FPGA_BASE + 0x101)
+#define OMAP1510P1_PPT_CONTROL			IOMEM(OMAP1510_FPGA_BASE + 0x102)
+
+#define OMAP1510_FPGA_TOUCHSCREEN		IOMEM(OMAP1510_FPGA_BASE + 0x204)
+
+#define INNOVATOR_FPGA_INFO			IOMEM(OMAP1510_FPGA_BASE + 0x205)
+#define INNOVATOR_FPGA_LCD_BRIGHT_LO		IOMEM(OMAP1510_FPGA_BASE + 0x206)
+#define INNOVATOR_FPGA_LCD_BRIGHT_HI		IOMEM(OMAP1510_FPGA_BASE + 0x207)
+#define INNOVATOR_FPGA_LED_GRN_LO		IOMEM(OMAP1510_FPGA_BASE + 0x208)
+#define INNOVATOR_FPGA_LED_GRN_HI		IOMEM(OMAP1510_FPGA_BASE + 0x209)
+#define INNOVATOR_FPGA_LED_RED_LO		IOMEM(OMAP1510_FPGA_BASE + 0x20a)
+#define INNOVATOR_FPGA_LED_RED_HI		IOMEM(OMAP1510_FPGA_BASE + 0x20b)
+#define INNOVATOR_FPGA_EXP_CONTROL		IOMEM(OMAP1510_FPGA_BASE + 0x20d)
+#define INNOVATOR_FPGA_ISR2			IOMEM(OMAP1510_FPGA_BASE + 0x20e)
+#define INNOVATOR_FPGA_IMR2			IOMEM(OMAP1510_FPGA_BASE + 0x210)
+
+#define OMAP1510_FPGA_ETHR_START		(OMAP1510_FPGA_START + 0x300)
+
+/*
+ * Power up Giga UART driver, turn on HID clock.
+ * Turn off BT power, since we're not using it and it
+ * draws power.
+ */
+#define OMAP1510_FPGA_RESET_VALUE		0x42
+
+#define OMAP1510_FPGA_PCR_IF_PD0		(1 << 7)
+#define OMAP1510_FPGA_PCR_COM2_EN		(1 << 6)
+#define OMAP1510_FPGA_PCR_COM1_EN		(1 << 5)
+#define OMAP1510_FPGA_PCR_EXP_PD0		(1 << 4)
+#define OMAP1510_FPGA_PCR_EXP_PD1		(1 << 3)
+#define OMAP1510_FPGA_PCR_48MHZ_CLK		(1 << 2)
+#define OMAP1510_FPGA_PCR_4MHZ_CLK		(1 << 1)
+#define OMAP1510_FPGA_PCR_RSRVD_BIT0		(1 << 0)
+
+/*
+ * Innovator/OMAP1510 FPGA HID register bit definitions
+ */
+#define OMAP1510_FPGA_HID_SCLK	(1<<0)	/* output */
+#define OMAP1510_FPGA_HID_MOSI	(1<<1)	/* output */
+#define OMAP1510_FPGA_HID_nSS	(1<<2)	/* output 0/1 chip idle/select */
+#define OMAP1510_FPGA_HID_nHSUS	(1<<3)	/* output 0/1 host active/suspended */
+#define OMAP1510_FPGA_HID_MISO	(1<<4)	/* input */
+#define OMAP1510_FPGA_HID_ATN	(1<<5)	/* input  0/1 chip idle/ATN */
+#define OMAP1510_FPGA_HID_rsrvd	(1<<6)
+#define OMAP1510_FPGA_HID_RESETn (1<<7)	/* output - 0/1 USAR reset/run */
+
+/* The FPGA IRQ is cascaded through GPIO_13 */
+#define OMAP1510_INT_FPGA		(IH_GPIO_BASE + 13)
+
+/* IRQ Numbers for interrupts muxed through the FPGA */
+#define OMAP1510_INT_FPGA_ATN		(OMAP_FPGA_IRQ_BASE + 0)
+#define OMAP1510_INT_FPGA_ACK		(OMAP_FPGA_IRQ_BASE + 1)
+#define OMAP1510_INT_FPGA2		(OMAP_FPGA_IRQ_BASE + 2)
+#define OMAP1510_INT_FPGA3		(OMAP_FPGA_IRQ_BASE + 3)
+#define OMAP1510_INT_FPGA4		(OMAP_FPGA_IRQ_BASE + 4)
+#define OMAP1510_INT_FPGA5		(OMAP_FPGA_IRQ_BASE + 5)
+#define OMAP1510_INT_FPGA6		(OMAP_FPGA_IRQ_BASE + 6)
+#define OMAP1510_INT_FPGA7		(OMAP_FPGA_IRQ_BASE + 7)
+#define OMAP1510_INT_FPGA8		(OMAP_FPGA_IRQ_BASE + 8)
+#define OMAP1510_INT_FPGA9		(OMAP_FPGA_IRQ_BASE + 9)
+#define OMAP1510_INT_FPGA10		(OMAP_FPGA_IRQ_BASE + 10)
+#define OMAP1510_INT_FPGA11		(OMAP_FPGA_IRQ_BASE + 11)
+#define OMAP1510_INT_FPGA12		(OMAP_FPGA_IRQ_BASE + 12)
+#define OMAP1510_INT_ETHER		(OMAP_FPGA_IRQ_BASE + 13)
+#define OMAP1510_INT_FPGAUART1		(OMAP_FPGA_IRQ_BASE + 14)
+#define OMAP1510_INT_FPGAUART2		(OMAP_FPGA_IRQ_BASE + 15)
+#define OMAP1510_INT_FPGA_TS		(OMAP_FPGA_IRQ_BASE + 16)
+#define OMAP1510_INT_FPGA17		(OMAP_FPGA_IRQ_BASE + 17)
+#define OMAP1510_INT_FPGA_CAM		(OMAP_FPGA_IRQ_BASE + 18)
+#define OMAP1510_INT_FPGA_RTC_A		(OMAP_FPGA_IRQ_BASE + 19)
+#define OMAP1510_INT_FPGA_RTC_B		(OMAP_FPGA_IRQ_BASE + 20)
+#define OMAP1510_INT_FPGA_CD		(OMAP_FPGA_IRQ_BASE + 21)
+#define OMAP1510_INT_FPGA22		(OMAP_FPGA_IRQ_BASE + 22)
+#define OMAP1510_INT_FPGA23		(OMAP_FPGA_IRQ_BASE + 23)
+
 #endif /*  __ASM_ARCH_OMAP15XX_H */
 
diff --git a/arch/arm/mach-omap1/serial.c b/arch/arm/mach-omap1/serial.c
index b9d6834..d1ac080 100644
--- a/arch/arm/mach-omap1/serial.c
+++ b/arch/arm/mach-omap1/serial.c
@@ -23,7 +23,6 @@
 #include <asm/mach-types.h>
 
 #include <mach/mux.h>
-#include <plat/fpga.h>
 
 #include "pm.h"
 
diff --git a/arch/arm/plat-omap/debug-leds.c b/arch/arm/plat-omap/debug-leds.c
index ea29bbe..feca128 100644
--- a/arch/arm/plat-omap/debug-leds.c
+++ b/arch/arm/plat-omap/debug-leds.c
@@ -20,7 +20,7 @@
 #include <mach/hardware.h>
 #include <asm/mach-types.h>
 
-#include <plat/fpga.h>
+#include "fpga.h"
 
 /* Many OMAP development platforms reuse the same "debug board"; these
  * platforms include H2, H3, H4, and Perseus2.  There are 16 LEDs on the
diff --git a/arch/arm/plat-omap/fpga.h b/arch/arm/plat-omap/fpga.h
new file mode 100644
index 0000000..54faaa9
--- /dev/null
+++ b/arch/arm/plat-omap/fpga.h
@@ -0,0 +1,74 @@
+/*
+ * arch/arm/plat-omap/include/mach/fpga.h
+ *
+ * Interrupt handler for OMAP-1510 FPGA
+ *
+ * Copyright (C) 2001 RidgeRun, Inc.
+ * Author: Greg Lonnon <glonnon@ridgerun.com>
+ *
+ * Copyright (C) 2002 MontaVista Software, Inc.
+ *
+ * Separated FPGA interrupts from innovator1510.c and cleaned up for 2.6
+ * Copyright (C) 2004 Nokia Corporation by Tony Lindrgen <tony@atomide.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_ARCH_OMAP_FPGA_H
+#define __ASM_ARCH_OMAP_FPGA_H
+
+/*
+ * ---------------------------------------------------------------------------
+ *  H2/P2 Debug board FPGA
+ * ---------------------------------------------------------------------------
+ */
+/* maps in the FPGA registers and the ETHR registers */
+#define H2P2_DBG_FPGA_BASE		0xE8000000		/* VA */
+#define H2P2_DBG_FPGA_SIZE		SZ_4K			/* SIZE */
+#define H2P2_DBG_FPGA_START		0x04000000		/* PA */
+
+#define H2P2_DBG_FPGA_ETHR_START	(H2P2_DBG_FPGA_START + 0x300)
+#define H2P2_DBG_FPGA_FPGA_REV		IOMEM(H2P2_DBG_FPGA_BASE + 0x10)	/* FPGA Revision */
+#define H2P2_DBG_FPGA_BOARD_REV		IOMEM(H2P2_DBG_FPGA_BASE + 0x12)	/* Board Revision */
+#define H2P2_DBG_FPGA_GPIO		IOMEM(H2P2_DBG_FPGA_BASE + 0x14)	/* GPIO outputs */
+#define H2P2_DBG_FPGA_LEDS		IOMEM(H2P2_DBG_FPGA_BASE + 0x16)	/* LEDs outputs */
+#define H2P2_DBG_FPGA_MISC_INPUTS	IOMEM(H2P2_DBG_FPGA_BASE + 0x18)	/* Misc inputs */
+#define H2P2_DBG_FPGA_LAN_STATUS	IOMEM(H2P2_DBG_FPGA_BASE + 0x1A)	/* LAN Status line */
+#define H2P2_DBG_FPGA_LAN_RESET		IOMEM(H2P2_DBG_FPGA_BASE + 0x1C)	/* LAN Reset line */
+
+/* NOTE:  most boards don't have a static mapping for the FPGA ... */
+struct h2p2_dbg_fpga {
+	/* offset 0x00 */
+	u16		smc91x[8];
+	/* offset 0x10 */
+	u16		fpga_rev;
+	u16		board_rev;
+	u16		gpio_outputs;
+	u16		leds;
+	/* offset 0x18 */
+	u16		misc_inputs;
+	u16		lan_status;
+	u16		lan_reset;
+	u16		reserved0;
+	/* offset 0x20 */
+	u16		ps2_data;
+	u16		ps2_ctrl;
+	/* plus also 4 rs232 ports ... */
+};
+
+/* LEDs definition on debug board (16 LEDs, all physically green) */
+#define H2P2_DBG_FPGA_LED_GREEN		(1 << 15)
+#define H2P2_DBG_FPGA_LED_AMBER		(1 << 14)
+#define H2P2_DBG_FPGA_LED_RED		(1 << 13)
+#define H2P2_DBG_FPGA_LED_BLUE		(1 << 12)
+/*  cpu0 load-meter LEDs */
+#define H2P2_DBG_FPGA_LOAD_METER	(1 << 0)	// A bit of fun on our board ...
+#define H2P2_DBG_FPGA_LOAD_METER_SIZE	11
+#define H2P2_DBG_FPGA_LOAD_METER_MASK	((1 << H2P2_DBG_FPGA_LOAD_METER_SIZE) - 1)
+
+#define H2P2_DBG_FPGA_P2_LED_TIMER		(1 << 0)
+#define H2P2_DBG_FPGA_P2_LED_IDLE		(1 << 1)
+
+#endif
diff --git a/arch/arm/plat-omap/include/plat/fpga.h b/arch/arm/plat-omap/include/plat/fpga.h
deleted file mode 100644
index bd3c632..0000000
--- a/arch/arm/plat-omap/include/plat/fpga.h
+++ /dev/null
@@ -1,193 +0,0 @@
-/*
- * arch/arm/plat-omap/include/mach/fpga.h
- *
- * Interrupt handler for OMAP-1510 FPGA
- *
- * Copyright (C) 2001 RidgeRun, Inc.
- * Author: Greg Lonnon <glonnon@ridgerun.com>
- *
- * Copyright (C) 2002 MontaVista Software, Inc.
- *
- * Separated FPGA interrupts from innovator1510.c and cleaned up for 2.6
- * Copyright (C) 2004 Nokia Corporation by Tony Lindrgen <tony@atomide.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#ifndef __ASM_ARCH_OMAP_FPGA_H
-#define __ASM_ARCH_OMAP_FPGA_H
-
-extern void omap1510_fpga_init_irq(void);
-
-#define fpga_read(reg)			__raw_readb(reg)
-#define fpga_write(val, reg)		__raw_writeb(val, reg)
-
-/*
- * ---------------------------------------------------------------------------
- *  H2/P2 Debug board FPGA
- * ---------------------------------------------------------------------------
- */
-/* maps in the FPGA registers and the ETHR registers */
-#define H2P2_DBG_FPGA_BASE		0xE8000000		/* VA */
-#define H2P2_DBG_FPGA_SIZE		SZ_4K			/* SIZE */
-#define H2P2_DBG_FPGA_START		0x04000000		/* PA */
-
-#define H2P2_DBG_FPGA_ETHR_START	(H2P2_DBG_FPGA_START + 0x300)
-#define H2P2_DBG_FPGA_FPGA_REV		IOMEM(H2P2_DBG_FPGA_BASE + 0x10)	/* FPGA Revision */
-#define H2P2_DBG_FPGA_BOARD_REV		IOMEM(H2P2_DBG_FPGA_BASE + 0x12)	/* Board Revision */
-#define H2P2_DBG_FPGA_GPIO		IOMEM(H2P2_DBG_FPGA_BASE + 0x14)	/* GPIO outputs */
-#define H2P2_DBG_FPGA_LEDS		IOMEM(H2P2_DBG_FPGA_BASE + 0x16)	/* LEDs outputs */
-#define H2P2_DBG_FPGA_MISC_INPUTS	IOMEM(H2P2_DBG_FPGA_BASE + 0x18)	/* Misc inputs */
-#define H2P2_DBG_FPGA_LAN_STATUS	IOMEM(H2P2_DBG_FPGA_BASE + 0x1A)	/* LAN Status line */
-#define H2P2_DBG_FPGA_LAN_RESET		IOMEM(H2P2_DBG_FPGA_BASE + 0x1C)	/* LAN Reset line */
-
-/* NOTE:  most boards don't have a static mapping for the FPGA ... */
-struct h2p2_dbg_fpga {
-	/* offset 0x00 */
-	u16		smc91x[8];
-	/* offset 0x10 */
-	u16		fpga_rev;
-	u16		board_rev;
-	u16		gpio_outputs;
-	u16		leds;
-	/* offset 0x18 */
-	u16		misc_inputs;
-	u16		lan_status;
-	u16		lan_reset;
-	u16		reserved0;
-	/* offset 0x20 */
-	u16		ps2_data;
-	u16		ps2_ctrl;
-	/* plus also 4 rs232 ports ... */
-};
-
-/* LEDs definition on debug board (16 LEDs, all physically green) */
-#define H2P2_DBG_FPGA_LED_GREEN		(1 << 15)
-#define H2P2_DBG_FPGA_LED_AMBER		(1 << 14)
-#define H2P2_DBG_FPGA_LED_RED		(1 << 13)
-#define H2P2_DBG_FPGA_LED_BLUE		(1 << 12)
-/*  cpu0 load-meter LEDs */
-#define H2P2_DBG_FPGA_LOAD_METER	(1 << 0)	// A bit of fun on our board ...
-#define H2P2_DBG_FPGA_LOAD_METER_SIZE	11
-#define H2P2_DBG_FPGA_LOAD_METER_MASK	((1 << H2P2_DBG_FPGA_LOAD_METER_SIZE) - 1)
-
-#define H2P2_DBG_FPGA_P2_LED_TIMER		(1 << 0)
-#define H2P2_DBG_FPGA_P2_LED_IDLE		(1 << 1)
-
-/*
- * ---------------------------------------------------------------------------
- *  OMAP-1510 FPGA
- * ---------------------------------------------------------------------------
- */
-#define OMAP1510_FPGA_BASE		0xE8000000		/* VA */
-#define OMAP1510_FPGA_SIZE		SZ_4K
-#define OMAP1510_FPGA_START		0x08000000		/* PA */
-
-/* Revision */
-#define OMAP1510_FPGA_REV_LOW			IOMEM(OMAP1510_FPGA_BASE + 0x0)
-#define OMAP1510_FPGA_REV_HIGH			IOMEM(OMAP1510_FPGA_BASE + 0x1)
-
-#define OMAP1510_FPGA_LCD_PANEL_CONTROL		IOMEM(OMAP1510_FPGA_BASE + 0x2)
-#define OMAP1510_FPGA_LED_DIGIT			IOMEM(OMAP1510_FPGA_BASE + 0x3)
-#define INNOVATOR_FPGA_HID_SPI			IOMEM(OMAP1510_FPGA_BASE + 0x4)
-#define OMAP1510_FPGA_POWER			IOMEM(OMAP1510_FPGA_BASE + 0x5)
-
-/* Interrupt status */
-#define OMAP1510_FPGA_ISR_LO			IOMEM(OMAP1510_FPGA_BASE + 0x6)
-#define OMAP1510_FPGA_ISR_HI			IOMEM(OMAP1510_FPGA_BASE + 0x7)
-
-/* Interrupt mask */
-#define OMAP1510_FPGA_IMR_LO			IOMEM(OMAP1510_FPGA_BASE + 0x8)
-#define OMAP1510_FPGA_IMR_HI			IOMEM(OMAP1510_FPGA_BASE + 0x9)
-
-/* Reset registers */
-#define OMAP1510_FPGA_HOST_RESET		IOMEM(OMAP1510_FPGA_BASE + 0xa)
-#define OMAP1510_FPGA_RST			IOMEM(OMAP1510_FPGA_BASE + 0xb)
-
-#define OMAP1510_FPGA_AUDIO			IOMEM(OMAP1510_FPGA_BASE + 0xc)
-#define OMAP1510_FPGA_DIP			IOMEM(OMAP1510_FPGA_BASE + 0xe)
-#define OMAP1510_FPGA_FPGA_IO			IOMEM(OMAP1510_FPGA_BASE + 0xf)
-#define OMAP1510_FPGA_UART1			IOMEM(OMAP1510_FPGA_BASE + 0x14)
-#define OMAP1510_FPGA_UART2			IOMEM(OMAP1510_FPGA_BASE + 0x15)
-#define OMAP1510_FPGA_OMAP1510_STATUS		IOMEM(OMAP1510_FPGA_BASE + 0x16)
-#define OMAP1510_FPGA_BOARD_REV			IOMEM(OMAP1510_FPGA_BASE + 0x18)
-#define OMAP1510P1_PPT_DATA			IOMEM(OMAP1510_FPGA_BASE + 0x100)
-#define OMAP1510P1_PPT_STATUS			IOMEM(OMAP1510_FPGA_BASE + 0x101)
-#define OMAP1510P1_PPT_CONTROL			IOMEM(OMAP1510_FPGA_BASE + 0x102)
-
-#define OMAP1510_FPGA_TOUCHSCREEN		IOMEM(OMAP1510_FPGA_BASE + 0x204)
-
-#define INNOVATOR_FPGA_INFO			IOMEM(OMAP1510_FPGA_BASE + 0x205)
-#define INNOVATOR_FPGA_LCD_BRIGHT_LO		IOMEM(OMAP1510_FPGA_BASE + 0x206)
-#define INNOVATOR_FPGA_LCD_BRIGHT_HI		IOMEM(OMAP1510_FPGA_BASE + 0x207)
-#define INNOVATOR_FPGA_LED_GRN_LO		IOMEM(OMAP1510_FPGA_BASE + 0x208)
-#define INNOVATOR_FPGA_LED_GRN_HI		IOMEM(OMAP1510_FPGA_BASE + 0x209)
-#define INNOVATOR_FPGA_LED_RED_LO		IOMEM(OMAP1510_FPGA_BASE + 0x20a)
-#define INNOVATOR_FPGA_LED_RED_HI		IOMEM(OMAP1510_FPGA_BASE + 0x20b)
-#define INNOVATOR_FPGA_CAM_USB_CONTROL		IOMEM(OMAP1510_FPGA_BASE + 0x20c)
-#define INNOVATOR_FPGA_EXP_CONTROL		IOMEM(OMAP1510_FPGA_BASE + 0x20d)
-#define INNOVATOR_FPGA_ISR2			IOMEM(OMAP1510_FPGA_BASE + 0x20e)
-#define INNOVATOR_FPGA_IMR2			IOMEM(OMAP1510_FPGA_BASE + 0x210)
-
-#define OMAP1510_FPGA_ETHR_START		(OMAP1510_FPGA_START + 0x300)
-
-/*
- * Power up Giga UART driver, turn on HID clock.
- * Turn off BT power, since we're not using it and it
- * draws power.
- */
-#define OMAP1510_FPGA_RESET_VALUE		0x42
-
-#define OMAP1510_FPGA_PCR_IF_PD0		(1 << 7)
-#define OMAP1510_FPGA_PCR_COM2_EN		(1 << 6)
-#define OMAP1510_FPGA_PCR_COM1_EN		(1 << 5)
-#define OMAP1510_FPGA_PCR_EXP_PD0		(1 << 4)
-#define OMAP1510_FPGA_PCR_EXP_PD1		(1 << 3)
-#define OMAP1510_FPGA_PCR_48MHZ_CLK		(1 << 2)
-#define OMAP1510_FPGA_PCR_4MHZ_CLK		(1 << 1)
-#define OMAP1510_FPGA_PCR_RSRVD_BIT0		(1 << 0)
-
-/*
- * Innovator/OMAP1510 FPGA HID register bit definitions
- */
-#define OMAP1510_FPGA_HID_SCLK	(1<<0)	/* output */
-#define OMAP1510_FPGA_HID_MOSI	(1<<1)	/* output */
-#define OMAP1510_FPGA_HID_nSS	(1<<2)	/* output 0/1 chip idle/select */
-#define OMAP1510_FPGA_HID_nHSUS	(1<<3)	/* output 0/1 host active/suspended */
-#define OMAP1510_FPGA_HID_MISO	(1<<4)	/* input */
-#define OMAP1510_FPGA_HID_ATN	(1<<5)	/* input  0/1 chip idle/ATN */
-#define OMAP1510_FPGA_HID_rsrvd	(1<<6)
-#define OMAP1510_FPGA_HID_RESETn (1<<7)	/* output - 0/1 USAR reset/run */
-
-/* The FPGA IRQ is cascaded through GPIO_13 */
-#define OMAP1510_INT_FPGA		(IH_GPIO_BASE + 13)
-
-/* IRQ Numbers for interrupts muxed through the FPGA */
-#define OMAP1510_INT_FPGA_ATN		(OMAP_FPGA_IRQ_BASE + 0)
-#define OMAP1510_INT_FPGA_ACK		(OMAP_FPGA_IRQ_BASE + 1)
-#define OMAP1510_INT_FPGA2		(OMAP_FPGA_IRQ_BASE + 2)
-#define OMAP1510_INT_FPGA3		(OMAP_FPGA_IRQ_BASE + 3)
-#define OMAP1510_INT_FPGA4		(OMAP_FPGA_IRQ_BASE + 4)
-#define OMAP1510_INT_FPGA5		(OMAP_FPGA_IRQ_BASE + 5)
-#define OMAP1510_INT_FPGA6		(OMAP_FPGA_IRQ_BASE + 6)
-#define OMAP1510_INT_FPGA7		(OMAP_FPGA_IRQ_BASE + 7)
-#define OMAP1510_INT_FPGA8		(OMAP_FPGA_IRQ_BASE + 8)
-#define OMAP1510_INT_FPGA9		(OMAP_FPGA_IRQ_BASE + 9)
-#define OMAP1510_INT_FPGA10		(OMAP_FPGA_IRQ_BASE + 10)
-#define OMAP1510_INT_FPGA11		(OMAP_FPGA_IRQ_BASE + 11)
-#define OMAP1510_INT_FPGA12		(OMAP_FPGA_IRQ_BASE + 12)
-#define OMAP1510_INT_ETHER		(OMAP_FPGA_IRQ_BASE + 13)
-#define OMAP1510_INT_FPGAUART1		(OMAP_FPGA_IRQ_BASE + 14)
-#define OMAP1510_INT_FPGAUART2		(OMAP_FPGA_IRQ_BASE + 15)
-#define OMAP1510_INT_FPGA_TS		(OMAP_FPGA_IRQ_BASE + 16)
-#define OMAP1510_INT_FPGA17		(OMAP_FPGA_IRQ_BASE + 17)
-#define OMAP1510_INT_FPGA_CAM		(OMAP_FPGA_IRQ_BASE + 18)
-#define OMAP1510_INT_FPGA_RTC_A		(OMAP_FPGA_IRQ_BASE + 19)
-#define OMAP1510_INT_FPGA_RTC_B		(OMAP_FPGA_IRQ_BASE + 20)
-#define OMAP1510_INT_FPGA_CD		(OMAP_FPGA_IRQ_BASE + 21)
-#define OMAP1510_INT_FPGA22		(OMAP_FPGA_IRQ_BASE + 22)
-#define OMAP1510_INT_FPGA23		(OMAP_FPGA_IRQ_BASE + 23)
-
-#endif
diff --git a/drivers/usb/host/ohci-omap.c b/drivers/usb/host/ohci-omap.c
index 4531d03..439e6e4 100644
--- a/drivers/usb/host/ohci-omap.c
+++ b/drivers/usb/host/ohci-omap.c
@@ -25,7 +25,6 @@
 #include <asm/mach-types.h>
 
 #include <mach/mux.h>
-#include <plat/fpga.h>
 
 #include <mach/hardware.h>
 #include <mach/irqs.h>
@@ -93,14 +92,14 @@ static int omap_ohci_transceiver_power(int on)
 {
 	if (on) {
 		if (machine_is_omap_innovator() && cpu_is_omap1510())
-			fpga_write(fpga_read(INNOVATOR_FPGA_CAM_USB_CONTROL)
+			__raw_writeb(__raw_readb(INNOVATOR_FPGA_CAM_USB_CONTROL)
 				| ((1 << 5/*usb1*/) | (1 << 3/*usb2*/)),
 			       INNOVATOR_FPGA_CAM_USB_CONTROL);
 		else if (machine_is_omap_osk())
 			tps65010_set_gpio_out_value(GPIO1, LOW);
 	} else {
 		if (machine_is_omap_innovator() && cpu_is_omap1510())
-			fpga_write(fpga_read(INNOVATOR_FPGA_CAM_USB_CONTROL)
+			__raw_writeb(__raw_readb(INNOVATOR_FPGA_CAM_USB_CONTROL)
 				& ~((1 << 5/*usb1*/) | (1 << 3/*usb2*/)),
 			       INNOVATOR_FPGA_CAM_USB_CONTROL);
 		else if (machine_is_omap_osk())
diff --git a/drivers/video/omap/lcd_inn1510.c b/drivers/video/omap/lcd_inn1510.c
index b38b1dd..2ee4232 100644
--- a/drivers/video/omap/lcd_inn1510.c
+++ b/drivers/video/omap/lcd_inn1510.c
@@ -23,7 +23,8 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 
-#include <plat/fpga.h>
+#include <mach/hardware.h>
+
 #include "omapfb.h"
 
 static int innovator1510_panel_init(struct lcd_panel *panel,
@@ -38,13 +39,13 @@ static void innovator1510_panel_cleanup(struct lcd_panel *panel)
 
 static int innovator1510_panel_enable(struct lcd_panel *panel)
 {
-	fpga_write(0x7, OMAP1510_FPGA_LCD_PANEL_CONTROL);
+	__raw_writeb(0x7, OMAP1510_FPGA_LCD_PANEL_CONTROL);
 	return 0;
 }
 
 static void innovator1510_panel_disable(struct lcd_panel *panel)
 {
-	fpga_write(0x0, OMAP1510_FPGA_LCD_PANEL_CONTROL);
+	__raw_writeb(0x0, OMAP1510_FPGA_LCD_PANEL_CONTROL);
 }
 
 static unsigned long innovator1510_panel_get_caps(struct lcd_panel *panel)


^ permalink raw reply related

* Re: [PATCH 1/2 v6] of: add helper to parse display timings
From: Guennadi Liakhovetski @ 2012-10-04 21:35 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
	Laurent Pinchart, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1349373560-11128-2-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Hi Steffen

Sorry for chiming in so late in the game, but I've long been wanting to 
have a look at this and compare with what we do for V4L2, so, this seems a 
great opportunity to me:-)

On Thu, 4 Oct 2012, Steffen Trumtrar wrote:

> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  .../devicetree/bindings/video/display-timings.txt  |  222 ++++++++++++++++++++
>  drivers/of/Kconfig                                 |    5 +
>  drivers/of/Makefile                                |    1 +
>  drivers/of/of_display_timings.c                    |  183 ++++++++++++++++
>  include/linux/of_display_timings.h                 |   85 ++++++++
>  5 files changed, 496 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
>  create mode 100644 drivers/of/of_display_timings.c
>  create mode 100644 include/linux/of_display_timings.h
> 
> diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt
> new file mode 100644
> index 0000000..45e39bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/display-timings.txt
> @@ -0,0 +1,222 @@
> +display-timings bindings
> +=========
> +
> +display-timings-node
> +------------
> +
> +required properties:
> + - none
> +
> +optional properties:
> + - default-timing: the default timing value
> +
> +timings-subnode
> +---------------
> +
> +required properties:
> + - hactive, vactive: Display resolution
> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
> +   in pixels
> +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
> +   lines
> + - clock: displayclock in Hz

You're going to hate me for this, but eventually we want to actually 
reference clock objects in our DT bindings. For now, even if you don't 
want to actually add clock phandles and stuff here, I think, using the 
standard "clock-frequency" property would be much better!

> +
> +optional properties:
> + - hsync-active-high (bool): Hsync pulse is active high
> + - vsync-active-high (bool): Vsync pulse is active high

For the above two we also considered using bool properties but eventually 
settled down with integer ones:

- hsync-active = <1>

for active-high and 0 for active low. This has the added advantage of 
being able to omit this property in the .dts, which then doesn't mean, 
that the polarity is active low, but rather, that the hsync line is not 
used on this hardware. So, maybe it would be good to use the same binding 
here too?

> + - de-active-high (bool): Data-Enable pulse is active high
> + - pixelclk-inverted (bool): pixelclock is inverted

We don't (yet) have a de-active property in V4L, don't know whether we'll 
ever have to distingsuish between what some datasheets call "HREF" and 
HSYNC in DT, but maybe similarly to the above an integer would be 
preferred. As for pixclk, we call the property "pclk-sample" and it's also 
an integer.

> + - interlaced (bool)

Is "interlaced" a property of the hardware, i.e. of the board? Can the 
same display controller on one board require interlaced data and on 
another board - progressive? BTW, I'm not very familiar with display 
interfaces, but for interlaced you probably sometimes use a field signal, 
whose polarity you also want to specify here? We use a "field-even-active" 
integer property for it.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* Re: [PATCH 2/2 v6] of: add generic videomode description
From: Stephen Warren @ 2012-10-04 18:51 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, linux-fbdev, dri-devel, Tomi Valkeinen,
	Laurent Pinchart, linux-media
In-Reply-To: <1349373560-11128-3-git-send-email-s.trumtrar@pengutronix.de>

On 10/04/2012 11:59 AM, Steffen Trumtrar wrote:
> Get videomode from devicetree in a format appropriate for the
> backend. drm_display_mode and fb_videomode are supported atm.
> Uses the display signal timings from of_display_timings

> +++ b/drivers/of/of_videomode.c

> +int videomode_from_timing(struct display_timings *disp, struct videomode *vm,

> +	st = display_timings_get(disp, index);
> +
> +	if (!st) {

It's a little odd to leave a blank line between those two lines.

Only half of the code in this file seems OF-related; the routines to
convert a timing to a videomode or drm display mode seem like they'd be
useful outside device tree, so I wonder if putting them into
of_videomode.c is the correct thing to do. Still, it's probably not a
big deal.

^ permalink raw reply

* Re: [PATCH 1/2 v6] of: add helper to parse display timings
From: Stephen Warren @ 2012-10-04 18:47 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, linux-fbdev, dri-devel, Tomi Valkeinen,
	Laurent Pinchart, linux-media
In-Reply-To: <1349373560-11128-2-git-send-email-s.trumtrar@pengutronix.de>

On 10/04/2012 11:59 AM, Steffen Trumtrar wrote:

A patch description would be useful for something like this.

> diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt
> new file mode 100644
...
> +Usage in backend
> +========

Everything before that point in the file looks fine to me.

Everything after this point in the file seems to be Linux-specific
implementation details. Does it really belong in the DT binding
documentation, rather than some Linux-specific documentation file?

> +struct drm_display_mode
> +===========> +
> +  +----------+---------------------------------------------+----------+-------+
> +  |          |                                             |          |       |  ↑
> +  |          |                                             |          |       |  |
> +  |          |                                             |          |       |  |
> +  +----------###############################################----------+-------+  |

I suspect the entire horizontal box above (and the entire vertical box
all the way down the left-hand side) should be on the bottom/right
instead of top/left. The reason I think this is because all of
vsync_start, vsync_end, vdisplay have to be referenced to some known
point, which is usually zero or the start of the timing definition, /or/
there would be some value indicating the size of the top marging/porch
in order to say where those other values are referenced to.

> +  |          #   ↑         ↑          ↑                    #          |       |  |
> +  |          #   |         |          |                    #          |       |  |
> +  |          #   |         |          |                    #          |       |  |
> +  |          #   |         |          |                    #          |       |  |
> +  |          #   |         |          |                    #          |       |  |
> +  |          #   |         |          |       hdisplay     #          |       |  |
> +  |          #<--+--------------------+------------------->#          |       |  |
> +  |          #   |         |          |                    #          |       |  |
> +  |          #   |vsync_start         |                    #          |       |  |
> +  |          #   |         |          |                    #          |       |  |
> +  |          #   |         |vsync_end |                    #          |       |  |
> +  |          #   |         |          |vdisplay            #          |       |  |
> +  |          #   |         |          |                    #          |       |  |
> +  |          #   |         |          |                    #          |       |  |
> +  |          #   |         |          |                    #          |       |  | vtotal
> +  |          #   |         |          |                    #          |       |  |
> +  |          #   |         |          |     hsync_start    #          |       |  |
> +  |          #<--+---------+----------+------------------------------>|       |  |
> +  |          #   |         |          |                    #          |       |  |
> +  |          #   |         |          |     hsync_end      #          |       |  |
> +  |          #<--+---------+----------+-------------------------------------->|  |
> +  |          #   |         |          ↓                    #          |       |  |
> +  +----------####|#########|################################----------+-------+  |
> +  |          |   |         |                               |          |       |  |
> +  |          |   |         |                               |          |       |  |
> +  |          |   ↓         |                               |          |       |  |
> +  +----------+-------------+-------------------------------+----------+-------+  |
> +  |          |             |                               |          |       |  |
> +  |          |             |                               |          |       |  |
> +  |          |             ↓                               |          |       |  ↓
> +  +----------+---------------------------------------------+----------+-------+
> +                                   htotal
> +   <------------------------------------------------------------------------->

> diff --git a/drivers/of/of_display_timings.c b/drivers/of/of_display_timings.c

> +static int parse_property(struct device_node *np, char *name,
> +				struct timing_entry *result)

> +	if (cells = 1)
> +		ret = of_property_read_u32_array(np, name, &result->typ, cells);

Should that branch not just set result->min/max to typ as well?
Presumably it'd prevent any code that interprets struct timing_entry
from having to check if those values were 0 or not?

> +	else if (cells = 3)
> +		ret = of_property_read_u32_array(np, name, &result->min, cells);

> +struct display_timings *of_get_display_timing_list(struct device_node *np)

> +	entry = of_parse_phandle(timings_np, "default-timing", 0);
> +
> +	if (!entry) {
> +		pr_info("%s: no default-timing specified\n", __func__);
> +		entry = of_find_node_by_name(np, "timing");

I don't think you want to require the node have an explicit name; I
don't recall the DT binding documentation making that a requirement.
Instead, can't you either just leave the default unset, or pick the
first DT child node, irrespective of name?

> +	if (!entry) {
> +		pr_info("%s: no timing specifications given\n", __func__);
> +		return disp;
> +	}

The DT bindings don't state that it's mandatory to have some timing
specified, although I agree that it makes sense in practice.

> +	for_each_child_of_node(timings_np, entry) {
> +		struct signal_timing *st;
> +
> +		st = of_get_display_timing(entry);
> +
> +		if (!st)
> +			continue;

I wonder if that shouldn't be an error?

> +		if (strcmp(default_timing, entry->full_name) = 0)
> +			disp->default_timing = disp->num_timings;

Hmm. Why not compare the node pointers rather than the name? Also, if
the parsing failed, then this can lead to default_timing being
uninitialized anyway...

> +		disp->timings[disp->num_timings] = st;
> +		disp->num_timings++;
> +	}

> +	if (disp->num_timings >= 0)
> +		pr_info("%s: got %d timings. Using timing #%d as default\n", __func__,
> +			disp->num_timings , disp->default_timing + 1);
> +	else
> +		pr_info("%s: no timings specified\n", __func__);

The message in the else clause is not necessarily true; there may have
been some specified, but they just couldn't be parsed.

> +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;
> +
> +	default_np = of_parse_phandle(np, "default-timing", 0);
> +
> +	if (default_np)
> +		return 0;

If this function checks that a default-timing property exists, shouldn't
the function be named of_display_default_timing_exists()?

^ permalink raw reply

* [PATCH 2/2 v6] of: add generic videomode description
From: Steffen Trumtrar @ 2012-10-04 17:59 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, linux-media, Tomi Valkeinen
In-Reply-To: <1349373560-11128-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: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/of/Kconfig           |    5 +
 drivers/of/Makefile          |    1 +
 drivers/of/of_videomode.c    |  212 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_videomode.h |   41 ++++++++
 4 files changed, 259 insertions(+)
 create mode 100644 drivers/of/of_videomode.c
 create mode 100644 include/linux/of_videomode.h

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 646deb0..74282e2 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -88,4 +88,9 @@ config OF_DISPLAY_TIMINGS
 	help
 	  helper to parse display timings from the devicetree
 
+config OF_VIDEOMODE
+	def_bool y
+	help
+	  helper to get videomodes from the devicetree
+
 endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index c8e9603..09d556f 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_PCI)	+= of_pci.o
 obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
 obj-$(CONFIG_OF_MTD)	+= of_mtd.o
 obj-$(CONFIG_OF_DISPLAY_TIMINGS) += of_display_timings.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
new file mode 100644
index 0000000..76ac16e
--- /dev/null
+++ b/drivers/of/of_videomode.c
@@ -0,0 +1,212 @@
+/*
+ * 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/fb.h>
+#include <linux/slab.h>
+#include <drm/drm_mode.h>
+#include <linux/of_display_timings.h>
+#include <linux/of_videomode.h>
+
+void dump_fb_videomode(struct fb_videomode *m)
+{
+        pr_debug("fb_videomode = %d %d %d %d %d %d %d %d %d %d %d %d %d\n",
+                m->refresh, m->xres, m->yres, m->pixclock, m->left_margin,
+                m->right_margin, m->upper_margin, m->lower_margin,
+                m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag);
+}
+
+void dump_drm_displaymode(struct drm_display_mode *m)
+{
+        pr_debug("drm_displaymode = %d %d %d %d %d %d %d %d %d\n",
+                m->hdisplay, m->hsync_start, m->hsync_end, m->htotal,
+                m->vdisplay, m->vsync_start, m->vsync_end, m->vtotal,
+                m->clock);
+}
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+			int index)
+{
+	struct signal_timing *st = NULL;
+
+	if (!vm)
+		return -EINVAL;
+
+	st = display_timings_get(disp, index);
+
+	if (!st) {
+		pr_err("%s: no signal timings found\n", __func__);
+		return -EINVAL;
+	}
+
+	vm->pixelclock = signal_timing_get_value(&st->pixelclock, 0);
+	vm->hactive = signal_timing_get_value(&st->hactive, 0);
+	vm->hfront_porch = signal_timing_get_value(&st->hfront_porch, 0);
+	vm->hback_porch = signal_timing_get_value(&st->hback_porch, 0);
+	vm->hsync_len = signal_timing_get_value(&st->hsync_len, 0);
+
+	vm->vactive = signal_timing_get_value(&st->vactive, 0);
+	vm->vfront_porch = signal_timing_get_value(&st->vfront_porch, 0);
+	vm->vback_porch = signal_timing_get_value(&st->vback_porch, 0);
+	vm->vsync_len = signal_timing_get_value(&st->vsync_len, 0);
+
+	vm->vah = st->vsync_pol_active_high;
+	vm->hah = st->hsync_pol_active_high;
+	vm->interlaced = st->interlaced;
+	vm->doublescan = st->doublescan;
+
+	return 0;
+}
+
+int of_get_videomode(struct device_node *np, struct videomode *vm, int index)
+{
+	struct display_timings *disp;
+	int ret = 0;
+
+	disp = of_get_display_timing_list(np);
+
+	if (!disp) {
+		pr_err("%s: no timings specified\n", __func__);
+		return -EINVAL;
+	}
+
+	if (index = OF_DEFAULT_TIMING)
+		index = disp->default_timing;
+
+	ret = videomode_from_timing(disp, vm, index);
+
+	if (ret)
+		return ret;
+
+	display_timings_release(disp);
+
+	if (!vm) {
+		pr_err("%s: could not get videomode %d\n", __func__, index);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
+
+#if defined(CONFIG_DRM)
+int videomode_to_display_mode(struct videomode *vm, struct drm_display_mode *dmode)
+{
+	memset(dmode, 0, sizeof(*dmode));
+
+	dmode->hdisplay = vm->hactive;
+	dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
+	dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
+	dmode->htotal = dmode->hsync_end + vm->hback_porch;
+
+	dmode->vdisplay = vm->vactive;
+	dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
+	dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
+	dmode->vtotal = dmode->vsync_end + vm->vback_porch;
+
+	dmode->clock = vm->pixelclock / 1000;
+
+	if (vm->hah)
+		dmode->flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		dmode->flags |= DRM_MODE_FLAG_NHSYNC;
+	if (vm->vah)
+		dmode->flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		dmode->flags |= DRM_MODE_FLAG_NVSYNC;
+	if (vm->interlaced)
+		dmode->flags |= DRM_MODE_FLAG_INTERLACE;
+	if (vm->doublescan)
+		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
+	drm_mode_set_name(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_to_display_mode);
+
+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);
+#else
+int videomode_to_display_mode(struct videomode *vm, struct drm_display_mode *dmode)
+{
+	return 0;
+}
+
+int of_get_drm_display_mode(struct device_node *np, struct drm_display_mode *dmode,
+			int index)
+{
+	return 0;
+}
+#endif
+
+int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode *fbmode)
+{
+	memset(fbmode, 0, sizeof(*fbmode));
+
+	fbmode->xres = vm->hactive;
+	fbmode->left_margin = vm->hback_porch;
+	fbmode->right_margin = vm->hfront_porch;
+	fbmode->hsync_len = vm->hsync_len;
+
+	fbmode->yres = vm->vactive;
+	fbmode->upper_margin = vm->vback_porch;
+	fbmode->lower_margin = vm->vfront_porch;
+	fbmode->vsync_len = vm->vsync_len;
+
+	fbmode->pixclock = KHZ2PICOS(vm->pixelclock) / 1000;
+
+	if (vm->hah)
+		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
+	if (vm->vah)
+		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->interlaced)
+		fbmode->vmode |= FB_VMODE_INTERLACED;
+	if (vm->doublescan)
+		fbmode->vmode |= FB_VMODE_DOUBLE;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_to_fb_videomode);
+
+int of_get_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_drm_display_mode);
diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
new file mode 100644
index 0000000..96efe01
--- /dev/null
+++ b/include/linux/of_videomode.h
@@ -0,0 +1,41 @@
+/*
+ * 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 <drm/drmP.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;
+
+	bool hah;
+	bool vah;
+	bool interlaced;
+	bool doublescan;
+
+};
+
+int videomode_to_display_mode(struct videomode *vm, struct drm_display_mode *dmode);
+int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode *fbmode);
+int of_get_videomode(struct device_node *np, struct videomode *vm, int index);
+int of_get_drm_display_mode(struct device_node *np, struct drm_display_mode *dmode,
+			int index);
+int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb, int index);
+#endif /* __LINUX_VIDEOMODE_H */
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 1/2 v6] of: add helper to parse display timings
From: Steffen Trumtrar @ 2012-10-04 17:59 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, linux-media, Tomi Valkeinen
In-Reply-To: <1349373560-11128-1-git-send-email-s.trumtrar@pengutronix.de>

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 .../devicetree/bindings/video/display-timings.txt  |  222 ++++++++++++++++++++
 drivers/of/Kconfig                                 |    5 +
 drivers/of/Makefile                                |    1 +
 drivers/of/of_display_timings.c                    |  183 ++++++++++++++++
 include/linux/of_display_timings.h                 |   85 ++++++++
 5 files changed, 496 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
 create mode 100644 drivers/of/of_display_timings.c
 create mode 100644 include/linux/of_display_timings.h

diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt
new file mode 100644
index 0000000..45e39bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timings.txt
@@ -0,0 +1,222 @@
+display-timings bindings
+=========
+
+display-timings-node
+------------
+
+required properties:
+ - none
+
+optional properties:
+ - default-timing: the default timing value
+
+timings-subnode
+---------------
+
+required properties:
+ - hactive, vactive: Display resolution
+ - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
+   in pixels
+   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
+   lines
+ - clock: displayclock in Hz
+
+optional properties:
+ - hsync-active-high (bool): Hsync pulse is active high
+ - vsync-active-high (bool): Vsync pulse is active high
+ - de-active-high (bool): Data-Enable pulse is active high
+ - pixelclk-inverted (bool): pixelclock is inverted
+ - interlaced (bool)
+ - doublescan (bool)
+
+There are different ways of describing the capabilities of a display. The devicetree
+representation corresponds to the one commonly found in datasheets for displays.
+If a display supports multiple signal timings, the default-timing can be specified.
+
+The parameters are defined as
+
+struct signal_timing
+=========+
+  +----------+---------------------------------------------+----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |vback_porch                 |          |       |
+  |          |                ↓                            |          |       |
+  +----------###############################################----------+-------+
+  |          #                ↑                            #          |       |
+  |          #                |                            #          |       |
+  |  hback   #                |                            #  hfront  | hsync |
+  |   porch  #                |       hactive              #  porch   |  len  |
+  |<-------->#<---------------+--------------------------->#<-------->|<----->|
+  |          #                |                            #          |       |
+  |          #                |vactive                     #          |       |
+  |          #                |                            #          |       |
+  |          #                ↓                            #          |       |
+  +----------###############################################----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |vfront_porch                |          |       |
+  |          |                ↓                            |          |       |
+  +----------+---------------------------------------------+----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |vsync_len                   |          |       |
+  |          |                ↓                            |          |       |
+  +----------+---------------------------------------------+----------+-------+
+
+
+Example:
+
+	display-timings {
+		default-timing = <&timing0>;
+		timing0: 1920p24 {
+			/* 1920x1080p24 */
+			clock = <52000000>;
+			hactive = <1920>;
+			vactive = <1080>;
+			hfront-porch = <25>;
+			hback-porch = <25>;
+			hsync-len = <25>;
+			vback-porch = <2>;
+			vfront-porch = <2>;
+			vsync-len = <2>;
+			hsync-active-high;
+		};
+	};
+
+Every property also supports the use of ranges, so the commonly used datasheet
+description with <min typ max>-tuples can be used.
+
+Example:
+
+	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.
+
+Supported modes
+=======+
+The generic videomode read in by the driver can be converted to the following
+modes with the following parameters
+
+struct fb_videomode
+=========+
+  +----------+---------------------------------------------+----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |upper_margin                |          |       |
+  |          |                ↓                            |          |       |
+  +----------###############################################----------+-------+
+  |          #                ↑                            #          |       |
+  |          #                |                            #          |       |
+  |          #                |                            #          |       |
+  |          #                |                            #          |       |
+  |   left   #                |                            #  right   | hsync |
+  |  margin  #                |       xres                 #  margin  |  len  |
+  |<-------->#<---------------+--------------------------->#<-------->|<----->|
+  |          #                |                            #          |       |
+  |          #                |                            #          |       |
+  |          #                |                            #          |       |
+  |          #                |yres                        #          |       |
+  |          #                |                            #          |       |
+  |          #                |                            #          |       |
+  |          #                |                            #          |       |
+  |          #                |                            #          |       |
+  |          #                |                            #          |       |
+  |          #                |                            #          |       |
+  |          #                |                            #          |       |
+  |          #                |                            #          |       |
+  |          #                ↓                            #          |       |
+  +----------###############################################----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |lower_margin                |          |       |
+  |          |                ↓                            |          |       |
+  +----------+---------------------------------------------+----------+-------+
+  |          |                ↑                            |          |       |
+  |          |                |vsync_len                   |          |       |
+  |          |                ↓                            |          |       |
+  +----------+---------------------------------------------+----------+-------+
+
+clock in nanoseconds
+
+struct drm_display_mode
+===========+
+  +----------+---------------------------------------------+----------+-------+
+  |          |                                             |          |       |  ↑
+  |          |                                             |          |       |  |
+  |          |                                             |          |       |  |
+  +----------###############################################----------+-------+  |
+  |          #   ↑         ↑          ↑                    #          |       |  |
+  |          #   |         |          |                    #          |       |  |
+  |          #   |         |          |                    #          |       |  |
+  |          #   |         |          |                    #          |       |  |
+  |          #   |         |          |                    #          |       |  |
+  |          #   |         |          |       hdisplay     #          |       |  |
+  |          #<--+--------------------+------------------->#          |       |  |
+  |          #   |         |          |                    #          |       |  |
+  |          #   |vsync_start         |                    #          |       |  |
+  |          #   |         |          |                    #          |       |  |
+  |          #   |         |vsync_end |                    #          |       |  |
+  |          #   |         |          |vdisplay            #          |       |  |
+  |          #   |         |          |                    #          |       |  |
+  |          #   |         |          |                    #          |       |  |
+  |          #   |         |          |                    #          |       |  | vtotal
+  |          #   |         |          |                    #          |       |  |
+  |          #   |         |          |     hsync_start    #          |       |  |
+  |          #<--+---------+----------+------------------------------>|       |  |
+  |          #   |         |          |                    #          |       |  |
+  |          #   |         |          |     hsync_end      #          |       |  |
+  |          #<--+---------+----------+-------------------------------------->|  |
+  |          #   |         |          ↓                    #          |       |  |
+  +----------####|#########|################################----------+-------+  |
+  |          |   |         |                               |          |       |  |
+  |          |   |         |                               |          |       |  |
+  |          |   ↓         |                               |          |       |  |
+  +----------+-------------+-------------------------------+----------+-------+  |
+  |          |             |                               |          |       |  |
+  |          |             |                               |          |       |  |
+  |          |             ↓                               |          |       |  ↓
+  +----------+---------------------------------------------+----------+-------+
+                                   htotal
+   <------------------------------------------------------------------------->
+
+clock in kilohertz
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index dfba3e6..646deb0 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -83,4 +83,9 @@ config OF_MTD
 	depends on MTD
 	def_bool y
 
+config OF_DISPLAY_TIMINGS
+	def_bool y
+	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..e47bc63
--- /dev/null
+++ b/drivers/of/of_display_timings.c
@@ -0,0 +1,183 @@
+/*
+ * 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>
+
+/* every signal_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);
+	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;
+}
+
+struct signal_timing *of_get_display_timing(struct device_node *np)
+{
+	struct signal_timing *st;
+	int ret = 0;
+
+	st = kzalloc(sizeof(*st), GFP_KERNEL);
+
+	if (!st) {
+		pr_err("%s: could not allocate signal_timing struct\n", __func__);
+		return NULL;
+	}
+
+	ret |= parse_property(np, "hback-porch", &st->hback_porch);
+	ret |= parse_property(np, "hfront-porch", &st->hfront_porch);
+	ret |= parse_property(np, "hactive", &st->hactive);
+	ret |= parse_property(np, "hsync-len", &st->hsync_len);
+	ret |= parse_property(np, "vback-porch", &st->vback_porch);
+	ret |= parse_property(np, "vfront-porch", &st->vfront_porch);
+	ret |= parse_property(np, "vactive", &st->vactive);
+	ret |= parse_property(np, "vsync-len", &st->vsync_len);
+	ret |= parse_property(np, "clock", &st->pixelclock);
+
+	st->vsync_pol_active_high = of_property_read_bool(np, "vsync-active-high");
+	st->hsync_pol_active_high = of_property_read_bool(np, "hsync-active-high");
+	st->de_pol_active_high = of_property_read_bool(np, "de-active-high");
+	st->pixelclk_pol_inverted = of_property_read_bool(np, "pixelclk-inverted");
+	st->interlaced = of_property_read_bool(np, "interlaced");
+	st->doublescan = of_property_read_bool(np, "doublescan");
+
+	if (ret) {
+		pr_err("%s: error reading timing properties\n", __func__);
+		return NULL;
+	}
+
+	return st;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timing);
+
+struct display_timings *of_get_display_timing_list(struct device_node *np)
+{
+	struct device_node *timings_np;
+	struct device_node *entry;
+	struct display_timings *disp;
+	char *default_timing;
+
+	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, "default-timing", 0);
+
+	if (!entry) {
+		pr_info("%s: no default-timing specified\n", __func__);
+		entry = of_find_node_by_name(np, "timing");
+	}
+
+	if (!entry) {
+		pr_info("%s: no timing specifications given\n", __func__);
+		return disp;
+	}
+
+	pr_info("%s: using %s as default timing\n", __func__, entry->name);
+
+	default_timing = (char *)entry->full_name;
+
+	disp->num_timings = 0;
+
+	for_each_child_of_node(timings_np, entry) {
+		disp->num_timings++;
+	}
+
+	disp->timings = kzalloc(sizeof(struct signal_timing *)*disp->num_timings,
+				GFP_KERNEL);
+
+	disp->num_timings = 0;
+
+	for_each_child_of_node(timings_np, entry) {
+		struct signal_timing *st;
+
+		st = of_get_display_timing(entry);
+
+		if (!st)
+			continue;
+
+		if (strcmp(default_timing, entry->full_name) = 0)
+			disp->default_timing = disp->num_timings;
+
+		disp->timings[disp->num_timings] = st;
+		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->default_timing + 1);
+	else
+		pr_info("%s: no timings specified\n", __func__);
+
+	return disp;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timing_list);
+
+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;
+
+	default_np = of_parse_phandle(np, "default-timing", 0);
+
+	if (default_np)
+		return 0;
+
+	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..1ad719e
--- /dev/null
+++ b/include/linux/of_display_timings.h
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * description of display timings
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_DISPLAY_TIMINGS_H
+#define __LINUX_OF_DISPLAY_TIMINGS_H
+
+#define OF_DEFAULT_TIMING -1
+
+struct display_timings {
+	unsigned int num_timings;
+	unsigned int default_timing;
+
+	struct signal_timing **timings;
+};
+
+struct timing_entry {
+	u32 min;
+	u32 typ;
+	u32 max;
+};
+
+struct signal_timing {
+	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;
+
+	bool vsync_pol_active_high;
+	bool hsync_pol_active_high;
+	bool de_pol_active_high;
+	bool pixelclk_pol_inverted;
+	bool interlaced;
+	bool doublescan;
+};
+
+struct display_timings *of_get_display_timing_list(struct device_node *np);
+struct signal_timing *of_get_display_timing(struct device_node *np);
+int of_display_timings_exists(struct device_node *np);
+
+/* placeholder function until ranges are really needed */
+static inline u32 signal_timing_get_value(struct timing_entry *te, int index)
+{
+	return te->typ;
+}
+
+static inline void timings_release(struct display_timings *disp)
+{
+	int i;
+
+	for (i = 0; i < disp->num_timings; i++)
+		kfree(disp->timings[i]);
+}
+
+static inline void display_timings_release(struct display_timings *disp)
+{
+	timings_release(disp);
+	kfree(disp->timings);
+}
+
+static inline struct signal_timing *display_timings_get(struct display_timings *disp,
+							 int index)
+{
+	struct signal_timing *st;
+
+	if (disp->num_timings > index) {
+		st = disp->timings[index];
+		return st;
+	}
+	else
+		return NULL;
+}
+
+#endif
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 0/2 v6] of: add display helper
From: Steffen Trumtrar @ 2012-10-04 17:59 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, linux-media, Tomi Valkeinen

Hi!

In accordance with Stepehn Warren, I downsized the binding.
Now, just the display-timing is described, as I think, it is way easier to agree
on those and have a complete binding.

Regards,
Steffen

Steffen Trumtrar (2):
  of: add helper to parse display timings
  of: add generic videomode description

 .../devicetree/bindings/video/display-timings.txt  |  222 ++++++++++++++++++++
 drivers/of/Kconfig                                 |   10 +
 drivers/of/Makefile                                |    2 +
 drivers/of/of_display_timings.c                    |  183 ++++++++++++++++
 drivers/of/of_videomode.c                          |  212 +++++++++++++++++++
 include/linux/of_display_timings.h                 |   85 ++++++++
 include/linux/of_videomode.h                       |   41 ++++
 7 files changed, 755 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
 create mode 100644 drivers/of/of_display_timings.c
 create mode 100644 drivers/of/of_videomode.c
 create mode 100644 include/linux/of_display_timings.h
 create mode 100644 include/linux/of_videomode.h

-- 
1.7.10.4


^ permalink raw reply

* omap DSS cmdline resolution not working for HDMI?
From: Tony Lindgren @ 2012-10-04 17:56 UTC (permalink / raw)
  To: linux-omap; +Cc: linux-fbdev, Tomi Valkeinen

Hi,

FYI, looks like for some reason DSS command line is not
working for HDMI while it works for DSS. On my panda es
I'm trying to set my motorola lapdock resolution from
cmdline with:

omapdss.def_disp=hdmi omapfb.mode=hdmi:1366x768@60

But it does not seem to do anything and resolution is
VGA. If I change the cable to DVI port this works:

omapdss.def_disp=dvi omapfb.mode=dvi:1366x768@60

Any ideas? This is with current linux next.

I can change the HDMI resolution OK from userspace with:

echo "1" > /sys/devices/platform/omapdss/display1/enabled
echo "0" > /sys/devices/platform/omapdss/overlay0/enabled
echo "tv" > /sys/devices/platform/omapdss/overlay0/manager
echo "1" > /sys/devices/platform/omapdss/overlay0/enabled
echo "85500,1366/70/213/143,768/3/24/3" > /sys/devices/platform/omapdss/display1/timings

The reason to use HDMI instead of DVI here is that HDMI
also has the speakers on the lapdock ;)

Then I'm able to switch between HDMI panel and DVI panel
just fine using overlay0. I don't know if getting both
HDMI and DVI to work the same time using overlay1 is
supposed to work, but trying use overlay1 produces the
following:

echo "1" > /sys/devices/platform/omapdss/display0/enabled
echo "0" > /sys/devices/platform/omapdss/overlay1/enabled
echo "lcd2" > /sys/devices/platform/omapdss/overlay1/manager
echo "1" > /sys/devices/platform/omapdss/overlay1/enabled
echo "170666,1920/336/128/208,1200/38/1/3" > /sys/devices/platform/omapdss/display0/timings

[  816.446044] omapdss DPI: Could not find exact pixel clock. Requested 23500 kHz, got 23630 kHz
[  881.639221] omapdss APPLY: timeout in wait_pending_extra_info_updates
[  958.946594] ------------[ cut here ]------------
[  958.953277] WARNING: at drivers/bus/omap_l3_noc.c:97 l3_interrupt_handler+0xc0/0x184()
[  958.965576] L3 standard error: TARGET:DMM2 at address 0x0
...

Regards,

Tony

^ permalink raw reply

* Re: [PATCH 2/2] drivers/video/exynos/exynos_mipi_dsi.c: fix error return code
From: Donghwa Lee @ 2012-10-04  7:42 UTC (permalink / raw)
  To: Peter Senna Tschudin
  Cc: Inki Dae, kernel-janitors, Kyungmin Park,
	Florian Tobias Schandinat, linux-fbdev, linux-kernel
In-Reply-To: <1349268044-32616-2-git-send-email-peter.senna@gmail.com>


On Wed, Oct 03, 2012 at 21:40, Peter Senna Tschudin wrote

> From: Peter Senna Tschudin <peter.senna@gmail.com>
> 
> Convert a nonnegative error return code to a negative one, as returned
> elsewhere in the function.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> (
> if@p1 (\(ret < 0\|ret != 0\))
>  { ... return ret; }
> |
> ret@p1 = 0
> )
> ... when != ret = e1
>     when != &ret
> *if(...)
> {
>   ... when != ret = e2
>       when forall
>  return ret;
> }
> // </smpl>
> 


I agree with you. Other codes is already used negative error return codes.

> 
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> 
> ---
>  drivers/video/exynos/exynos_mipi_dsi.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
> index c4f25de..07d70a3 100644
> --- a/drivers/video/exynos/exynos_mipi_dsi.c
> +++ b/drivers/video/exynos/exynos_mipi_dsi.c
> @@ -375,6 +375,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
>  	dsim->clock = clk_get(&pdev->dev, "dsim0");
>  	if (IS_ERR(dsim->clock)) {
>  		dev_err(&pdev->dev, "failed to get dsim clock source\n");
> +		ret = -ENODEV;
>  		goto err_clock_get;
>  	}
>  
> @@ -383,6 +384,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res) {
>  		dev_err(&pdev->dev, "failed to get io memory region\n");
> +		ret = -ENODEV;
>  		goto err_platform_get;
>  	}
>  
> @@ -407,6 +409,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
>  	dsim_ddi = exynos_mipi_dsi_bind_lcd_ddi(dsim, dsim_pd->lcd_panel_name);
>  	if (!dsim_ddi) {
>  		dev_err(&pdev->dev, "mipi_dsim_ddi object not found.\n");
> +		ret = -EINVAL;
>  		goto err_bind;
>  	}
>  
> 
> 



^ permalink raw reply

* [PATCH] video: exynos_dp: use clk_prepare_enable and clk_disable_unprepare
From: Jingoo Han @ 2012-10-04  6:45 UTC (permalink / raw)
  To: 'Florian Tobias Schandinat'
  Cc: linux-fbdev, linux-samsung-soc, 'Thomas Abraham',
	'Jingoo Han'

Convert clk_enable/clk_disable to clk_prepare_enable/clk_disable_unprepare
calls as required by common clock framework.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/exynos/exynos_dp_core.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index cdc1398..d55470e 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -885,7 +885,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 		return PTR_ERR(dp->clock);
 	}
 
-	clk_enable(dp->clock);
+	clk_prepare_enable(dp->clock);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
@@ -956,7 +956,7 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
 	if (pdata && pdata->phy_exit)
 		pdata->phy_exit();
 
-	clk_disable(dp->clock);
+	clk_disable_unprepare(dp->clock);
 
 	return 0;
 }
@@ -971,7 +971,7 @@ static int exynos_dp_suspend(struct device *dev)
 	if (pdata && pdata->phy_exit)
 		pdata->phy_exit();
 
-	clk_disable(dp->clock);
+	clk_disable_unprepare(dp->clock);
 
 	return 0;
 }
@@ -985,7 +985,7 @@ static int exynos_dp_resume(struct device *dev)
 	if (pdata && pdata->phy_init)
 		pdata->phy_init();
 
-	clk_enable(dp->clock);
+	clk_prepare_enable(dp->clock);
 
 	exynos_dp_init_dp(dp);
 
-- 
1.7.1



^ permalink raw reply related

* Re: [PATCH] video: s3c-fb: use clk_prepare_enable and clk_disable_unprepare
From: Jingoo Han @ 2012-10-04  6:18 UTC (permalink / raw)
  To: 'Thomas Abraham', linux-fbdev
  Cc: FlorianSchandinat, kgene.kim, linux-samsung-soc,
	'Jingoo Han'
In-Reply-To: <1349222260-3248-1-git-send-email-thomas.abraham@linaro.org>

On Wednesday, October 03, 2012 8:58 AM Thomas Abraham wrote
> 
> Convert clk_enable/clk_disable to clk_prepare_enable/clk_disable_unprepare
> calls as required by common clock framework.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>


It looks good. Also, I have tested this patch with Exynos4210.

Acked-by: Jingoo Han <jg1.han@samsung.com>


Best regards,
Jingoo Han


> ---
>  drivers/video/s3c-fb.c |   28 ++++++++++++++--------------
>  1 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 52b744f..2ed7b63 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -1404,7 +1404,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
>  		return PTR_ERR(sfb->bus_clk);
>  	}
> 
> -	clk_enable(sfb->bus_clk);
> +	clk_prepare_enable(sfb->bus_clk);
> 
>  	if (!sfb->variant.has_clksel) {
>  		sfb->lcd_clk = devm_clk_get(dev, "sclk_fimd");
> @@ -1414,7 +1414,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
>  			goto err_bus_clk;
>  		}
> 
> -		clk_enable(sfb->lcd_clk);
> +		clk_prepare_enable(sfb->lcd_clk);
>  	}
> 
>  	pm_runtime_enable(sfb->dev);
> @@ -1504,10 +1504,10 @@ err_lcd_clk:
>  	pm_runtime_disable(sfb->dev);
> 
>  	if (!sfb->variant.has_clksel)
> -		clk_disable(sfb->lcd_clk);
> +		clk_disable_unprepare(sfb->lcd_clk);
> 
>  err_bus_clk:
> -	clk_disable(sfb->bus_clk);
> +	clk_disable_unprepare(sfb->bus_clk);
> 
>  	return ret;
>  }
> @@ -1531,9 +1531,9 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
>  			s3c_fb_release_win(sfb, sfb->windows[win]);
> 
>  	if (!sfb->variant.has_clksel)
> -		clk_disable(sfb->lcd_clk);
> +		clk_disable_unprepare(sfb->lcd_clk);
> 
> -	clk_disable(sfb->bus_clk);
> +	clk_disable_unprepare(sfb->bus_clk);
> 
>  	pm_runtime_put_sync(sfb->dev);
>  	pm_runtime_disable(sfb->dev);
> @@ -1561,9 +1561,9 @@ static int s3c_fb_suspend(struct device *dev)
>  	}
> 
>  	if (!sfb->variant.has_clksel)
> -		clk_disable(sfb->lcd_clk);
> +		clk_disable_unprepare(sfb->lcd_clk);
> 
> -	clk_disable(sfb->bus_clk);
> +	clk_disable_unprepare(sfb->bus_clk);
> 
>  	pm_runtime_put_sync(sfb->dev);
> 
> @@ -1581,10 +1581,10 @@ static int s3c_fb_resume(struct device *dev)
> 
>  	pm_runtime_get_sync(sfb->dev);
> 
> -	clk_enable(sfb->bus_clk);
> +	clk_prepare_enable(sfb->bus_clk);
> 
>  	if (!sfb->variant.has_clksel)
> -		clk_enable(sfb->lcd_clk);
> +		clk_prepare_enable(sfb->lcd_clk);
> 
>  	/* setup gpio and output polarity controls */
>  	pd->setup_gpio();
> @@ -1640,9 +1640,9 @@ static int s3c_fb_runtime_suspend(struct device *dev)
>  	struct s3c_fb *sfb = platform_get_drvdata(pdev);
> 
>  	if (!sfb->variant.has_clksel)
> -		clk_disable(sfb->lcd_clk);
> +		clk_disable_unprepare(sfb->lcd_clk);
> 
> -	clk_disable(sfb->bus_clk);
> +	clk_disable_unprepare(sfb->bus_clk);
> 
>  	return 0;
>  }
> @@ -1653,10 +1653,10 @@ static int s3c_fb_runtime_resume(struct device *dev)
>  	struct s3c_fb *sfb = platform_get_drvdata(pdev);
>  	struct s3c_fb_platdata *pd = sfb->pdata;
> 
> -	clk_enable(sfb->bus_clk);
> +	clk_prepare_enable(sfb->bus_clk);
> 
>  	if (!sfb->variant.has_clksel)
> -		clk_enable(sfb->lcd_clk);
> +		clk_prepare_enable(sfb->lcd_clk);
> 
>  	/* setup gpio and output polarity controls */
>  	pd->setup_gpio();
> --
> 1.7.4.1


^ permalink raw reply

* Re: [PATCH V3] video: exynos_dp: Add device tree support to DP driver
From: Jingoo Han @ 2012-10-04  1:50 UTC (permalink / raw)
  To: 'Ajay kumar', linux-fbdev
  Cc: linux-samsung-soc, FlorianSchandinat, thomas.ab,
	'devicetree-discuss', 'Sylwester Nawrocki',
	'Tomasz Figa', 'Jingoo Han'
In-Reply-To: <CAEC9eQNFos4Sw57eubX-USEjKBnKf9ETeWCU_sT1vdX3MgN4dA@mail.gmail.com>

On Monday, October 01, 2012 2:40 PM Ajay kumar wrote
> 
> On Fri, Sep 28, 2012 at 5:41 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Thursday, September 27, 2012 10:45 PM Sylwester Nawrocki wrote
> >> On 09/24/2012 09:36 PM, Ajay Kumar wrote:

[...]

> >> > +Example:
> >> > +
> >> > +SOC specific portion:
> >> > +   dptx_phy: dptx_phy@0x10040720 {
> >> > +           compatible = "samsung,dp-phy";
> >> > +           samsung,dptx_phy_reg =<0x10040720>;
> >> > +           samsung,enable_bit =<1>;
> >> > +   };
> >> > +
> >> > +   display-port-controller {
> >> > +           compatible = "samsung,exynos5-dp";
> >> > +           reg =<0x145B0000 0x10000>;
> >> > +           interrupts =<10 3>;
> >> > +           interrupt-parent =<&combiner>;
> >> > +           dp_phy =<&dptx_phy>;
> >>
> >> Shouldn't it be "samsung,dp_phy" ?
> >
> > Do you mean this ? It is not working.
> > +               dp_phy = "samsung,dp_phy";
> I din't get this.
> What do I need to change here?

As Tomasz Figa mentioned, it means that

-+		dp_phy =<&dptx_phy>;
++		samsung,dp_phy =<&dptx_phy>;

It is because this is a Samsung-specific property.

Best regards,
Jingoo Han




^ permalink raw reply

* Re: [PATCH 0/6] OMAPDSS: remove cpu_is_* calls
From: Jingoo Han @ 2012-10-04  1:33 UTC (permalink / raw)
  To: 'Tomi Valkeinen'
  Cc: linux-omap, linux-fbdev, 'Archit Taneja',
	'Chandrabhanu Mahapatra',
	'Raphaël Assénat', 'Tony Lindgren',
	'Jingoo Han'
In-Reply-To: <1348828527-13309-1-git-send-email-tomi.valkeinen@ti.com>

On Friday, September 28, 2012 7:35 PM Tomi Valkeinen wrote
> 
> Hi,
> 
> This series adds an omapdss_version enum that is passed via platform data to
> omapdss driver. This version identifier is then used instead of cpu_is_*()
> calls.

Hi Tomi,

As you mentioned, cpu_is_*() is not preferable in driver.
Actually, I thought so, when I saw the OMAPDSS driver a few months ago.
Anyway, it looks good. :)

Best regards,
Jingoo Han

> 
> After these, omapdss no longer contains any plat/ or mach/ includes. omapfb,
> vrfb and vram still do, though.
> 
>  Tomi
> 
> Tomi Valkeinen (6):
>   OMAPDSS: add omapdss_version
>   OMAPDSS: use omapdss_version in dss_features.c
>   OMAPDSS: DISPC: use omapdss_version
>   OMAPDSS: DSS: use omapdss_version
>   OMAPDSS: HDMI: use omapdss_version
>   OMAPDSS: remove <plat/cpu.h> includes
> 
>  arch/arm/mach-omap2/display.c          |   38 +++++++++++++++++++
>  drivers/video/omap2/dss/core.c         |    2 +-
>  drivers/video/omap2/dss/dispc.c        |   41 +++++++++++++-------
>  drivers/video/omap2/dss/dss.c          |   39 +++++++++++++------
>  drivers/video/omap2/dss/dss_features.c |   64 ++++++++++++++++++++++----------
>  drivers/video/omap2/dss/dss_features.h |    5 ++-
>  drivers/video/omap2/dss/hdmi.c         |    3 +-
>  include/video/omapdss.h                |   14 +++++++
>  8 files changed, 157 insertions(+), 49 deletions(-)
> 
> --
> 1.7.9.5
> 
> --
> 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 v6 0/4] Runtime Interpreted Power Sequences
From: Stephen Warren @ 2012-10-03 15:30 UTC (permalink / raw)
  To: Alex Courbot
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Stephen Warren, linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Leela Krishna Amudala,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Brown,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
	Anton Vorontsov, Tomi Valkeinen,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Woodhouse,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <506BF62F.6040308-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

On 10/03/2012 02:24 AM, Alex Courbot wrote:
> On 09/14/2012 12:24 AM, Stephen Warren wrote:
>> On 09/13/2012 01:29 AM, Mark Brown wrote:
>>> On Thu, Sep 13, 2012 at 04:26:34PM +0900, Alex Courbot wrote:
>>>> On Thursday 13 September 2012 15:19:30 Mark Brown wrote:
>>>>>> On Thursday 13 September 2012 14:25:53 Mark Brown wrote:
>>>>>>> It would be sensible to make sure that the framework is done in
>>>>>>> such a
>>>>>>> way that drivers can use it - there will be drivers (perhaps not
>>>>>>> display
>>>>>>> ones) that have a known power sequence and which could benefit
>>>>>>> from the
>>>>>>> ability to use library code to implement it based on the user simply
>>>>>>> supplying named resources.
>>>
>>>>>> Not sure I understand what you mean, but things should be working
>>>>>> this way
>>>>>> already - regulators and PWMs are acquired by name using the standard
>>>>>> regulator_get() and pwm_get() functions. GPIOs do not, AFAIK, have
>>>>>> a way
>>>>>> to be referenced by name so their number is used instead.
>>>
>>>>> Right, but the sequencing for enabling them is currently open coded in
>>>>> each driver.
>>>
>>>> Mmm then I'm afraid I don't see what you wanted to say initially -
>>>> could you
>>>> elaborate?
>>>
>>> The driver knows the power sequence.  Having to type the same sequence
>>> into the DT or platform data for each board using the device wouuld be
>>> retarded so we need the drivers to be able to give the sequence to the
>>> library if they're going to be able to reuse it (which is a lot of what
>>> Tomi is talking about).
>>
>> I believe that's trivial to implement. The relevant function is:
>>
>> struct power_seq_set *devm_power_seq_set_build(struct device *dev,
>>            struct platform_power_seq_set *pseq);
>>
>> It's up to the driver whether pseq comes from platform data or is
>> hard-coded into the driver (or not provided at all, for the DT case).
>> So, the only change needed to convert a "hard-coded" driver to this API
>> is to convert the current custom data structure (or code) that describes
>> the sequence into a struct platform_power_seq_set.
> 
> If we go this way (which looks good IMO!), then maybe we should abandon
> that "platform" denomination and merge platform_power_seq* structures
> with the currently private power_seq*, and also replace the "building"
> step with a resources acquisition one. Calling these structures
> "platform" implies they are for platform data while they can be used to
> perform more flexible things as Mark mentioned.

That all seems reasonable.

> Also making the resolved
> resource visible would allow drivers to "patch" generic sequences with
> the proper GPIO numbers at runtime.

That doesn't sound like a great idea to me, but we can simply avoid
doing this even though it's technically possible.

> We would also avoid a few memory
> copies and both design and usage would be simplified, at the cost of
> having more things exposed. How does that sound?

Sounds fine to me at least.

^ permalink raw reply

* [PATCH 2/2] drivers/video/exynos/exynos_mipi_dsi.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-03 12:40 UTC (permalink / raw)
  To: Inki Dae
  Cc: kernel-janitors, Donghwa Lee, Kyungmin Park,
	Florian Tobias Schandinat, linux-fbdev, linux-kernel

From: Peter Senna Tschudin <peter.senna@gmail.com>

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
    when != &ret
*if(...)
{
  ... when != ret = e2
      when forall
 return ret;
}
// </smpl>


Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

---
 drivers/video/exynos/exynos_mipi_dsi.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index c4f25de..07d70a3 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -375,6 +375,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	dsim->clock = clk_get(&pdev->dev, "dsim0");
 	if (IS_ERR(dsim->clock)) {
 		dev_err(&pdev->dev, "failed to get dsim clock source\n");
+		ret = -ENODEV;
 		goto err_clock_get;
 	}
 
@@ -383,6 +384,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(&pdev->dev, "failed to get io memory region\n");
+		ret = -ENODEV;
 		goto err_platform_get;
 	}
 
@@ -407,6 +409,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	dsim_ddi = exynos_mipi_dsi_bind_lcd_ddi(dsim, dsim_pd->lcd_panel_name);
 	if (!dsim_ddi) {
 		dev_err(&pdev->dev, "mipi_dsim_ddi object not found.\n");
+		ret = -EINVAL;
 		goto err_bind;
 	}
 


^ permalink raw reply related

* [PATCH 1/2] drivers/video/savage/savagefb_driver.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-03 12:40 UTC (permalink / raw)
  To: Antonino Daplas
  Cc: kernel-janitors, Florian Tobias Schandinat, linux-fbdev,
	linux-kernel

From: Peter Senna Tschudin <peter.senna@gmail.com>

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
    when != &ret
*if(...)
{
  ... when != ret = e2
      when forall
 return ret;
}
// </smpl>


Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

---
 drivers/video/savage/savagefb_driver.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/savage/savagefb_driver.c b/drivers/video/savage/savagefb_driver.c
index 0d0f52c..f4f53b0 100644
--- a/drivers/video/savage/savagefb_driver.c
+++ b/drivers/video/savage/savagefb_driver.c
@@ -2266,8 +2266,10 @@ static int __devinit savagefb_probe(struct pci_dev* dev,
 	lpitch = info->var.xres_virtual*((info->var.bits_per_pixel + 7) >> 3);
 	info->var.yres_virtual = info->fix.smem_len/lpitch;
 
-	if (info->var.yres_virtual < info->var.yres)
+	if (info->var.yres_virtual < info->var.yres) {
+		err = -ENOMEM;
 		goto failed;
+	}
 
 #if defined(CONFIG_FB_SAVAGE_ACCEL)
 	/*


^ permalink raw reply related

* Re: [PATCH 1/2] of: add helper to parse display specs
From: Steffen Trumtrar @ 2012-10-03 11:06 UTC (permalink / raw)
  To: Stephen Warren
  Cc: devicetree-discuss, linux-fbdev, dri-devel, Tomi Valkeinen,
	Laurent Pinchart, kernel, linux-media
In-Reply-To: <5069CA74.7040409@wwwdotorg.org>

On Mon, Oct 01, 2012 at 10:53:08AM -0600, Stephen Warren wrote:
> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
> > Parse a display-node with timings and hardware-specs from devictree.
> 
> > diff --git a/Documentation/devicetree/bindings/video/display b/Documentation/devicetree/bindings/video/display
> > new file mode 100644
> > index 0000000..722766a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/display
> 
> This should be display.txt.
> 
Okay

> > @@ -0,0 +1,208 @@
> > +display bindings
> > +=========
> > +
> > +display-node
> > +------------
> 
> I'm not personally convinced about the direction this is going. While I
> think it's reasonable to define DT bindings for displays, and DT
> bindings for display modes, I'm not sure that it's reasonable to couple
> them together into a single binding.
> 
> I think creating a well-defined timing binding first will be much
> simpler than doing so within the context of a display binding; the
> scope/content of a general display binding seems much less well-defined
> to me at least, for reasons I mentioned before.
> 

Yes, you are right. I'm in the middle of moving things around a little.
It seems best, to have bindings only for the timings at the moment and
get people to agree on those and use them, instead of all the adhoc solutions
based on of_videomode v2.

Then, the of_get_display_timings and the conversion via videomode to fb_videomode
etc can be combined with Laurent Pincharts panel proposal.

> > +required properties:
> > + - none
> > +
> > +optional properties:
> > + - default-timing: the default timing value
> > + - width-mm, height-mm: Display dimensions in mm
> 
> > + - hsync-active-high (bool): Hsync pulse is active high
> > + - vsync-active-high (bool): Vsync pulse is active high
> 
> At least those two properties should exist in the display timing instead
> (or perhaps as well). There are certainly cases where different similar
> display modes are differentiated by hsync/vsync polarity more than
> anything else. This is probably more likely with analog display
> connectors than digital, but I see no reason why a DT binding for
> display timing shouldn't cover both.
> 

Yes. Will do.

> > + - de-active-high (bool): Data-Enable pulse is active high
> > + - pixelclk-inverted (bool): pixelclock is inverted
> 
> > + - pixel-per-clk
> 
> pixel-per-clk is probably something that should either be part of the
> timing definition, or something computed internally to the display
> driver based on rules for the signal type, rather than something
> represented in DT.
> 
> The above comment assumes this property is intended to represent DVI's
> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
> it's something to do with e.g. a single-data-rate vs. double-data-rate
> property of the underlying physical connection, that's most likely
> something that should be defined in a binding specific to e.g. LVDS,
> rather than something generic.
> 
> > + - link-width: number of channels (e.g. LVDS)
> > + - bpp: bits-per-pixel
> > +
> > +timings-subnode
> > +---------------
> > +
> > +required properties:
> > +subnodes that specify
> > + - hactive, vactive: Display resolution
> > + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
> > +   in pixels
> > +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
> > +   lines
> > + - clock: displayclock in Hz
> > +
> > +There are different ways of describing a display and its capabilities. The devicetree
> > +representation corresponds to the one commonly found in datasheets for displays.
> > +The description of the display and its timing is split in two parts: first the display
> > +properties like size in mm and (optionally) multiple subnodes with the supported timings.
> > +If a display supports multiple signal timings, the default-timing can be specified.
> > +
> > +Example:
> > +
> > +	display@0 {
> > +		width-mm = <800>;
> > +		height-mm = <480>;
> > +		default-timing = <&timing0>;
> > +		timings {
> > +			timing0: timing@0 {
> 
> If you're going to use a unit address ("@0") to ensure that node names
> are unique (which is not mandatory), then each node also needs a reg
> property with matching value, and #address-cells/#size-cells in the
> parent. Instead, you could name the nodes something unique based on the
> mode name to avoid this, e.g. 1080p24 { ... }.
> 

Ah, okay. Wasn't sure that was valid. I prefer to not use unit addresses.

 

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

^ permalink raw reply

* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-10-03  8:24 UTC (permalink / raw)
  To: Stephen Warren
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Stephen Warren, linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Leela Krishna Amudala,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Brown,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
	Anton Vorontsov, Tomi Valkeinen,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Woodhouse,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <5051FAC5.40501-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

On 09/14/2012 12:24 AM, Stephen Warren wrote:
> On 09/13/2012 01:29 AM, Mark Brown wrote:
>> On Thu, Sep 13, 2012 at 04:26:34PM +0900, Alex Courbot wrote:
>>> On Thursday 13 September 2012 15:19:30 Mark Brown wrote:
>>>>> On Thursday 13 September 2012 14:25:53 Mark Brown wrote:
>>>>>> It would be sensible to make sure that the framework is done in such a
>>>>>> way that drivers can use it - there will be drivers (perhaps not display
>>>>>> ones) that have a known power sequence and which could benefit from the
>>>>>> ability to use library code to implement it based on the user simply
>>>>>> supplying named resources.
>>
>>>>> Not sure I understand what you mean, but things should be working this way
>>>>> already - regulators and PWMs are acquired by name using the standard
>>>>> regulator_get() and pwm_get() functions. GPIOs do not, AFAIK, have a way
>>>>> to be referenced by name so their number is used instead.
>>
>>>> Right, but the sequencing for enabling them is currently open coded in
>>>> each driver.
>>
>>> Mmm then I'm afraid I don't see what you wanted to say initially - could you
>>> elaborate?
>>
>> The driver knows the power sequence.  Having to type the same sequence
>> into the DT or platform data for each board using the device wouuld be
>> retarded so we need the drivers to be able to give the sequence to the
>> library if they're going to be able to reuse it (which is a lot of what
>> Tomi is talking about).
>
> I believe that's trivial to implement. The relevant function is:
>
> struct power_seq_set *devm_power_seq_set_build(struct device *dev,
> 		   struct platform_power_seq_set *pseq);
>
> It's up to the driver whether pseq comes from platform data or is
> hard-coded into the driver (or not provided at all, for the DT case).
> So, the only change needed to convert a "hard-coded" driver to this API
> is to convert the current custom data structure (or code) that describes
> the sequence into a struct platform_power_seq_set.

If we go this way (which looks good IMO!), then maybe we should abandon 
that "platform" denomination and merge platform_power_seq* structures 
with the currently private power_seq*, and also replace the "building" 
step with a resources acquisition one. Calling these structures 
"platform" implies they are for platform data while they can be used to 
perform more flexible things as Mark mentioned. Also making the resolved 
resource visible would allow drivers to "patch" generic sequences with 
the proper GPIO numbers at runtime. We would also avoid a few memory 
copies and both design and usage would be simplified, at the cost of 
having more things exposed. How does that sound?

Alex.


^ permalink raw reply

* [PATCH] video: s3c-fb: use clk_prepare_enable and clk_disable_unprepare
From: Thomas Abraham @ 2012-10-02 23:57 UTC (permalink / raw)
  To: linux-fbdev; +Cc: FlorianSchandinat, jg1.han, kgene.kim, linux-samsung-soc

Convert clk_enable/clk_disable to clk_prepare_enable/clk_disable_unprepare
calls as required by common clock framework.

Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 drivers/video/s3c-fb.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 52b744f..2ed7b63 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -1404,7 +1404,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
 		return PTR_ERR(sfb->bus_clk);
 	}
 
-	clk_enable(sfb->bus_clk);
+	clk_prepare_enable(sfb->bus_clk);
 
 	if (!sfb->variant.has_clksel) {
 		sfb->lcd_clk = devm_clk_get(dev, "sclk_fimd");
@@ -1414,7 +1414,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
 			goto err_bus_clk;
 		}
 
-		clk_enable(sfb->lcd_clk);
+		clk_prepare_enable(sfb->lcd_clk);
 	}
 
 	pm_runtime_enable(sfb->dev);
@@ -1504,10 +1504,10 @@ err_lcd_clk:
 	pm_runtime_disable(sfb->dev);
 
 	if (!sfb->variant.has_clksel)
-		clk_disable(sfb->lcd_clk);
+		clk_disable_unprepare(sfb->lcd_clk);
 
 err_bus_clk:
-	clk_disable(sfb->bus_clk);
+	clk_disable_unprepare(sfb->bus_clk);
 
 	return ret;
 }
@@ -1531,9 +1531,9 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
 			s3c_fb_release_win(sfb, sfb->windows[win]);
 
 	if (!sfb->variant.has_clksel)
-		clk_disable(sfb->lcd_clk);
+		clk_disable_unprepare(sfb->lcd_clk);
 
-	clk_disable(sfb->bus_clk);
+	clk_disable_unprepare(sfb->bus_clk);
 
 	pm_runtime_put_sync(sfb->dev);
 	pm_runtime_disable(sfb->dev);
@@ -1561,9 +1561,9 @@ static int s3c_fb_suspend(struct device *dev)
 	}
 
 	if (!sfb->variant.has_clksel)
-		clk_disable(sfb->lcd_clk);
+		clk_disable_unprepare(sfb->lcd_clk);
 
-	clk_disable(sfb->bus_clk);
+	clk_disable_unprepare(sfb->bus_clk);
 
 	pm_runtime_put_sync(sfb->dev);
 
@@ -1581,10 +1581,10 @@ static int s3c_fb_resume(struct device *dev)
 
 	pm_runtime_get_sync(sfb->dev);
 
-	clk_enable(sfb->bus_clk);
+	clk_prepare_enable(sfb->bus_clk);
 
 	if (!sfb->variant.has_clksel)
-		clk_enable(sfb->lcd_clk);
+		clk_prepare_enable(sfb->lcd_clk);
 
 	/* setup gpio and output polarity controls */
 	pd->setup_gpio();
@@ -1640,9 +1640,9 @@ static int s3c_fb_runtime_suspend(struct device *dev)
 	struct s3c_fb *sfb = platform_get_drvdata(pdev);
 
 	if (!sfb->variant.has_clksel)
-		clk_disable(sfb->lcd_clk);
+		clk_disable_unprepare(sfb->lcd_clk);
 
-	clk_disable(sfb->bus_clk);
+	clk_disable_unprepare(sfb->bus_clk);
 
 	return 0;
 }
@@ -1653,10 +1653,10 @@ static int s3c_fb_runtime_resume(struct device *dev)
 	struct s3c_fb *sfb = platform_get_drvdata(pdev);
 	struct s3c_fb_platdata *pd = sfb->pdata;
 
-	clk_enable(sfb->bus_clk);
+	clk_prepare_enable(sfb->bus_clk);
 
 	if (!sfb->variant.has_clksel)
-		clk_enable(sfb->lcd_clk);
+		clk_prepare_enable(sfb->lcd_clk);
 
 	/* setup gpio and output polarity controls */
 	pd->setup_gpio();
-- 
1.7.4.1


^ permalink raw reply related

* Re: [PATCH 1/2] of: add helper to parse display specs
From: Mitch Bradley @ 2012-10-01 21:08 UTC (permalink / raw)
  To: Stephen Warren
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Hans Verkuil,
	Tomi Valkeinen, Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Steffen Trumtrar, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5069FC20.8060708-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

On 10/1/2012 10:25 AM, Stephen Warren wrote:
> On 10/01/2012 01:16 PM, Mitch Bradley wrote:
>> On 10/1/2012 6:53 AM, Stephen Warren wrote:
>>> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
>>>> Parse a display-node with timings and hardware-specs from devictree.
>>>
>>>> diff --git a/Documentation/devicetree/bindings/video/display b/Documentation/devicetree/bindings/video/display
>>>> new file mode 100644
>>>> index 0000000..722766a
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/video/display
>>>
>>> This should be display.txt.
>>>
>>>> @@ -0,0 +1,208 @@
>>>> +display bindings
>>>> +=========
>>>> +
>>>> +display-node
>>>> +------------
>>>
>>> I'm not personally convinced about the direction this is going. While I
>>> think it's reasonable to define DT bindings for displays, and DT
>>> bindings for display modes, I'm not sure that it's reasonable to couple
>>> them together into a single binding.
>>>
>>> I think creating a well-defined timing binding first will be much
>>> simpler than doing so within the context of a display binding; the
>>> scope/content of a general display binding seems much less well-defined
>>> to me at least, for reasons I mentioned before.
>>>
>>>> +required properties:
>>>> + - none
>>>> +
>>>> +optional properties:
>>>> + - default-timing: the default timing value
>>>> + - width-mm, height-mm: Display dimensions in mm
>>>
>>>> + - hsync-active-high (bool): Hsync pulse is active high
>>>> + - vsync-active-high (bool): Vsync pulse is active high
>>>
>>> At least those two properties should exist in the display timing instead
>>> (or perhaps as well). There are certainly cases where different similar
>>> display modes are differentiated by hsync/vsync polarity more than
>>> anything else. This is probably more likely with analog display
>>> connectors than digital, but I see no reason why a DT binding for
>>> display timing shouldn't cover both.
>>>
>>>> + - de-active-high (bool): Data-Enable pulse is active high
>>>> + - pixelclk-inverted (bool): pixelclock is inverted
>>>
>>>> + - pixel-per-clk
>>>
>>> pixel-per-clk is probably something that should either be part of the
>>> timing definition, or something computed internally to the display
>>> driver based on rules for the signal type, rather than something
>>> represented in DT.
>>>
>>> The above comment assumes this property is intended to represent DVI's
>>> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
>>> it's something to do with e.g. a single-data-rate vs. double-data-rate
>>> property of the underlying physical connection, that's most likely
>>> something that should be defined in a binding specific to e.g. LVDS,
>>> rather than something generic.
>>>
>>>> + - link-width: number of channels (e.g. LVDS)
>>>> + - bpp: bits-per-pixel
>>>> +
>>>> +timings-subnode
>>>> +---------------
>>>> +
>>>> +required properties:
>>>> +subnodes that specify
>>>> + - hactive, vactive: Display resolution
>>>> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
>>>> +   in pixels
>>>> +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
>>>> +   lines
>>>> + - clock: displayclock in Hz
>>>> +
>>>> +There are different ways of describing a display and its capabilities. The devicetree
>>>> +representation corresponds to the one commonly found in datasheets for displays.
>>>> +The description of the display and its timing is split in two parts: first the display
>>>> +properties like size in mm and (optionally) multiple subnodes with the supported timings.
>>>> +If a display supports multiple signal timings, the default-timing can be specified.
>>>> +
>>>> +Example:
>>>> +
>>>> +	display@0 {
>>>> +		width-mm = <800>;
>>>> +		height-mm = <480>;
>>>> +		default-timing = <&timing0>;
>>>> +		timings {
>>>> +			timing0: timing@0 {
>>>
>>> If you're going to use a unit address ("@0") to ensure that node names
>>> are unique (which is not mandatory), then each node also needs a reg
>>> property with matching value, and #address-cells/#size-cells in the
>>> parent. Instead, you could name the nodes something unique based on the
>>> mode name to avoid this, e.g. 1080p24 { ... }.
>>
>>
>> I'm concerned that numbered nodes are being misused as arrays.
>>
>> It's easy to make real arrays by including multiple cells in the value
>> of each timing parameter, and easy to choose a cell by saying the array
>> index instead of using the phandle.
> 
> In this case though, arrays don't work out so well in my opinion:
> 
> We want to describe a set of unrelated display modes that the display
> can handle. These are logically separate entities. I don't think
> combining the values that represent say 5 different modes into a single
> set of properties really makes sense here; a separate node or property
> per display mode really does make sense because they're separate objects.

That argument seems pretty dependent on how you choose to look at things.


> 
> Related, each display timing parameter (e.g. hsync length, position,
> ...) has a range, so min/typical/max values. These are already
> represented as a 3-cell property as I believe you're proposing.
> Combining that with a cell that represents n different modes in a single
> array seems like it'd end up with something rather hard to read, at
> least for humans even if it would be admittedly trivial for a CPU.


That argument is better.

> 

^ permalink raw reply

* Re: [PATCH 1/2] of: add helper to parse display specs
From: Stephen Warren @ 2012-10-01 20:25 UTC (permalink / raw)
  To: Mitch Bradley
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Hans Verkuil,
	Tomi Valkeinen, Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Steffen Trumtrar, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5069EC1C.2050506-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org>

On 10/01/2012 01:16 PM, Mitch Bradley wrote:
> On 10/1/2012 6:53 AM, Stephen Warren wrote:
>> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
>>> Parse a display-node with timings and hardware-specs from devictree.
>>
>>> diff --git a/Documentation/devicetree/bindings/video/display b/Documentation/devicetree/bindings/video/display
>>> new file mode 100644
>>> index 0000000..722766a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/video/display
>>
>> This should be display.txt.
>>
>>> @@ -0,0 +1,208 @@
>>> +display bindings
>>> +=========
>>> +
>>> +display-node
>>> +------------
>>
>> I'm not personally convinced about the direction this is going. While I
>> think it's reasonable to define DT bindings for displays, and DT
>> bindings for display modes, I'm not sure that it's reasonable to couple
>> them together into a single binding.
>>
>> I think creating a well-defined timing binding first will be much
>> simpler than doing so within the context of a display binding; the
>> scope/content of a general display binding seems much less well-defined
>> to me at least, for reasons I mentioned before.
>>
>>> +required properties:
>>> + - none
>>> +
>>> +optional properties:
>>> + - default-timing: the default timing value
>>> + - width-mm, height-mm: Display dimensions in mm
>>
>>> + - hsync-active-high (bool): Hsync pulse is active high
>>> + - vsync-active-high (bool): Vsync pulse is active high
>>
>> At least those two properties should exist in the display timing instead
>> (or perhaps as well). There are certainly cases where different similar
>> display modes are differentiated by hsync/vsync polarity more than
>> anything else. This is probably more likely with analog display
>> connectors than digital, but I see no reason why a DT binding for
>> display timing shouldn't cover both.
>>
>>> + - de-active-high (bool): Data-Enable pulse is active high
>>> + - pixelclk-inverted (bool): pixelclock is inverted
>>
>>> + - pixel-per-clk
>>
>> pixel-per-clk is probably something that should either be part of the
>> timing definition, or something computed internally to the display
>> driver based on rules for the signal type, rather than something
>> represented in DT.
>>
>> The above comment assumes this property is intended to represent DVI's
>> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
>> it's something to do with e.g. a single-data-rate vs. double-data-rate
>> property of the underlying physical connection, that's most likely
>> something that should be defined in a binding specific to e.g. LVDS,
>> rather than something generic.
>>
>>> + - link-width: number of channels (e.g. LVDS)
>>> + - bpp: bits-per-pixel
>>> +
>>> +timings-subnode
>>> +---------------
>>> +
>>> +required properties:
>>> +subnodes that specify
>>> + - hactive, vactive: Display resolution
>>> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
>>> +   in pixels
>>> +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
>>> +   lines
>>> + - clock: displayclock in Hz
>>> +
>>> +There are different ways of describing a display and its capabilities. The devicetree
>>> +representation corresponds to the one commonly found in datasheets for displays.
>>> +The description of the display and its timing is split in two parts: first the display
>>> +properties like size in mm and (optionally) multiple subnodes with the supported timings.
>>> +If a display supports multiple signal timings, the default-timing can be specified.
>>> +
>>> +Example:
>>> +
>>> +	display@0 {
>>> +		width-mm = <800>;
>>> +		height-mm = <480>;
>>> +		default-timing = <&timing0>;
>>> +		timings {
>>> +			timing0: timing@0 {
>>
>> If you're going to use a unit address ("@0") to ensure that node names
>> are unique (which is not mandatory), then each node also needs a reg
>> property with matching value, and #address-cells/#size-cells in the
>> parent. Instead, you could name the nodes something unique based on the
>> mode name to avoid this, e.g. 1080p24 { ... }.
> 
> 
> I'm concerned that numbered nodes are being misused as arrays.
> 
> It's easy to make real arrays by including multiple cells in the value
> of each timing parameter, and easy to choose a cell by saying the array
> index instead of using the phandle.

In this case though, arrays don't work out so well in my opinion:

We want to describe a set of unrelated display modes that the display
can handle. These are logically separate entities. I don't think
combining the values that represent say 5 different modes into a single
set of properties really makes sense here; a separate node or property
per display mode really does make sense because they're separate objects.

Related, each display timing parameter (e.g. hsync length, position,
...) has a range, so min/typical/max values. These are already
represented as a 3-cell property as I believe you're proposing.
Combining that with a cell that represents n different modes in a single
array seems like it'd end up with something rather hard to read, at
least for humans even if it would be admittedly trivial for a CPU.

^ permalink raw reply

* [GIT PULL] OMAP DSS for v3.7
From: Tomi Valkeinen @ 2012-10-01 19:45 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: linux-omap, linux-fbdev

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

Hi Florian,

Here are omapdss changes for 3.7 merge window.

There's something funny with the diff stat, though. I can see a few
changes there that I have not made, for example to
drivers/video/console/fbcon.c. I believe they come from my "Merge branch
'fbdev-for-linus' of git://github.com/schandinat/linux-2.6", although I
don't know why they show up in the diff stat.

Merge with v3.6 goes without conflicts, but there will be some conflicts
with OMAP platform stuff, at least according to linux-next reports.
Those conflicts are trivial, though.

 Tomi

The following changes since commit 4cbe5a555fa58a79b6ecbb6c531b8bab0650778d:

  Linux 3.6-rc4 (2012-09-01 10:39:58 -0700)

are available in the git repository at:

  git://gitorious.org/linux-omap-dss2/linux.git tags/omapdss-for-3.7

for you to fetch changes up to 13b1ba7de8d0ecc42e4f9c002d5b0c1a48f05e58:

  OMAPDSS: add missing include for string.h (2012-09-28 10:03:03 +0300)

----------------------------------------------------------------
Omapdss driver changes for the 3.7 merge window.

Notable changes:

* Basic writeback support for DISPC level. Writeback is not yet usable, though,
  as we need higher level code to actually expose the writeback feature to
  userspace.
* Rewriting the omapdss output drivers. We're trying to remove the hard links
  between the omapdss and the panels, and this rewrite work moves us closer to
  that goal.
* Cleanup and restructuring patches that have been made while working on device
  tree support for omapdss. Device tree support is still some way ahead, but
  these patches are good cleanups in themselves.
* Basic OMAP5 DSS support for DPI and DSI outputs.
* Workaround for the problem that GFX overlay's fifo is too small for high
  resolution scenarios, causing underflows.
* Cleanups that remove dependencies to omap platform code.

----------------------------------------------------------------
Archit Taneja (70):
      OMAPDSS: APPLY: Constify timings argument in dss_mgr_set_timings
      OMAPDSS: DPI: Add locking for DPI interface
      OMAPDSS: Displays: Add locking in generic DPI panel driver
      OMAPDSS: DPI: Maintain our own timings field in driver data
      OMAPDSS: DPI displays: Take care of panel timings in the driver itself
      OMAPDSS: DSI: Maintain own copy of timings in driver data
      OMAPDSS: DSI: Add function to set panel size for command mode panels
      OMAPDSS: DSI: Update manager timings on a manual update
      OMAPDSS: HDMI: Use our own omap_video_timings field when setting interface timings
      OMAPDSS: HDMI: Add locking for hdmi interface set timing functions
      OMAPDSS: SDI: Create a function to set timings
      OMAPDSS: SDI: Maintain our own timings field in driver data
      OMAPDSS: VENC: Split VENC into interface and panel driver
      OMAPDSS: VENC: Maintain our own timings field in driver data
      OMAPDSS: RFBI: Remove partial update support
      OMAPDSS: RFBI: Add function to set panel size
      OMAPDSS: DSI: Maintain copy of pixel format in driver data
      OMAPDSS: RFBI: Maintain copy of pixel size in driver data
      OMAPDSS: RFBI: Maintain copy of number of data lines in driver data
      OMAPDSS: DPI: Maintain copy of number of data lines in driver data
      OMAPDSS: SDI: Maintain copy of data pairs in driver data
      OMAPDSS: DSI: Maintain copy of operation mode in driver data
      OMAPDSS: DSI: Rename dsi_videomode_data to dsi_videomode_timings
      OMAPDSS: DSI: Maintain copy of video mode timings in driver data
      OMAPDSS: RFBI: Maitain copy of rfbi timings in driver data
      OMAPDSS: VENC: Maintain copy of venc type in driver data
      OMAPDSS: VENC: Maintian copy of video output polarity info in private data
      OMAPFB: Clear framebuffers before they are registered
      OMAPDSS: Add basic omap5 features to dss and dispc
      OMAPDSS: DSI: Pass dsi platform device wherever possible
      OMAPDSS: APPLY: Remove omap_dss_device references in wait_for_go functions
      OMAPDSS: outputs: Create a new entity called outputs
      OMAPDSS: outputs: Create and register output instances
      OMAPDSS: output: Add set/unset device ops for omap_dss_output
      OMAPDSS: APPLY: Add manager set/unset output ops for omap_overlay_manager
      OMAPDSS: Remove manager->device references
      OMAP_VOUT: Remove manager->device references
      OMAPFB: remove manager->device references
      OMAPDRM: Remove manager->device references
      OMAPDSS: Create links between managers, outputs and devices
      OMAPDSS: DPI: Replace dssdev->manager with dssdev->output->manager references
      OMAPDSS: DSI: Remove dsi_pdev_map global struct
      OMAPDSS: DSI: Replace dssdev->manager with dssdev->output->manager references
      OMAPDSS: SDI: Replace dssdev->manager with dssdev->output->manager references
      OMAPDSS: RFBI: Replace dssdev->manager with dssdev->output->manager references
      OMAPDSS: VENC: Replace dssdev->manager with dssdev->output->manager references
      OMAPDSS: HDMI: Replace dssdev->manager with dssdev->output->manager references
      OMAPFB: Change dssdev->manager references
      OMAPDSS: MANAGER: Update display sysfs store
      OMAPDSS: OVERLAY/MANAGER: Get device via output
      OMAPDSS: APPLY: Remove omap_dss_device references from dss_ovl_enable/disable
      OMAPDSS: Remove old way of setting manager and device links
      OMAPDSS: DISPC: Constify omap_overlay_info in dispc_ovl_setup()
      OMAPDSS: DISPC: Simplify function names for setting pipeline input and output sizes
      OMAPDSS: DISPC: Pass overlay caps as a parameter to dispc plane functions
      OMAPDSS: OVERLAY: Add position and replication as overlay caps
      OMAPDSS: DISPC: Make dispc_ovl_setup call dispc_ovl_setup_common
      OMAPDSS: DISPC: Don't pass channel out when configuring overlays
      OMAPDSS: DIPSC: Relax scaling limitations when in memory to memory mode
      OMAPDSS: DISPC: Allow both upscaling and downscaling of chroma
      OMAPDSS: DISPC: Add writeback register offsets and dss features structs
      OMAPDSS: DISPC: Configure input and output sizes for writeback
      OMAPDSS: DISPC: Downscale chroma if plane is writeback
      OMAPDSS: DISPC: Don't set chroma resampling bit for writeback
      OMAPDSS: DISPC: Add function to set channel in for writeback
      OMAPDSS: DISPC: Configure overlay-like parameters in dispc_wb_setup
      OMAPDSS: DISPC: Configure writeback specific parameters in dispc_wb_setup()
      OMAPDSS: DISPC: Configure writeback FIFOs
      OMAPDSS: DISPC: Add manager like functions for writeback
      OMAPDSS: DISPC: Configure color conversion coefficients for writeback

Chandrabhanu Mahapatra (8):
      OMAPDSS: DISPC: Cleanup cpu_is_xxxx checks
      OMAPDSS: DSS: Remove redundant functions
      OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
      ARM: OMAP: Disable venc for OMAP4
      OMAPDSS: VENC: Remove cpu_is_xxxx checks
      OMAPDSS: DPI: Remove cpu_is_xxxx checks
      OMAPDSS: Correct DISPC_IRQ bit definitions for LCD3
      OMAPDSS: DISPC: Add predecimation limit for TILER based rotations

Grazvydas Ignotas (1):
      OMAPFB: fix framebuffer console colors

Jassi Brar (1):
      OMAPDSS: DISPC: Use msleep instead of blocking mdelay

Raphaël Assénat (1):
      OMAPDSS: Do not require a VDDS_DSI regulator on AM35xx

Ricardo Neri (2):
      OMAPDSS: HDMI: Disable PLL properly in case of error at power_on
      OMAPDSS: DISPC: Improvements to DIGIT sync signal selection

Tomi Valkeinen (53):
      Merge output work from Archit
      OMAPDSS: HDMI: fix initial HDMI enable
      Merge tag 'v3.6-rc4'
      OMAPDSS: HDMI: Move GPIO handling to HDMI driver
      OMAPDSS: HDMI: Add delay to wait for 5V power
      OMAP4: TWL: add vdda_hdmi_dac regulator supply
      OMAPDSS: HDMI: use vdda_hdmi_dac
      OMAPDSS: Add DSI fclk maximum to dss_features
      OMAPDSS: DSI: calculate dsi clock
      OMAP: 4430SDP: remove DSI clock config from board file
      OMAPDSS: fix use of dssdev->caps
      OMAPDSS: Taal: use devm_* functions
      OMAPFB1: remove unnecessary includes
      OMAPFB1: remove a non-used table
      OMAPDSS: remove unnecessary includes
      OMAPFB: clear framebuffers with CPU
      OMAPDSS: VRAM: Remove clearing with sDMA
      OMAPDSS: Taal: Reogranize for device tree
      OMAPDSS: TFP410: use devm_gpio_request_one
      OMAPDSS: split overlay sysfs code
      OMAPDSS: split manager sysfs code
      OMAPDSS: clean up dss_mgr_set_lcd_config
      OMAPDSS: clean up dss_mgr_set_timings
      Revert "OMAPDSS: APPLY: add fifo-merge support"
      Revert "OMAPDSS: APPLY: add fifo merge support funcs"
      OMAPDSS: Improve fifo management code
      OMAPDSS: Use WB fifo for GFX overlay
      OMAPDSS: fix set_timings
      OMAPDSS: fix dss_ovl_unset_manager
      Merge miscellaneous omapdss changes
      Merge branch 'fbdev-for-linus' of git://github.com/schandinat/linux-2.6
      OMAPDSS: omap_dss_register_device() doesn't take display index
      OMAPDSS: Add dss_get_default_display_name()
      OMAPDSS: register only one display device per output
      OMAPDSS: explicitely initialize dssdev->channel for new displays
      OMAPDSS: handle errors in dss_init_device
      OMAPDSS: cleanup dss_recheck_connections
      OMAPDSS: cleanup dss_recheck_connections further
      OMAPDSS: alloc dssdevs dynamically
      OMAPDSS: DSI: improve DSI clock calcs for DISPC
      OMAPDSS: move dss feats to the end of dss.c
      OMAPDSS: Add support for DPI source selection
      OMAPDSS: DSI: Add FEAT_DSI_PLL_SELFREQDCO
      OMAPDSS: DSI: Add FEAT_DSI_PLL_REFSEL
      OMAPDSS: DSI: Add new linebuffer size for OMAP5
      OMAPDSS: DSI: Add code to disable PHY DCC
      OMAPDSS: DSI: make OMAP2_DSS_DSI depend on ARCH_OMAP5
      Merge omapdss single-dssdev series
      Merge OMAP5 DSS changes to omapdss
      OMAPDSS: DSI: fix tlpx_half reg field length
      Merge branch 'archit/outputs-for-3.7'
      Merge branch 'archit/wb-dispc-for-3.7'
      OMAPDSS: add missing include for string.h

 arch/arm/mach-omap2/board-4430sdp.c                |   73 +-
 arch/arm/mach-omap2/board-omap4panda.c             |   27 +-
 arch/arm/mach-omap2/display.c                      |    1 -
 arch/arm/mach-omap2/twl-common.c                   |    6 +
 drivers/media/video/omap/omap_vout.c               |   75 +-
 drivers/staging/omapdrm/omap_drv.c                 |    5 +-
 drivers/video/auo_k190x.c                          |    2 -
 drivers/video/console/bitblit.c                    |    2 +-
 drivers/video/console/fbcon.c                      |    2 +-
 drivers/video/mb862xx/mb862xxfbdrv.c               |    2 +
 drivers/video/omap/hwa742.c                        |    1 -
 drivers/video/omap/lcd_ams_delta.c                 |    1 -
 drivers/video/omap/lcd_palmte.c                    |    1 -
 drivers/video/omap/omapfb_main.c                   |    9 -
 drivers/video/omap2/displays/panel-acx565akm.c     |   14 +-
 drivers/video/omap2/displays/panel-generic-dpi.c   |   76 +-
 .../omap2/displays/panel-lgphilips-lb035q02.c      |    3 +
 drivers/video/omap2/displays/panel-n8x0.c          |   31 +-
 .../omap2/displays/panel-nec-nl8048hl11-01b.c      |    3 +
 drivers/video/omap2/displays/panel-picodlp.c       |    4 +
 .../video/omap2/displays/panel-sharp-ls037v7dw01.c |    3 +
 drivers/video/omap2/displays/panel-taal.c          |  239 ++---
 drivers/video/omap2/displays/panel-tfp410.c        |   20 +-
 .../video/omap2/displays/panel-tpo-td043mtea1.c    |    7 +-
 drivers/video/omap2/dss/Kconfig                    |    2 +-
 drivers/video/omap2/dss/Makefile                   |    4 +-
 drivers/video/omap2/dss/apply.c                    |  330 ++-----
 drivers/video/omap2/dss/core.c                     |   91 +-
 drivers/video/omap2/dss/dispc.c                    | 1019 ++++++++++++++------
 drivers/video/omap2/dss/dispc.h                    |   37 +
 drivers/video/omap2/dss/display.c                  |  108 ++-
 drivers/video/omap2/dss/dpi.c                      |  181 +++-
 drivers/video/omap2/dss/dsi.c                      |  675 ++++++++++---
 drivers/video/omap2/dss/dss.c                      |  257 +++--
 drivers/video/omap2/dss/dss.h                      |   79 +-
 drivers/video/omap2/dss/dss_features.c             |  278 +++++-
 drivers/video/omap2/dss/dss_features.h             |    7 +
 drivers/video/omap2/dss/hdmi.c                     |  247 +++--
 drivers/video/omap2/dss/hdmi_panel.c               |   31 +-
 drivers/video/omap2/dss/manager-sysfs.c            |  512 ++++++++++
 drivers/video/omap2/dss/manager.c                  |  473 +--------
 drivers/video/omap2/dss/output.c                   |  148 +++
 drivers/video/omap2/dss/overlay-sysfs.c            |  456 +++++++++
 drivers/video/omap2/dss/overlay.c                  |  492 +---------
 drivers/video/omap2/dss/rfbi.c                     |  222 +++--
 drivers/video/omap2/dss/sdi.c                      |  136 ++-
 drivers/video/omap2/dss/venc.c                     |  337 +++----
 drivers/video/omap2/dss/venc_panel.c               |  251 +++++
 drivers/video/omap2/omapfb/omapfb-ioctl.c          |    7 +-
 drivers/video/omap2/omapfb/omapfb-main.c           |   34 +-
 drivers/video/omap2/omapfb/omapfb.h                |    5 +-
 drivers/video/omap2/vram.c                         |   56 --
 include/video/omapdss.h                            |  112 ++-
 53 files changed, 4592 insertions(+), 2602 deletions(-)
 create mode 100644 drivers/video/omap2/dss/manager-sysfs.c
 create mode 100644 drivers/video/omap2/dss/output.c
 create mode 100644 drivers/video/omap2/dss/overlay-sysfs.c
 create mode 100644 drivers/video/omap2/dss/venc_panel.c


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] of: add helper to parse display specs
From: Mitch Bradley @ 2012-10-01 19:16 UTC (permalink / raw)
  To: Stephen Warren
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Hans Verkuil,
	Tomi Valkeinen, Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Steffen Trumtrar, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5069CA74.7040409-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

On 10/1/2012 6:53 AM, Stephen Warren wrote:
> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
>> Parse a display-node with timings and hardware-specs from devictree.
> 
>> diff --git a/Documentation/devicetree/bindings/video/display b/Documentation/devicetree/bindings/video/display
>> new file mode 100644
>> index 0000000..722766a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/video/display
> 
> This should be display.txt.
> 
>> @@ -0,0 +1,208 @@
>> +display bindings
>> +=========
>> +
>> +display-node
>> +------------
> 
> I'm not personally convinced about the direction this is going. While I
> think it's reasonable to define DT bindings for displays, and DT
> bindings for display modes, I'm not sure that it's reasonable to couple
> them together into a single binding.
> 
> I think creating a well-defined timing binding first will be much
> simpler than doing so within the context of a display binding; the
> scope/content of a general display binding seems much less well-defined
> to me at least, for reasons I mentioned before.
> 
>> +required properties:
>> + - none
>> +
>> +optional properties:
>> + - default-timing: the default timing value
>> + - width-mm, height-mm: Display dimensions in mm
> 
>> + - hsync-active-high (bool): Hsync pulse is active high
>> + - vsync-active-high (bool): Vsync pulse is active high
> 
> At least those two properties should exist in the display timing instead
> (or perhaps as well). There are certainly cases where different similar
> display modes are differentiated by hsync/vsync polarity more than
> anything else. This is probably more likely with analog display
> connectors than digital, but I see no reason why a DT binding for
> display timing shouldn't cover both.
> 
>> + - de-active-high (bool): Data-Enable pulse is active high
>> + - pixelclk-inverted (bool): pixelclock is inverted
> 
>> + - pixel-per-clk
> 
> pixel-per-clk is probably something that should either be part of the
> timing definition, or something computed internally to the display
> driver based on rules for the signal type, rather than something
> represented in DT.
> 
> The above comment assumes this property is intended to represent DVI's
> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
> it's something to do with e.g. a single-data-rate vs. double-data-rate
> property of the underlying physical connection, that's most likely
> something that should be defined in a binding specific to e.g. LVDS,
> rather than something generic.
> 
>> + - link-width: number of channels (e.g. LVDS)
>> + - bpp: bits-per-pixel
>> +
>> +timings-subnode
>> +---------------
>> +
>> +required properties:
>> +subnodes that specify
>> + - hactive, vactive: Display resolution
>> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
>> +   in pixels
>> +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
>> +   lines
>> + - clock: displayclock in Hz
>> +
>> +There are different ways of describing a display and its capabilities. The devicetree
>> +representation corresponds to the one commonly found in datasheets for displays.
>> +The description of the display and its timing is split in two parts: first the display
>> +properties like size in mm and (optionally) multiple subnodes with the supported timings.
>> +If a display supports multiple signal timings, the default-timing can be specified.
>> +
>> +Example:
>> +
>> +	display@0 {
>> +		width-mm = <800>;
>> +		height-mm = <480>;
>> +		default-timing = <&timing0>;
>> +		timings {
>> +			timing0: timing@0 {
> 
> If you're going to use a unit address ("@0") to ensure that node names
> are unique (which is not mandatory), then each node also needs a reg
> property with matching value, and #address-cells/#size-cells in the
> parent. Instead, you could name the nodes something unique based on the
> mode name to avoid this, e.g. 1080p24 { ... }.


I'm concerned that numbered nodes are being misused as arrays.

It's easy to make real arrays by including multiple cells in the value
of each timing parameter, and easy to choose a cell by saying the array
index instead of using the phandle.



> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

^ permalink raw reply

* Re: [PATCH 1/2] of: add helper to parse display specs
From: Stephen Warren @ 2012-10-01 16:53 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, linux-fbdev, dri-devel, Hans Verkuil,
	Tomi Valkeinen, Laurent Pinchart, kernel, linux-media
In-Reply-To: <1348500924-8551-2-git-send-email-s.trumtrar@pengutronix.de>

On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
> Parse a display-node with timings and hardware-specs from devictree.

> diff --git a/Documentation/devicetree/bindings/video/display b/Documentation/devicetree/bindings/video/display
> new file mode 100644
> index 0000000..722766a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/display

This should be display.txt.

> @@ -0,0 +1,208 @@
> +display bindings
> +=========
> +
> +display-node
> +------------

I'm not personally convinced about the direction this is going. While I
think it's reasonable to define DT bindings for displays, and DT
bindings for display modes, I'm not sure that it's reasonable to couple
them together into a single binding.

I think creating a well-defined timing binding first will be much
simpler than doing so within the context of a display binding; the
scope/content of a general display binding seems much less well-defined
to me at least, for reasons I mentioned before.

> +required properties:
> + - none
> +
> +optional properties:
> + - default-timing: the default timing value
> + - width-mm, height-mm: Display dimensions in mm

> + - hsync-active-high (bool): Hsync pulse is active high
> + - vsync-active-high (bool): Vsync pulse is active high

At least those two properties should exist in the display timing instead
(or perhaps as well). There are certainly cases where different similar
display modes are differentiated by hsync/vsync polarity more than
anything else. This is probably more likely with analog display
connectors than digital, but I see no reason why a DT binding for
display timing shouldn't cover both.

> + - de-active-high (bool): Data-Enable pulse is active high
> + - pixelclk-inverted (bool): pixelclock is inverted

> + - pixel-per-clk

pixel-per-clk is probably something that should either be part of the
timing definition, or something computed internally to the display
driver based on rules for the signal type, rather than something
represented in DT.

The above comment assumes this property is intended to represent DVI's
requirement for pixel clock doubling for low-pixel-clock-rate modes. If
it's something to do with e.g. a single-data-rate vs. double-data-rate
property of the underlying physical connection, that's most likely
something that should be defined in a binding specific to e.g. LVDS,
rather than something generic.

> + - link-width: number of channels (e.g. LVDS)
> + - bpp: bits-per-pixel
> +
> +timings-subnode
> +---------------
> +
> +required properties:
> +subnodes that specify
> + - hactive, vactive: Display resolution
> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
> +   in pixels
> +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
> +   lines
> + - clock: displayclock in Hz
> +
> +There are different ways of describing a display and its capabilities. The devicetree
> +representation corresponds to the one commonly found in datasheets for displays.
> +The description of the display and its timing is split in two parts: first the display
> +properties like size in mm and (optionally) multiple subnodes with the supported timings.
> +If a display supports multiple signal timings, the default-timing can be specified.
> +
> +Example:
> +
> +	display@0 {
> +		width-mm = <800>;
> +		height-mm = <480>;
> +		default-timing = <&timing0>;
> +		timings {
> +			timing0: timing@0 {

If you're going to use a unit address ("@0") to ensure that node names
are unique (which is not mandatory), then each node also needs a reg
property with matching value, and #address-cells/#size-cells in the
parent. Instead, you could name the nodes something unique based on the
mode name to avoid this, e.g. 1080p24 { ... }.


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).