Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 4/4] OMAPDSS: Add dump and debug support for LCD3
From: Chandrabhanu Mahapatra @ 2012-06-28  9:53 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1340874806.git.cmahapatra@ti.com>

DISPC functions have been modified to provide clock and register dumps and debug
support for the LCD3 manager.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   70 ++++++++++++++++++++++----------------
 1 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 644ff53..29b92ea 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2854,12 +2854,32 @@ unsigned long dispc_core_clk_rate(void)
 	return fclk / lcd;
 }
 
-void dispc_dump_clocks(struct seq_file *s)
+void dispc_dump_clocks_channel(struct seq_file *s, enum omap_channel channel)
 {
 	int lcd, pcd;
+	enum omap_dss_clk_source lcd_clk_src;
+
+	seq_printf(s, "- %s -\n", mgr_desc[channel].name);
+
+	lcd_clk_src = dss_get_lcd_clk_source(channel);
+
+	seq_printf(s, "%s clk source = %s (%s)\n", mgr_desc[channel].name,
+		dss_get_generic_clk_source_name(lcd_clk_src),
+		dss_feat_get_clk_source_name(lcd_clk_src));
+
+	dispc_mgr_get_lcd_divisor(channel, &lcd, &pcd);
+
+	seq_printf(s, "lck\t\t%-16lulck div\t%u\n",
+		dispc_mgr_lclk_rate(channel), lcd);
+	seq_printf(s, "pck\t\t%-16lupck div\t%u\n",
+		dispc_mgr_pclk_rate(channel), pcd);
+}
+
+void dispc_dump_clocks(struct seq_file *s)
+{
+	int lcd;
 	u32 l;
 	enum omap_dss_clk_source dispc_clk_src = dss_get_dispc_clk_source();
-	enum omap_dss_clk_source lcd_clk_src;
 
 	if (dispc_runtime_get())
 		return;
@@ -2880,36 +2900,13 @@ void dispc_dump_clocks(struct seq_file *s)
 		seq_printf(s, "lck\t\t%-16lulck div\t%u\n",
 				(dispc_fclk_rate()/lcd), lcd);
 	}
-	seq_printf(s, "- LCD1 -\n");
 
-	lcd_clk_src = dss_get_lcd_clk_source(OMAP_DSS_CHANNEL_LCD);
+	dispc_dump_clocks_channel(s, OMAP_DSS_CHANNEL_LCD);
 
-	seq_printf(s, "lcd1_clk source = %s (%s)\n",
-		dss_get_generic_clk_source_name(lcd_clk_src),
-		dss_feat_get_clk_source_name(lcd_clk_src));
-
-	dispc_mgr_get_lcd_divisor(OMAP_DSS_CHANNEL_LCD, &lcd, &pcd);
-
-	seq_printf(s, "lck\t\t%-16lulck div\t%u\n",
-			dispc_mgr_lclk_rate(OMAP_DSS_CHANNEL_LCD), lcd);
-	seq_printf(s, "pck\t\t%-16lupck div\t%u\n",
-			dispc_mgr_pclk_rate(OMAP_DSS_CHANNEL_LCD), pcd);
-	if (dss_has_feature(FEAT_MGR_LCD2)) {
-		seq_printf(s, "- LCD2 -\n");
-
-		lcd_clk_src = dss_get_lcd_clk_source(OMAP_DSS_CHANNEL_LCD2);
-
-		seq_printf(s, "lcd2_clk source = %s (%s)\n",
-			dss_get_generic_clk_source_name(lcd_clk_src),
-			dss_feat_get_clk_source_name(lcd_clk_src));
-
-		dispc_mgr_get_lcd_divisor(OMAP_DSS_CHANNEL_LCD2, &lcd, &pcd);
-
-		seq_printf(s, "lck\t\t%-16lulck div\t%u\n",
-				dispc_mgr_lclk_rate(OMAP_DSS_CHANNEL_LCD2), lcd);
-		seq_printf(s, "pck\t\t%-16lupck div\t%u\n",
-				dispc_mgr_pclk_rate(OMAP_DSS_CHANNEL_LCD2), pcd);
-	}
+	if (dss_has_feature(FEAT_MGR_LCD2))
+		dispc_dump_clocks_channel(s, OMAP_DSS_CHANNEL_LCD2);
+	if (dss_has_feature(FEAT_MGR_LCD3))
+		dispc_dump_clocks_channel(s, OMAP_DSS_CHANNEL_LCD3);
 
 	dispc_runtime_put();
 }
@@ -2962,6 +2959,12 @@ void dispc_dump_irqs(struct seq_file *s)
 		PIS(ACBIAS_COUNT_STAT2);
 		PIS(SYNC_LOST2);
 	}
+	if (dss_has_feature(FEAT_MGR_LCD3)) {
+		PIS(FRAMEDONE3);
+		PIS(VSYNC3);
+		PIS(ACBIAS_COUNT_STAT3);
+		PIS(SYNC_LOST3);
+	}
 #undef PIS
 }
 #endif
@@ -2973,6 +2976,7 @@ static void dispc_dump_regs(struct seq_file *s)
 		[OMAP_DSS_CHANNEL_LCD]		= "LCD",
 		[OMAP_DSS_CHANNEL_DIGIT]	= "TV",
 		[OMAP_DSS_CHANNEL_LCD2]		= "LCD2",
+		[OMAP_DSS_CHANNEL_LCD3]		= "LCD3",
 	};
 	const char *ovl_names[] = {
 		[OMAP_DSS_GFX]		= "GFX",
@@ -3005,6 +3009,10 @@ static void dispc_dump_regs(struct seq_file *s)
 		DUMPREG(DISPC_CONTROL2);
 		DUMPREG(DISPC_CONFIG2);
 	}
+	if (dss_has_feature(FEAT_MGR_LCD3)) {
+		DUMPREG(DISPC_CONTROL3);
+		DUMPREG(DISPC_CONFIG3);
+	}
 
 #undef DUMPREG
 
@@ -3387,6 +3395,8 @@ static void print_irq_status(u32 status)
 	PIS(SYNC_LOST_DIGIT);
 	if (dss_has_feature(FEAT_MGR_LCD2))
 		PIS(SYNC_LOST2);
+	if (dss_has_feature(FEAT_MGR_LCD3))
+		PIS(SYNC_LOST3);
 #undef PIS
 
 	printk("\n");
-- 
1.7.1


^ permalink raw reply related

* [PATCH 3/4] OMAPDSS: Add LCD3 overlay manager and Clock and IRQ support
From: Chandrabhanu Mahapatra @ 2012-06-28  9:53 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1340874806.git.cmahapatra@ti.com>

The support feature for LCD3 manager has been added into the DSS features
structure. LCD3 panel has registers as DISPC_CONTROL3 and DISPC_CONFIG3 just
like those in LCD and LCD2 panels. These registers control the Display
Controller (DISPC) module for LCD3 output. The three LCDs support Display Serial
Interface (DSI), Remote Frame Buffer Interface (RFBI) and Parallel CMOS Output
Interface (DPI). These LCDs can be connected through parallel output interface
using DISPC and RFBI or DPI. For serial interface DSS uses DSI.

The LCD3 panel, just like LCD and LCD2 panels, has a clock switch in DSS_CTRL
register which has been enabled. The clock switch chooses between DSS_CLK and
DPLL_DSI1_C_CLK1 as source for LCD3_CLK. New IRQs as DISPC_IRQ_VSYNC3,
DISPC_IRQ_FRAMEDONE3, DISPC_IRQ_ACBIAS_COUNT_STAT3 and DISPC_IRQ_SYNC_LOST3 have
been added specific to the new manager.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c        |   48 ++++++++++++++++++++++++++++++-
 drivers/video/omap2/dss/dispc.h        |    2 +
 drivers/video/omap2/dss/dss.c          |   12 +++++--
 drivers/video/omap2/dss/dss_features.h |    5 ++-
 drivers/video/omap2/dss/manager.c      |    4 ++
 drivers/video/omap2/dss/overlay.c      |   14 ++++++++-
 include/video/omapdss.h                |    5 +++
 7 files changed, 80 insertions(+), 10 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 6c16b81..644ff53 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -191,6 +191,23 @@ static const struct {
 			[DISPC_MGR_FLD_FIFOHANDCHECK]	= { DISPC_CONFIG2,  16, 16 },
 		},
 	},
+	[OMAP_DSS_CHANNEL_LCD3] = {
+		.name		= "LCD3",
+		.vsync_irq	= DISPC_IRQ_VSYNC3,
+		.framedone_irq	= DISPC_IRQ_FRAMEDONE3,
+		.sync_lost_irq	= DISPC_IRQ_SYNC_LOST3,
+		.reg_desc	= {
+			[DISPC_MGR_FLD_ENABLE]		= { DISPC_CONTROL3,  0,  0 },
+			[DISPC_MGR_FLD_STNTFT]		= { DISPC_CONTROL3,  3,  3 },
+			[DISPC_MGR_FLD_GO]		= { DISPC_CONTROL3,  5,  5 },
+			[DISPC_MGR_FLD_TFTDATALINES]	= { DISPC_CONTROL3,  9,  8 },
+			[DISPC_MGR_FLD_STALLMODE]	= { DISPC_CONTROL3, 11, 11 },
+			[DISPC_MGR_FLD_TCKENABLE]	= { DISPC_CONFIG3,  10, 10 },
+			[DISPC_MGR_FLD_TCKSELECTION]	= { DISPC_CONFIG3,  11, 11 },
+			[DISPC_MGR_FLD_CPR]		= { DISPC_CONFIG3,  15, 15 },
+			[DISPC_MGR_FLD_FIFOHANDCHECK]	= { DISPC_CONFIG3,  16, 16 },
+		},
+	},
 };
 
 static void _omap_dispc_set_irqs(void);
@@ -240,6 +257,10 @@ static void dispc_save_context(void)
 		SR(CONTROL2);
 		SR(CONFIG2);
 	}
+	if (dss_has_feature(FEAT_MGR_LCD3)) {
+		SR(CONTROL3);
+		SR(CONFIG3);
+	}
 
 	for (i = 0; i < dss_feat_get_num_mgrs(); i++) {
 		SR(DEFAULT_COLOR(i));
@@ -353,6 +374,8 @@ static void dispc_restore_context(void)
 		RR(GLOBAL_ALPHA);
 	if (dss_has_feature(FEAT_MGR_LCD2))
 		RR(CONFIG2);
+	if (dss_has_feature(FEAT_MGR_LCD3))
+		RR(CONFIG3);
 
 	for (i = 0; i < dss_feat_get_num_mgrs(); i++) {
 		RR(DEFAULT_COLOR(i));
@@ -438,6 +461,8 @@ static void dispc_restore_context(void)
 	RR(CONTROL);
 	if (dss_has_feature(FEAT_MGR_LCD2))
 		RR(CONTROL2);
+	if (dss_has_feature(FEAT_MGR_LCD3))
+		RR(CONTROL3);
 	/* clear spurious SYNC_LOST_DIGIT interrupts */
 	dispc_write_reg(DISPC_IRQSTATUS, DISPC_IRQ_SYNC_LOST_DIGIT);
 
@@ -477,7 +502,8 @@ void dispc_runtime_put(void)
 static inline bool dispc_mgr_is_lcd(enum omap_channel channel)
 {
 	if (channel = OMAP_DSS_CHANNEL_LCD ||
-			channel = OMAP_DSS_CHANNEL_LCD2)
+			channel = OMAP_DSS_CHANNEL_LCD2 ||
+			channel = OMAP_DSS_CHANNEL_LCD3)
 		return true;
 	else
 		return false;
@@ -868,6 +894,15 @@ void dispc_ovl_set_channel_out(enum omap_plane plane, enum omap_channel channel)
 			chan = 0;
 			chan2 = 1;
 			break;
+		case OMAP_DSS_CHANNEL_LCD3:
+			if (dss_has_feature(FEAT_MGR_LCD3)) {
+				chan = 0;
+				chan2 = 2;
+			} else {
+				BUG();
+				return;
+			}
+			break;
 		default:
 			BUG();
 			return;
@@ -903,7 +938,14 @@ static enum omap_channel dispc_ovl_get_channel_out(enum omap_plane plane)
 
 	val = dispc_read_reg(DISPC_OVL_ATTRIBUTES(plane));
 
-	if (dss_has_feature(FEAT_MGR_LCD2)) {
+	if (dss_has_feature(FEAT_MGR_LCD3)) {
+		if (FLD_GET(val, 31, 30) = 0)
+			channel = FLD_GET(val, shift, shift);
+		else if (FLD_GET(val, 31, 30) = 1)
+			channel = OMAP_DSS_CHANNEL_LCD2;
+		else
+			channel = OMAP_DSS_CHANNEL_LCD3;
+	} else if (dss_has_feature(FEAT_MGR_LCD2)) {
 		if (FLD_GET(val, 31, 30) = 0)
 			channel = FLD_GET(val, shift, shift);
 		else
@@ -3588,6 +3630,8 @@ static void _omap_dispc_initialize_irq(void)
 	dispc.irq_error_mask = DISPC_IRQ_MASK_ERROR;
 	if (dss_has_feature(FEAT_MGR_LCD2))
 		dispc.irq_error_mask |= DISPC_IRQ_SYNC_LOST2;
+	if (dss_has_feature(FEAT_MGR_LCD3))
+		dispc.irq_error_mask |= DISPC_IRQ_SYNC_LOST3;
 	if (dss_feat_get_num_ovls() > 3)
 		dispc.irq_error_mask |= DISPC_IRQ_VID3_FIFO_UNDERFLOW;
 
diff --git a/drivers/video/omap2/dss/dispc.h b/drivers/video/omap2/dss/dispc.h
index 420c980..92d8a9b 100644
--- a/drivers/video/omap2/dss/dispc.h
+++ b/drivers/video/omap2/dss/dispc.h
@@ -36,6 +36,8 @@
 #define DISPC_CONTROL2			0x0238
 #define DISPC_CONFIG2			0x0620
 #define DISPC_DIVISOR			0x0804
+#define DISPC_CONTROL3                  0x0848
+#define DISPC_CONFIG3                   0x084C
 
 /* DISPC overlay registers */
 #define DISPC_OVL_BA0(n)		(DISPC_OVL_BASE(n) + \
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 7706323..4c351e5 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -388,7 +388,8 @@ void dss_select_lcd_clk_source(enum omap_channel channel,
 		dsi_wait_pll_hsdiv_dispc_active(dsidev);
 		break;
 	case OMAP_DSS_CLK_SRC_DSI2_PLL_HSDIV_DISPC:
-		BUG_ON(channel != OMAP_DSS_CHANNEL_LCD2);
+		BUG_ON(channel != OMAP_DSS_CHANNEL_LCD2 &&
+		       channel != OMAP_DSS_CHANNEL_LCD3);
 		b = 1;
 		dsidev = dsi_get_dsidev_from_id(1);
 		dsi_wait_pll_hsdiv_dispc_active(dsidev);
@@ -398,10 +399,12 @@ void dss_select_lcd_clk_source(enum omap_channel channel,
 		return;
 	}
 
-	pos = channel = OMAP_DSS_CHANNEL_LCD ? 0 : 12;
+	pos = channel = OMAP_DSS_CHANNEL_LCD ? 0 :
+	     (channel = OMAP_DSS_CHANNEL_LCD2 ? 12 : 19);
 	REG_FLD_MOD(DSS_CONTROL, b, pos, pos);	/* LCDx_CLK_SWITCH */
 
-	ix = channel = OMAP_DSS_CHANNEL_LCD ? 0 : 1;
+	ix = channel = OMAP_DSS_CHANNEL_LCD ? 0 :
+	    (channel = OMAP_DSS_CHANNEL_LCD2 ? 1 : 2);
 	dss.lcd_clk_source[ix] = clk_src;
 }
 
@@ -418,7 +421,8 @@ enum omap_dss_clk_source dss_get_dsi_clk_source(int dsi_module)
 enum omap_dss_clk_source dss_get_lcd_clk_source(enum omap_channel channel)
 {
 	if (dss_has_feature(FEAT_LCD_CLK_SRC)) {
-		int ix = channel = OMAP_DSS_CHANNEL_LCD ? 0 : 1;
+		int ix = channel = OMAP_DSS_CHANNEL_LCD ? 0 :
+			(channel = OMAP_DSS_CHANNEL_LCD2 ? 1 : 2);
 		return dss.lcd_clk_source[ix];
 	} else {
 		/* LCD_CLK source is the same as DISPC_FCLK source for
diff --git a/drivers/video/omap2/dss/dss_features.h b/drivers/video/omap2/dss/dss_features.h
index bdf469f..996ffcb 100644
--- a/drivers/video/omap2/dss/dss_features.h
+++ b/drivers/video/omap2/dss/dss_features.h
@@ -24,9 +24,9 @@
 #include "ti_hdmi.h"
 #endif
 
-#define MAX_DSS_MANAGERS	3
+#define MAX_DSS_MANAGERS	4
 #define MAX_DSS_OVERLAYS	4
-#define MAX_DSS_LCD_MANAGERS	2
+#define MAX_DSS_LCD_MANAGERS	3
 #define MAX_NUM_DSI		2
 
 /* DSS has feature id */
@@ -36,6 +36,7 @@ enum dss_feat_id {
 	FEAT_PCKFREEENABLE,
 	FEAT_FUNCGATED,
 	FEAT_MGR_LCD2,
+	FEAT_MGR_LCD3,
 	FEAT_LINEBUFFERSPLIT,
 	FEAT_ROWREPEATENABLE,
 	FEAT_RESIZECONF,
diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index bb602a2..a51eb06 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -541,6 +541,10 @@ int dss_init_overlay_managers(struct platform_device *pdev)
 			mgr->name = "lcd2";
 			mgr->id = OMAP_DSS_CHANNEL_LCD2;
 			break;
+		case 3:
+			mgr->name = "lcd3";
+			mgr->id = OMAP_DSS_CHANNEL_LCD3;
+			break;
 		}
 
 		mgr->set_device = &dss_mgr_set_device;
diff --git a/drivers/video/omap2/dss/overlay.c b/drivers/video/omap2/dss/overlay.c
index b0ba60f..898ceed 100644
--- a/drivers/video/omap2/dss/overlay.c
+++ b/drivers/video/omap2/dss/overlay.c
@@ -528,14 +528,24 @@ void dss_recheck_connections(struct omap_dss_device *dssdev, bool force)
 	struct omap_overlay_manager *lcd_mgr;
 	struct omap_overlay_manager *tv_mgr;
 	struct omap_overlay_manager *lcd2_mgr = NULL;
+	struct omap_overlay_manager *lcd3_mgr = NULL;
 	struct omap_overlay_manager *mgr = NULL;
 
 	lcd_mgr = omap_dss_get_overlay_manager(OMAP_DSS_OVL_MGR_LCD);
 	tv_mgr = omap_dss_get_overlay_manager(OMAP_DSS_OVL_MGR_TV);
 	if (dss_has_feature(FEAT_MGR_LCD2))
 		lcd2_mgr = omap_dss_get_overlay_manager(OMAP_DSS_OVL_MGR_LCD2);
-
-	if (dssdev->channel = OMAP_DSS_CHANNEL_LCD2) {
+	if (dss_has_feature(FEAT_MGR_LCD3))
+		lcd3_mgr = omap_dss_get_overlay_manager(OMAP_DSS_OVL_MGR_LCD3);
+
+	if (dssdev->channel = OMAP_DSS_CHANNEL_LCD3) {
+		if (!lcd3_mgr->device || force) {
+			if (lcd3_mgr->device)
+				lcd3_mgr->unset_device(lcd3_mgr);
+			lcd3_mgr->set_device(lcd3_mgr, dssdev);
+			mgr = lcd3_mgr;
+		}
+	} else if (dssdev->channel = OMAP_DSS_CHANNEL_LCD2) {
 		if (!lcd2_mgr->device || force) {
 			if (lcd2_mgr->device)
 				lcd2_mgr->unset_device(lcd2_mgr);
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index cf4f716..2f9fff4 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -48,6 +48,10 @@
 #define DISPC_IRQ_FRAMEDONEWB		(1 << 23)
 #define DISPC_IRQ_FRAMEDONETV		(1 << 24)
 #define DISPC_IRQ_WBBUFFEROVERFLOW	(1 << 25)
+#define DISPC_IRQ_FRAMEDONE3		(1 << 26)
+#define DISPC_IRQ_VSYNC3		(1 << 27)
+#define DISPC_IRQ_ACBIAS_COUNT_STAT3	(1 << 28)
+#define DISPC_IRQ_SYNC_LOST3		(1 << 29)
 
 struct omap_dss_device;
 struct omap_overlay_manager;
@@ -173,6 +177,7 @@ enum omap_dss_overlay_managers {
 	OMAP_DSS_OVL_MGR_LCD,
 	OMAP_DSS_OVL_MGR_TV,
 	OMAP_DSS_OVL_MGR_LCD2,
+	OMAP_DSS_OVL_MGR_LCD3,
 };
 
 enum omap_dss_rotation_type {
-- 
1.7.1


^ permalink raw reply related

* [PATCH 2/4] OMAPDSS: Add support for LCD3 channel
From: Chandrabhanu Mahapatra @ 2012-06-28  9:52 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1340874806.git.cmahapatra@ti.com>

OMAP5 Display Subsystem (DSS) architecture comes with a additional LCD3 channel
with its own dedicated overlay manager. The current patch adds LCD3 channel and
basic register support for LCD3 channel. It adds register addresses for various
Display Controller (DISPC) registers like DISPC_DEFAULT_COLOR, DISPC_TIMING_H,
DISPC_DIVISORo, etc.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.h |   26 ++++++++++++++++++++++++++
 include/video/omapdss.h         |    1 +
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.h b/drivers/video/omap2/dss/dispc.h
index f278080..420c980 100644
--- a/drivers/video/omap2/dss/dispc.h
+++ b/drivers/video/omap2/dss/dispc.h
@@ -118,6 +118,8 @@ static inline u16 DISPC_DEFAULT_COLOR(enum omap_channel channel)
 		return 0x0050;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x03AC;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x0814;
 	default:
 		BUG();
 		return 0;
@@ -133,6 +135,8 @@ static inline u16 DISPC_TRANS_COLOR(enum omap_channel channel)
 		return 0x0058;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x03B0;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x0818;
 	default:
 		BUG();
 		return 0;
@@ -149,6 +153,8 @@ static inline u16 DISPC_TIMING_H(enum omap_channel channel)
 		return 0;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x0400;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x0840;
 	default:
 		BUG();
 		return 0;
@@ -165,6 +171,8 @@ static inline u16 DISPC_TIMING_V(enum omap_channel channel)
 		return 0;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x0404;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x0844;
 	default:
 		BUG();
 		return 0;
@@ -181,6 +189,8 @@ static inline u16 DISPC_POL_FREQ(enum omap_channel channel)
 		return 0;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x0408;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x083C;
 	default:
 		BUG();
 		return 0;
@@ -197,6 +207,8 @@ static inline u16 DISPC_DIVISORo(enum omap_channel channel)
 		return 0;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x040C;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x0838;
 	default:
 		BUG();
 		return 0;
@@ -213,6 +225,8 @@ static inline u16 DISPC_SIZE_MGR(enum omap_channel channel)
 		return 0x0078;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x03CC;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x0834;
 	default:
 		BUG();
 		return 0;
@@ -229,6 +243,8 @@ static inline u16 DISPC_DATA_CYCLE1(enum omap_channel channel)
 		return 0;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x03C0;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x0828;
 	default:
 		BUG();
 		return 0;
@@ -245,6 +261,8 @@ static inline u16 DISPC_DATA_CYCLE2(enum omap_channel channel)
 		return 0;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x03C4;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x082C;
 	default:
 		BUG();
 		return 0;
@@ -261,6 +279,8 @@ static inline u16 DISPC_DATA_CYCLE3(enum omap_channel channel)
 		return 0;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x03C8;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x0830;
 	default:
 		BUG();
 		return 0;
@@ -277,6 +297,8 @@ static inline u16 DISPC_CPR_COEF_R(enum omap_channel channel)
 		return 0;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x03BC;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x0824;
 	default:
 		BUG();
 		return 0;
@@ -293,6 +315,8 @@ static inline u16 DISPC_CPR_COEF_G(enum omap_channel channel)
 		return 0;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x03B8;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x0820;
 	default:
 		BUG();
 		return 0;
@@ -309,6 +333,8 @@ static inline u16 DISPC_CPR_COEF_B(enum omap_channel channel)
 		return 0;
 	case OMAP_DSS_CHANNEL_LCD2:
 		return 0x03B4;
+	case OMAP_DSS_CHANNEL_LCD3:
+		return 0x081C;
 	default:
 		BUG();
 		return 0;
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index c8e59b4..cf4f716 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -75,6 +75,7 @@ enum omap_channel {
 	OMAP_DSS_CHANNEL_LCD	= 0,
 	OMAP_DSS_CHANNEL_DIGIT	= 1,
 	OMAP_DSS_CHANNEL_LCD2	= 2,
+	OMAP_DSS_CHANNEL_LCD3	= 3,
 };
 
 enum omap_color_mode {
-- 
1.7.1


^ permalink raw reply related

* [PATCH 1/4] OMAPDSS: Cleanup implementation of LCD channels
From: Chandrabhanu Mahapatra @ 2012-06-28  9:52 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1340874806.git.cmahapatra@ti.com>

The current implementation of LCD channels and managers consists of a number of
if-else construct which has been replaced by a simpler interface. A constant
structure mgr_desc has been created in Display Controller (DISPC) module. The
mgr_desc contains for each channel its name, irqs and  is initialized one time
with all registers and their corresponding fields to be written to enable
various features of Display Subsystem. This structure is later used by various
functions of DISPC which simplifies the further implementation of LCD channels
and its corresponding managers.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c   |  233 +++++++++++++++++--------------------
 drivers/video/omap2/dss/dsi.c     |    6 +-
 drivers/video/omap2/dss/dss.h     |    6 +
 drivers/video/omap2/dss/manager.c |   12 +--
 4 files changed, 121 insertions(+), 136 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 4749ac3..6c16b81 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -57,6 +57,8 @@
 
 #define DISPC_MAX_NR_ISRS		8
 
+#define DISPC_MGR_FLD_MAX		9
+
 struct omap_dispc_isr_data {
 	omap_dispc_isr_t	isr;
 	void			*arg;
@@ -119,6 +121,78 @@ enum omap_color_component {
 	DISPC_COLOR_COMPONENT_UV		= 1 << 1,
 };
 
+enum mgr_reg_fields {
+	DISPC_MGR_FLD_ENABLE,
+	DISPC_MGR_FLD_STNTFT,
+	DISPC_MGR_FLD_GO,
+	DISPC_MGR_FLD_TFTDATALINES,
+	DISPC_MGR_FLD_STALLMODE,
+	DISPC_MGR_FLD_TCKENABLE,
+	DISPC_MGR_FLD_TCKSELECTION,
+	DISPC_MGR_FLD_CPR,
+	DISPC_MGR_FLD_FIFOHANDCHECK,
+};
+
+static const struct {
+	const char *name;
+	u32 vsync_irq;
+	u32 framedone_irq;
+	u32 sync_lost_irq;
+	struct reg_field reg_desc[DISPC_MGR_FLD_MAX];
+} mgr_desc[] = {
+	[OMAP_DSS_CHANNEL_LCD] = {
+		.name		= "LCD",
+		.vsync_irq	= DISPC_IRQ_VSYNC,
+		.framedone_irq	= DISPC_IRQ_FRAMEDONE,
+		.sync_lost_irq	= DISPC_IRQ_SYNC_LOST,
+		.reg_desc	= {
+			[DISPC_MGR_FLD_ENABLE]		= { DISPC_CONTROL,  0,  0 },
+			[DISPC_MGR_FLD_STNTFT]		= { DISPC_CONTROL,  3,  3 },
+			[DISPC_MGR_FLD_GO]		= { DISPC_CONTROL,  5,  5 },
+			[DISPC_MGR_FLD_TFTDATALINES]	= { DISPC_CONTROL,  9,  8 },
+			[DISPC_MGR_FLD_STALLMODE]	= { DISPC_CONTROL, 11, 11 },
+			[DISPC_MGR_FLD_TCKENABLE]	= { DISPC_CONFIG,  10, 10 },
+			[DISPC_MGR_FLD_TCKSELECTION]	= { DISPC_CONFIG,  11, 11 },
+			[DISPC_MGR_FLD_CPR]		= { DISPC_CONFIG,  15, 15 },
+			[DISPC_MGR_FLD_FIFOHANDCHECK]	= { DISPC_CONFIG,  16, 16 },
+		},
+	},
+	[OMAP_DSS_CHANNEL_DIGIT] = {
+		.name		= "DIGIT",
+		.vsync_irq	= DISPC_IRQ_EVSYNC_ODD | DISPC_IRQ_EVSYNC_EVEN,
+		.framedone_irq	= DISPC_IRQ_FRAMEDONETV,
+		.sync_lost_irq	= DISPC_IRQ_SYNC_LOST_DIGIT,
+		.reg_desc	= {
+			[DISPC_MGR_FLD_ENABLE]		= { DISPC_CONTROL,  1,  1 },
+			[DISPC_MGR_FLD_STNTFT]		= { },
+			[DISPC_MGR_FLD_GO]		= { DISPC_CONTROL,  6,  6 },
+			[DISPC_MGR_FLD_TFTDATALINES]	= { },
+			[DISPC_MGR_FLD_STALLMODE]	= { },
+			[DISPC_MGR_FLD_TCKENABLE]	= { DISPC_CONFIG,  12, 12 },
+			[DISPC_MGR_FLD_TCKSELECTION]	= { DISPC_CONFIG,  13, 13 },
+			[DISPC_MGR_FLD_CPR]		= { },
+			[DISPC_MGR_FLD_FIFOHANDCHECK]	= { DISPC_CONFIG,  16, 16 },
+		},
+	},
+	[OMAP_DSS_CHANNEL_LCD2] = {
+		.name		= "LCD2",
+		.vsync_irq	= DISPC_IRQ_VSYNC2,
+		.framedone_irq	= DISPC_IRQ_FRAMEDONE2,
+		.sync_lost_irq	= DISPC_IRQ_SYNC_LOST2,
+		.reg_desc	= {
+			[DISPC_MGR_FLD_ENABLE]		= { DISPC_CONTROL2,  0,  0 },
+			[DISPC_MGR_FLD_STNTFT]		= { DISPC_CONTROL2,  3,  3 },
+			[DISPC_MGR_FLD_GO]		= { DISPC_CONTROL2,  5,  5 },
+			[DISPC_MGR_FLD_TFTDATALINES]	= { DISPC_CONTROL2,  9,  8 },
+			[DISPC_MGR_FLD_STALLMODE]	= { DISPC_CONTROL2, 11, 11 },
+			[DISPC_MGR_FLD_TCKENABLE]	= { DISPC_CONFIG2,  10, 10 },
+			[DISPC_MGR_FLD_TCKSELECTION]	= { DISPC_CONFIG2,  11, 11 },
+			[DISPC_MGR_FLD_CPR]		= { DISPC_CONFIG2,  15, 15 },
+			[DISPC_MGR_FLD_FIFOHANDCHECK]	= { DISPC_CONFIG2,  16, 16 },
+		},
+	},
+};
+
 static void _omap_dispc_set_irqs(void);
 
 static inline void dispc_write_reg(const u16 idx, u32 val)
@@ -131,6 +205,19 @@ static inline u32 dispc_read_reg(const u16 idx)
 	return __raw_readl(dispc.base + idx);
 }
 
+static u32 mgr_fld_read(enum omap_channel channel, enum mgr_reg_fields regfld)
+{
+	const struct reg_field rfld = mgr_desc[channel].reg_desc[regfld];
+	return FLD_GET(dispc_read_reg(rfld.reg), rfld.high, rfld.low);
+}
+
+static void mgr_fld_write(enum omap_channel channel,
+					enum mgr_reg_fields regfld, int val) {
+	const struct reg_field rfld = mgr_desc[channel].reg_desc[regfld];
+	dispc_write_reg(rfld.reg, FLD_MOD(dispc_read_reg(rfld.reg), val,
+				rfld.high, rfld.low));
+}
+
 #define SR(reg) \
 	dispc.ctx[DISPC_##reg / sizeof(u32)] = dispc_read_reg(DISPC_##reg)
 #define RR(reg) \
@@ -398,90 +485,39 @@ static inline bool dispc_mgr_is_lcd(enum omap_channel channel)
 
 u32 dispc_mgr_get_vsync_irq(enum omap_channel channel)
 {
-	switch (channel) {
-	case OMAP_DSS_CHANNEL_LCD:
-		return DISPC_IRQ_VSYNC;
-	case OMAP_DSS_CHANNEL_LCD2:
-		return DISPC_IRQ_VSYNC2;
-	case OMAP_DSS_CHANNEL_DIGIT:
-		return DISPC_IRQ_EVSYNC_ODD | DISPC_IRQ_EVSYNC_EVEN;
-	default:
-		BUG();
-		return 0;
-	}
+	return mgr_desc[channel].vsync_irq;
 }
 
 u32 dispc_mgr_get_framedone_irq(enum omap_channel channel)
 {
-	switch (channel) {
-	case OMAP_DSS_CHANNEL_LCD:
-		return DISPC_IRQ_FRAMEDONE;
-	case OMAP_DSS_CHANNEL_LCD2:
-		return DISPC_IRQ_FRAMEDONE2;
-	case OMAP_DSS_CHANNEL_DIGIT:
-		return 0;
-	default:
-		BUG();
-		return 0;
-	}
+	return mgr_desc[channel].framedone_irq;
 }
 
 bool dispc_mgr_go_busy(enum omap_channel channel)
 {
-	int bit;
-
-	if (dispc_mgr_is_lcd(channel))
-		bit = 5; /* GOLCD */
-	else
-		bit = 6; /* GODIGIT */
-
-	if (channel = OMAP_DSS_CHANNEL_LCD2)
-		return REG_GET(DISPC_CONTROL2, bit, bit) = 1;
-	else
-		return REG_GET(DISPC_CONTROL, bit, bit) = 1;
+	return mgr_fld_read(channel, DISPC_MGR_FLD_GO) = 1;
 }
 
 void dispc_mgr_go(enum omap_channel channel)
 {
-	int bit;
 	bool enable_bit, go_bit;
 
-	if (dispc_mgr_is_lcd(channel))
-		bit = 0; /* LCDENABLE */
-	else
-		bit = 1; /* DIGITALENABLE */
-
 	/* if the channel is not enabled, we don't need GO */
-	if (channel = OMAP_DSS_CHANNEL_LCD2)
-		enable_bit = REG_GET(DISPC_CONTROL2, bit, bit) = 1;
-	else
-		enable_bit = REG_GET(DISPC_CONTROL, bit, bit) = 1;
+	enable_bit = mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE) = 1;
 
 	if (!enable_bit)
 		return;
 
-	if (dispc_mgr_is_lcd(channel))
-		bit = 5; /* GOLCD */
-	else
-		bit = 6; /* GODIGIT */
-
-	if (channel = OMAP_DSS_CHANNEL_LCD2)
-		go_bit = REG_GET(DISPC_CONTROL2, bit, bit) = 1;
-	else
-		go_bit = REG_GET(DISPC_CONTROL, bit, bit) = 1;
+	go_bit = mgr_fld_read(channel, DISPC_MGR_FLD_GO) = 1;
 
 	if (go_bit) {
 		DSSERR("GO bit not down for channel %d\n", channel);
 		return;
 	}
 
-	DSSDBG("GO %s\n", channel = OMAP_DSS_CHANNEL_LCD ? "LCD" :
-		(channel = OMAP_DSS_CHANNEL_LCD2 ? "LCD2" : "DIGIT"));
+	DSSDBG("GO %s\n", mgr_desc[channel].name);
 
-	if (channel = OMAP_DSS_CHANNEL_LCD2)
-		REG_FLD_MOD(DISPC_CONTROL2, 1, bit, bit);
-	else
-		REG_FLD_MOD(DISPC_CONTROL, 1, bit, bit);
+	mgr_fld_write(channel, DISPC_MGR_FLD_GO, 1);
 }
 
 static void dispc_ovl_write_firh_reg(enum omap_plane plane, int reg, u32 value)
@@ -922,16 +958,10 @@ void dispc_enable_gamma_table(bool enable)
 
 static void dispc_mgr_enable_cpr(enum omap_channel channel, bool enable)
 {
-	u16 reg;
-
-	if (channel = OMAP_DSS_CHANNEL_LCD)
-		reg = DISPC_CONFIG;
-	else if (channel = OMAP_DSS_CHANNEL_LCD2)
-		reg = DISPC_CONFIG2;
-	else
+	if (channel = OMAP_DSS_CHANNEL_DIGIT)
 		return;
 
-	REG_FLD_MOD(reg, enable, 15, 15);
+	mgr_fld_write(channel, DISPC_MGR_FLD_CPR, enable);
 }
 
 static void dispc_mgr_set_cpr_coef(enum omap_channel channel,
@@ -2254,14 +2284,9 @@ static void dispc_disable_isr(void *data, u32 mask)
 
 static void _enable_lcd_out(enum omap_channel channel, bool enable)
 {
-	if (channel = OMAP_DSS_CHANNEL_LCD2) {
-		REG_FLD_MOD(DISPC_CONTROL2, enable ? 1 : 0, 0, 0);
-		/* flush posted write */
-		dispc_read_reg(DISPC_CONTROL2);
-	} else {
-		REG_FLD_MOD(DISPC_CONTROL, enable ? 1 : 0, 0, 0);
-		dispc_read_reg(DISPC_CONTROL);
-	}
+	mgr_fld_write(channel, DISPC_MGR_FLD_ENABLE, enable);
+	/* flush posted write */
+	mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
 }
 
 static void dispc_mgr_enable_lcd_out(enum omap_channel channel, bool enable)
@@ -2274,12 +2299,9 @@ static void dispc_mgr_enable_lcd_out(enum omap_channel channel, bool enable)
 	/* When we disable LCD output, we need to wait until frame is done.
 	 * Otherwise the DSS is still working, and turning off the clocks
 	 * prevents DSS from going to OFF mode */
-	is_on = channel = OMAP_DSS_CHANNEL_LCD2 ?
-			REG_GET(DISPC_CONTROL2, 0, 0) :
-			REG_GET(DISPC_CONTROL, 0, 0);
+	is_on = mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
 
-	irq = channel = OMAP_DSS_CHANNEL_LCD2 ? DISPC_IRQ_FRAMEDONE2 :
-			DISPC_IRQ_FRAMEDONE;
+	irq = mgr_desc[channel].framedone_irq;
 
 	if (!enable && is_on) {
 		init_completion(&frame_done_completion);
@@ -2384,16 +2406,7 @@ static void dispc_mgr_enable_digit_out(bool enable)
 
 bool dispc_mgr_is_enabled(enum omap_channel channel)
 {
-	if (channel = OMAP_DSS_CHANNEL_LCD)
-		return !!REG_GET(DISPC_CONTROL, 0, 0);
-	else if (channel = OMAP_DSS_CHANNEL_DIGIT)
-		return !!REG_GET(DISPC_CONTROL, 1, 1);
-	else if (channel = OMAP_DSS_CHANNEL_LCD2)
-		return !!REG_GET(DISPC_CONTROL2, 0, 0);
-	else {
-		BUG();
-		return false;
-	}
+	return !!mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
 }
 
 void dispc_mgr_enable(enum omap_channel channel, bool enable)
@@ -2432,10 +2445,7 @@ void dispc_pck_free_enable(bool enable)
 
 void dispc_mgr_enable_fifohandcheck(enum omap_channel channel, bool enable)
 {
-	if (channel = OMAP_DSS_CHANNEL_LCD2)
-		REG_FLD_MOD(DISPC_CONFIG2, enable ? 1 : 0, 16, 16);
-	else
-		REG_FLD_MOD(DISPC_CONFIG, enable ? 1 : 0, 16, 16);
+	mgr_fld_write(channel, DISPC_MGR_FLD_FIFOHANDCHECK, enable);
 }
 
 
@@ -2458,10 +2468,7 @@ void dispc_mgr_set_lcd_display_type(enum omap_channel channel,
 		return;
 	}
 
-	if (channel = OMAP_DSS_CHANNEL_LCD2)
-		REG_FLD_MOD(DISPC_CONTROL2, mode, 3, 3);
-	else
-		REG_FLD_MOD(DISPC_CONTROL, mode, 3, 3);
+	mgr_fld_write(channel, DISPC_MGR_FLD_STNTFT, mode);
 }
 
 void dispc_set_loadmode(enum omap_dss_load_mode mode)
@@ -2479,24 +2486,14 @@ static void dispc_mgr_set_trans_key(enum omap_channel ch,
 		enum omap_dss_trans_key_type type,
 		u32 trans_key)
 {
-	if (ch = OMAP_DSS_CHANNEL_LCD)
-		REG_FLD_MOD(DISPC_CONFIG, type, 11, 11);
-	else if (ch = OMAP_DSS_CHANNEL_DIGIT)
-		REG_FLD_MOD(DISPC_CONFIG, type, 13, 13);
-	else /* OMAP_DSS_CHANNEL_LCD2 */
-		REG_FLD_MOD(DISPC_CONFIG2, type, 11, 11);
+	mgr_fld_write(ch, DISPC_MGR_FLD_TCKSELECTION, type);
 
 	dispc_write_reg(DISPC_TRANS_COLOR(ch), trans_key);
 }
 
 static void dispc_mgr_enable_trans_key(enum omap_channel ch, bool enable)
 {
-	if (ch = OMAP_DSS_CHANNEL_LCD)
-		REG_FLD_MOD(DISPC_CONFIG, enable, 10, 10);
-	else if (ch = OMAP_DSS_CHANNEL_DIGIT)
-		REG_FLD_MOD(DISPC_CONFIG, enable, 12, 12);
-	else /* OMAP_DSS_CHANNEL_LCD2 */
-		REG_FLD_MOD(DISPC_CONFIG2, enable, 10, 10);
+	mgr_fld_write(ch, DISPC_MGR_FLD_TCKENABLE, enable);
 }
 
 static void dispc_mgr_enable_alpha_fixed_zorder(enum omap_channel ch,
@@ -2547,10 +2544,7 @@ void dispc_mgr_set_tft_data_lines(enum omap_channel channel, u8 data_lines)
 		return;
 	}
 
-	if (channel = OMAP_DSS_CHANNEL_LCD2)
-		REG_FLD_MOD(DISPC_CONTROL2, code, 9, 8);
-	else
-		REG_FLD_MOD(DISPC_CONTROL, code, 9, 8);
+	mgr_fld_write(channel, DISPC_MGR_FLD_TFTDATALINES, code);
 }
 
 void dispc_mgr_set_io_pad_mode(enum dss_io_pad_mode mode)
@@ -2584,10 +2578,7 @@ void dispc_mgr_set_io_pad_mode(enum dss_io_pad_mode mode)
 
 void dispc_mgr_enable_stallmode(enum omap_channel channel, bool enable)
 {
-	if (channel = OMAP_DSS_CHANNEL_LCD2)
-		REG_FLD_MOD(DISPC_CONTROL2, enable, 11, 11);
-	else
-		REG_FLD_MOD(DISPC_CONTROL, enable, 11, 11);
+	mgr_fld_write(channel, DISPC_MGR_FLD_STALLMODE, enable);
 }
 
 static bool _dispc_mgr_size_ok(u16 width, u16 height)
@@ -3450,12 +3441,6 @@ static void dispc_error_worker(struct work_struct *work)
 		DISPC_IRQ_VID3_FIFO_UNDERFLOW,
 	};
 
-	static const unsigned sync_lost_bits[] = {
-		DISPC_IRQ_SYNC_LOST,
-		DISPC_IRQ_SYNC_LOST_DIGIT,
-		DISPC_IRQ_SYNC_LOST2,
-	};
-
 	spin_lock_irqsave(&dispc.irq_lock, flags);
 	errors = dispc.error_irqs;
 	dispc.error_irqs = 0;
@@ -3484,7 +3469,7 @@ static void dispc_error_worker(struct work_struct *work)
 		unsigned bit;
 
 		mgr = omap_dss_get_overlay_manager(i);
-		bit = sync_lost_bits[i];
+		bit = mgr_desc[i].sync_lost_irq;
 
 		if (bit & errors) {
 			struct omap_dss_device *dssdev = mgr->device;
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index ca8382d..2faf913 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -4360,8 +4360,7 @@ static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
 		timings.x_res = dw;
 		timings.y_res = dh;
 
-		irq = dssdev->manager->id = OMAP_DSS_CHANNEL_LCD ?
-			DISPC_IRQ_FRAMEDONE : DISPC_IRQ_FRAMEDONE2;
+		irq = dispc_mgr_get_framedone_irq(dssdev->manager->id);
 
 		r = omap_dispc_register_isr(dsi_framedone_irq_callback,
 			(void *) dssdev, irq);
@@ -4393,8 +4392,7 @@ static void dsi_display_uninit_dispc(struct omap_dss_device *dssdev)
 	if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_CMD_MODE) {
 		u32 irq;
 
-		irq = dssdev->manager->id = OMAP_DSS_CHANNEL_LCD ?
-			DISPC_IRQ_FRAMEDONE : DISPC_IRQ_FRAMEDONE2;
+		irq = dispc_mgr_get_framedone_irq(dssdev->manager->id);
 
 		omap_dispc_unregister_isr(dsi_framedone_irq_callback,
 			(void *) dssdev, irq);
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index dd1092c..df131fc 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -152,6 +152,12 @@ struct dsi_clock_info {
 	u16 lp_clk_div;
 };
 
+struct reg_field {
+	u16 reg;
+	u8 high;
+	u8 low;
+};
+
 struct seq_file;
 struct platform_device;
 
diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index 0cbcde4..bb602a2 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -500,16 +500,12 @@ static int dss_mgr_wait_for_vsync(struct omap_overlay_manager *mgr)
 	if (r)
 		return r;
 
-	if (mgr->device->type = OMAP_DISPLAY_TYPE_VENC) {
+	if (mgr->device->type = OMAP_DISPLAY_TYPE_VENC)
 		irq = DISPC_IRQ_EVSYNC_ODD;
-	} else if (mgr->device->type = OMAP_DISPLAY_TYPE_HDMI) {
+	else if (mgr->device->type = OMAP_DISPLAY_TYPE_HDMI)
 		irq = DISPC_IRQ_EVSYNC_EVEN;
-	} else {
-		if (mgr->id = OMAP_DSS_CHANNEL_LCD)
-			irq = DISPC_IRQ_VSYNC;
-		else
-			irq = DISPC_IRQ_VSYNC2;
-	}
+	else
+		irq = dispc_mgr_get_vsync_irq(mgr->id);
 
 	r = omap_dispc_wait_for_irq_interruptible_timeout(irq, timeout);
 
-- 
1.7.1


^ permalink raw reply related

* [PATCH 0/4] Add LCD3 channel support
From: Chandrabhanu Mahapatra @ 2012-06-28  9:52 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra

Hi everyone!
This following patch series enables LCD3 support which consists of two patches.

The 1st patch
 * adds enum mgr_reg_fields to describe various register fields manipulated for
   LCD support
 * replaces if-else checks for LCD implementation with simpler interface with
   the help of data from struct mgr_desc

The 2nd Patch
 * adds LCD3 channel support
 * updates DISPC registers for LCD3 support

The 3rd patch
 * enables LCD3 manager DSS feature
 * adds enables clock support for LCD3
 * adds IRQs specific to LCD3 manager

The 4th patch
 * enables dispc functions to provide dump functionality for clocks and IRQs
   specific to LCD3

This patch was based on mainline kernel v3.5rc4.

All your comments and suggestions are welcome.

Regards,
Chandrabhanu

Chandrabhanu Mahapatra (4):
  OMAPDSS: Cleanup implementation of LCD channels
  OMAPDSS: Add support for LCD3 channel
  OMAPDSS: Add LCD3 overlay manager and Clock and IRQ support
  OMAPDSS: Add dump and debug support for LCD3

 drivers/video/omap2/dss/dispc.c        |  351 ++++++++++++++++++--------------
 drivers/video/omap2/dss/dispc.h        |   28 +++
 drivers/video/omap2/dss/dsi.c          |    6 +-
 drivers/video/omap2/dss/dss.c          |   12 +-
 drivers/video/omap2/dss/dss.h          |    6 +
 drivers/video/omap2/dss/dss_features.h |    5 +-
 drivers/video/omap2/dss/manager.c      |   16 +-
 drivers/video/omap2/dss/overlay.c      |   14 +-
 include/video/omapdss.h                |    6 +
 9 files changed, 268 insertions(+), 176 deletions(-)


^ permalink raw reply

* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
From: Jassi Brar @ 2012-06-28  9:51 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne,
	patches
In-Reply-To: <1340869729.5037.7.camel@deskari>

On 28 June 2012 13:18, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Wed, 2012-06-27 at 19:35 +0530, jaswinder.singh@linaro.org wrote:
>> From: Jassi Brar <jaswinder.singh@linaro.org>
>>
>> We can easily keep track of latest EDID from the display and hence avoid
>> expensive EDID re-reads over I2C.
>> This could also help some cheapo displays that provide EDID reliably only
>> immediately after asserting HPD and not later.
>> Even with good displays, there is something in OMAPDSS that apparantly
>> messes up DDC occasionally while EDID is being read, giving the
>>   "operation stopped when reading edid" error.
>
> Btw, this is in nitpicking area, but what editor do you use? I find it
> difficult to read text that is not formatted properly =). At least vim
> formats text nicely with its formating commands.
>
Indeed a nitpick :)
I use vim and, iirc, checkpatch.pl gave 0 warning. Perhaps my poor
cmoposition. Please do tell how I could I make it more appealing to
you ?

>> --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
>> +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
>> @@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
>>
>>       hpd = gpio_get_value(ip_data->hpd_gpio);
>>
>> -     if (hpd)
>> +     if (hpd) {
>>               r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
>> -     else
>> +     } else {
>> +             /* Invalidate EDID Cache */
>> +             ip_data->edid_len = 0;
>>               r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
>> +     }
>
> There's a problem with this patch, which leaves a wrong EDID in the
> cache: if you first have the cable connected and hdmi is enabled, you
> then turn off the HDMI display device via sysfs, we do not go to
> hdmi_check_hpd_state at all. The next time hdmi is enabled, we only get
> the plug-in event, and thus EDID cache is never invalidated.
>
If the hdmi cable is not replugged during that period, I don't see why
would you want the EDID invalidated ?

^ permalink raw reply

* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Tomi Valkeinen @ 2012-06-28  7:58 UTC (permalink / raw)
  To: Jassi Brar; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <CAJe_Zhf+Yihu+tJbUxYYoKhW9uvKdEoaVk0v_Goz-oATz+Jncw@mail.gmail.com>

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

On Thu, 2012-06-28 at 13:16 +0530, Jassi Brar wrote:
> On 28 June 2012 12:11, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Wed, 2012-06-27 at 20:23 +0530, Jassi Brar wrote:
> >> On 27 June 2012 13:43, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> >
> >> > I don't like it at all that omapdss disables and enables the panels in
> >> > omapdss's suspend/resume hooks. But I'm not sure how this should work...
> >> > Should panel drivers each have their own suspend/resume hooks, and
> >> > handle it themselves? Or should the call to suspend/resume come from
> >> > upper layers, like omapfb or omapdrm.
> >> >
> >> > I made a prototype patch a few weeks ago to move the suspend to omapfb,
> >> > and it feels better than the current one, but I'm still not sure...
> >> >
> >> IIUC, I have similar opinion.
> >> Each panel having its own suspend/resume sounds like inter-dependency trouble.
> >
> > What do you mean with that? If we just consider omapdss and the panel
> > drivers, I see no dependency trouble. Panels are independent of each
> > other, and omapdss is supposed to handle any locking & refcounting
> > related to multiple panels already, as from omapdss's point of view
> > panel suspend is the same as panel disable.
> >
> > And if we take omapfb/omapdrm into equation, well, in any case it
> > couldn't be any worse than the current one where suspend is handled by
> > omapdss.
> >
> I just anticipate it not trivial keeping  omap_dss_device.state in
> sync with omap_dss_driver.suspend/resume
> when the latter is made system suspend/resume and former still
> affected by hdmi_panel_disable/enable.

This state variable is something I'd like to get rid of. While I think
it could work fine if we added some locking to omap_dss_device, I'd
actually like to get rid of omap_dss_device also. (totally another
issue, let's not go there).

But anyway, the state variable should only be written by the panel
driver, so I don't see a big problem there. The panel's
disable/enable/suspend/resume touch the state, but the panel driver
should protect that with a mutex.

> Perhaps .disable and .suspend would need merging?

Yes, I already have a patch that removes suspend/resume from
omap_dss_device, as they are identical to disable/enable. I haven't
merged it yet, as there's some funny code in omapdrm that breaks after
the change, and I haven't had time to look at it.

> But as I said, it 'sounds' like.  It all may be straight forward - you
> would know better.
> 
> 
> >> I too would prefer suspend/resume propagating from omap-fb/drm, which
> >> imho fits better with the notion of a linux device(omapdss is only
> >> backend). Though I don't have strong feelings about how core then take
> >> various panels up/down optimally.
> >
> > One one hand, I see the combination of omapdss (or the "output" side of
> > omapdss) and a panel as a whole entity. I mean, if you just load omapdss
> > and a panel driver, but no omapfb/omapdrm, you already have a working
> > panel. You don't _need_ omapfb/omapdrm there. And in that sense it'd
> > make sense if the panels did handle their own suspend/resume.
> >
> Well, I would think if omapdss+panels (backend) serve none other
> omapfb/drm, they should simply lie dormant hogging no resources if the
> omapfb/drm driver isn't loaded (if that isn't already the case).

That should be the case.

I need to look more carefully what fb/drm already do. If they already
have good suspend mechanisms and expect to be the ones handling system
suspend, then I guess the choice is quite clear.

 Tomi


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

^ permalink raw reply

* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Jassi Brar @ 2012-06-28  7:58 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340865706.2090.27.camel@lappyti>

On 28 June 2012 12:11, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Wed, 2012-06-27 at 20:23 +0530, Jassi Brar wrote:
>> On 27 June 2012 13:43, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> >
>> > I don't like it at all that omapdss disables and enables the panels in
>> > omapdss's suspend/resume hooks. But I'm not sure how this should work...
>> > Should panel drivers each have their own suspend/resume hooks, and
>> > handle it themselves? Or should the call to suspend/resume come from
>> > upper layers, like omapfb or omapdrm.
>> >
>> > I made a prototype patch a few weeks ago to move the suspend to omapfb,
>> > and it feels better than the current one, but I'm still not sure...
>> >
>> IIUC, I have similar opinion.
>> Each panel having its own suspend/resume sounds like inter-dependency trouble.
>
> What do you mean with that? If we just consider omapdss and the panel
> drivers, I see no dependency trouble. Panels are independent of each
> other, and omapdss is supposed to handle any locking & refcounting
> related to multiple panels already, as from omapdss's point of view
> panel suspend is the same as panel disable.
>
> And if we take omapfb/omapdrm into equation, well, in any case it
> couldn't be any worse than the current one where suspend is handled by
> omapdss.
>
I just anticipate it not trivial keeping  omap_dss_device.state in
sync with omap_dss_driver.suspend/resume
when the latter is made system suspend/resume and former still
affected by hdmi_panel_disable/enable.
Perhaps .disable and .suspend would need merging?

But as I said, it 'sounds' like.  It all may be straight forward - you
would know better.


>> I too would prefer suspend/resume propagating from omap-fb/drm, which
>> imho fits better with the notion of a linux device(omapdss is only
>> backend). Though I don't have strong feelings about how core then take
>> various panels up/down optimally.
>
> One one hand, I see the combination of omapdss (or the "output" side of
> omapdss) and a panel as a whole entity. I mean, if you just load omapdss
> and a panel driver, but no omapfb/omapdrm, you already have a working
> panel. You don't _need_ omapfb/omapdrm there. And in that sense it'd
> make sense if the panels did handle their own suspend/resume.
>
Well, I would think if omapdss+panels (backend) serve none other
omapfb/drm, they should simply lie dormant hogging no resources if the
omapfb/drm driver isn't loaded (if that isn't already the case).
Also one usually has tighter control over a private subsystem by
having only one "point of intervention by system" as compared to two
or more.

> But then, in real life it may be just simpler if omapfb/omapdrm handles
> the suspend.
>
Yeah, being a simpler option is always there.

^ permalink raw reply

* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
From: Tomi Valkeinen @ 2012-06-28  7:48 UTC (permalink / raw)
  To: jaswinder.singh
  Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne,
	patches
In-Reply-To: <1340805944-28805-1-git-send-email-jaswinder.singh@linaro.org>

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

On Wed, 2012-06-27 at 19:35 +0530, jaswinder.singh@linaro.org wrote:
> From: Jassi Brar <jaswinder.singh@linaro.org>
> 
> We can easily keep track of latest EDID from the display and hence avoid
> expensive EDID re-reads over I2C.
> This could also help some cheapo displays that provide EDID reliably only
> immediately after asserting HPD and not later.
> Even with good displays, there is something in OMAPDSS that apparantly
> messes up DDC occasionally while EDID is being read, giving the
>   "operation stopped when reading edid" error.

Btw, this is in nitpicking area, but what editor do you use? I find it
difficult to read text that is not formatted properly =). At least vim
formats text nicely with its formating commands.

> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
>  drivers/video/omap2/dss/hdmi.c            |    1 +
>  drivers/video/omap2/dss/ti_hdmi.h         |    2 ++
>  drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |   15 +++++++++++++--
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index 0738090..9853621 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -758,6 +758,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
>  	hdmi.ip_data.core_av_offset = HDMI_CORE_AV;
>  	hdmi.ip_data.pll_offset = HDMI_PLLCTRL;
>  	hdmi.ip_data.phy_offset = HDMI_PHY;
> +	hdmi.ip_data.edid_len = 0; /* Invalidate EDID Cache */
>  	mutex_init(&hdmi.ip_data.lock);
>  
>  	hdmi_panel_init();
> diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
> index cc292b8..4735860 100644
> --- a/drivers/video/omap2/dss/ti_hdmi.h
> +++ b/drivers/video/omap2/dss/ti_hdmi.h
> @@ -178,6 +178,8 @@ struct hdmi_ip_data {
>  	/* ti_hdmi_4xxx_ip private data. These should be in a separate struct */
>  	int hpd_gpio;
>  	struct mutex lock;
> +	u8 edid_cached[256];
> +	unsigned edid_len;
>  };
>  int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
>  void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
> diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> index 04acca9..b5c3dc4 100644
> --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> @@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
>  
>  	hpd = gpio_get_value(ip_data->hpd_gpio);
>  
> -	if (hpd)
> +	if (hpd) {
>  		r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
> -	else
> +	} else {
> +		/* Invalidate EDID Cache */
> +		ip_data->edid_len = 0;
>  		r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
> +	}

There's a problem with this patch, which leaves a wrong EDID in the
cache: if you first have the cable connected and hdmi is enabled, you
then turn off the HDMI display device via sysfs, we do not go to
hdmi_check_hpd_state at all. The next time hdmi is enabled, we only get
the plug-in event, and thus EDID cache is never invalidated.

Perhaps the EDID cache should be invalidated always in
hdmi_check_hpd_state. In that case I think there's a chance
(theoretical, perhaps), that we get two hpd interrupts, and for both the
hpd gpio reads 1. I'm not quite sure what that would imply (perhaps we
just missed the hpd gpio = 0 case), but even in that case invalidating
the cache sounds ok.

 Tomi


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

^ permalink raw reply

* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Tomi Valkeinen @ 2012-06-28  6:41 UTC (permalink / raw)
  To: Jassi Brar; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <CAJe_ZhdOiv8NgMB37h5_X_reK+urxVykW1tHDF9S50VVUvX=4g@mail.gmail.com>

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

On Wed, 2012-06-27 at 20:23 +0530, Jassi Brar wrote:
> On 27 June 2012 13:43, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >
> > I don't like it at all that omapdss disables and enables the panels in
> > omapdss's suspend/resume hooks. But I'm not sure how this should work...
> > Should panel drivers each have their own suspend/resume hooks, and
> > handle it themselves? Or should the call to suspend/resume come from
> > upper layers, like omapfb or omapdrm.
> >
> > I made a prototype patch a few weeks ago to move the suspend to omapfb,
> > and it feels better than the current one, but I'm still not sure...
> >
> IIUC, I have similar opinion.
> Each panel having its own suspend/resume sounds like inter-dependency trouble.

What do you mean with that? If we just consider omapdss and the panel
drivers, I see no dependency trouble. Panels are independent of each
other, and omapdss is supposed to handle any locking & refcounting
related to multiple panels already, as from omapdss's point of view
panel suspend is the same as panel disable.

And if we take omapfb/omapdrm into equation, well, in any case it
couldn't be any worse than the current one where suspend is handled by
omapdss.

> I too would prefer suspend/resume propagating from omap-fb/drm, which
> imho fits better with the notion of a linux device(omapdss is only
> backend). Though I don't have strong feelings about how core then take
> various panels up/down optimally.

One one hand, I see the combination of omapdss (or the "output" side of
omapdss) and a panel as a whole entity. I mean, if you just load omapdss
and a panel driver, but no omapfb/omapdrm, you already have a working
panel. You don't _need_ omapfb/omapdrm there. And in that sense it'd
make sense if the panels did handle their own suspend/resume.

But then, in real life it may be just simpler if omapfb/omapdrm handles
the suspend.

 Tomi


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

^ permalink raw reply

* [PATCH v2 2/2] video: mxsfb: add simple device tree probe
From: Shawn Guo @ 2012-06-28  3:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1340701379-8619-3-git-send-email-shawn.guo@linaro.org>

Add a simple device tree probe support for mxsfb driver.  Before
a common binding for struct fb_videomode is available, the driver will
keep using struct mxsfb_platform_data.  That said, platform code will
use auxdata to attach mxsfb_platform_data for device tree boot.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
Changes since v1:
* Remove enable-active-low property and use of_get_named_gpio_flags
  instead

 Documentation/devicetree/bindings/fb/mxsfb.txt |   19 ++++++++
 drivers/video/mxsfb.c                          |   60 ++++++++++++++++++-----
 2 files changed, 66 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/fb/mxsfb.txt

diff --git a/Documentation/devicetree/bindings/fb/mxsfb.txt b/Documentation/devicetree/bindings/fb/mxsfb.txt
new file mode 100644
index 0000000..b41e5e52
--- /dev/null
+++ b/Documentation/devicetree/bindings/fb/mxsfb.txt
@@ -0,0 +1,19 @@
+* Freescale MXS LCD Interface (LCDIF)
+
+Required properties:
+- compatible: Should be "fsl,<chip>-lcdif".  Supported chips include
+  imx23 and imx28.
+- reg: Address and length of the register set for lcdif
+- interrupts: Should contain lcdif interrupts
+
+Optional properties:
+- panel-enable-gpios : Should specify the gpio for panel enable
+
+Examples:
+
+lcdif@80030000 {
+	compatible = "fsl,imx28-lcdif";
+	reg = <0x80030000 2000>;
+	interrupts = <38 86>;
+	panel-enable-gpios = <&gpio3 30 0>;
+};
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 08dad8d..49619b4 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -41,6 +41,8 @@
 
 #include <linux/module.h>
 #include <linux/kernel.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
@@ -750,16 +752,43 @@ static void __devexit mxsfb_free_videomem(struct mxsfb_info *host)
 	}
 }
 
+static struct platform_device_id mxsfb_devtype[] = {
+	{
+		.name = "imx23-fb",
+		.driver_data = MXSFB_V3,
+	}, {
+		.name = "imx28-fb",
+		.driver_data = MXSFB_V4,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(platform, mxsfb_devtype);
+
+static const struct of_device_id mxsfb_dt_ids[] = {
+	{ .compatible = "fsl,imx23-lcdif", .data = &mxsfb_devtype[0], },
+	{ .compatible = "fsl,imx28-lcdif", .data = &mxsfb_devtype[1], },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxsfb_dt_ids);
+
 static int __devinit mxsfb_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id +			of_match_device(mxsfb_dt_ids, &pdev->dev);
 	struct mxsfb_platform_data *pdata = pdev->dev.platform_data;
 	struct resource *res;
 	struct mxsfb_info *host;
 	struct fb_info *fb_info;
 	struct fb_modelist *modelist;
 	struct pinctrl *pinctrl;
+	int panel_enable;
+	enum of_gpio_flags flags;
 	int i, ret;
 
+	if (of_id)
+		pdev->id_entry = of_id->data;
+
 	if (!pdata) {
 		dev_err(&pdev->dev, "No platformdata. Giving up\n");
 		return -ENODEV;
@@ -807,6 +836,22 @@ static int __devinit mxsfb_probe(struct platform_device *pdev)
 		goto error_getclock;
 	}
 
+	panel_enable = of_get_named_gpio_flags(pdev->dev.of_node,
+					       "panel-enable-gpios", 0, &flags);
+	if (gpio_is_valid(panel_enable)) {
+		unsigned long f = GPIOF_OUT_INIT_HIGH;
+		if (flags = OF_GPIO_ACTIVE_LOW)
+			f = GPIOF_OUT_INIT_LOW;
+		ret = devm_gpio_request_one(&pdev->dev, panel_enable,
+					    f, "panel-enable");
+		if (ret) {
+			dev_err(&pdev->dev,
+				"failed to request gpio %d: %d\n",
+				panel_enable, ret);
+			goto error_panel_enable;
+		}
+	}
+
 	fb_info->pseudo_palette = kmalloc(sizeof(u32) * 16, GFP_KERNEL);
 	if (!fb_info->pseudo_palette) {
 		ret = -ENOMEM;
@@ -854,6 +899,7 @@ error_register:
 error_init_fb:
 	kfree(fb_info->pseudo_palette);
 error_pseudo_pallette:
+error_panel_enable:
 	clk_put(host->clk);
 error_getclock:
 error_getpin:
@@ -901,19 +947,6 @@ static void mxsfb_shutdown(struct platform_device *pdev)
 	writel(CTRL_RUN, host->base + LCDC_CTRL + REG_CLR);
 }
 
-static struct platform_device_id mxsfb_devtype[] = {
-	{
-		.name = "imx23-fb",
-		.driver_data = MXSFB_V3,
-	}, {
-		.name = "imx28-fb",
-		.driver_data = MXSFB_V4,
-	}, {
-		/* sentinel */
-	}
-};
-MODULE_DEVICE_TABLE(platform, mxsfb_devtype);
-
 static struct platform_driver mxsfb_driver = {
 	.probe = mxsfb_probe,
 	.remove = __devexit_p(mxsfb_remove),
@@ -921,6 +954,7 @@ static struct platform_driver mxsfb_driver = {
 	.id_table = mxsfb_devtype,
 	.driver = {
 		   .name = DRIVER_NAME,
+		   .of_match_table = mxsfb_dt_ids,
 	},
 };
 
-- 
1.7.5.4



^ permalink raw reply related

* Re: [PATCH, RFC] displaymodes in devicetree
From: Sascha Hauer @ 2012-06-27 16:22 UTC (permalink / raw)
  To: Mitch Bradley
  Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart
In-Reply-To: <4FEB31CC.2060705-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org>

On Wed, Jun 27, 2012 at 06:16:12AM -1000, Mitch Bradley wrote:
> On 6/27/2012 2:43 AM, Sascha Hauer wrote:
> >Hi All,
> >
> >I'd like to have a possibility to describe fixed display modes in the
> >devicetree. This topic has been discussed before here:
> >
> >https://lists.ozlabs.org/pipermail/linuxppc-dev/2010-February/080683.html
> >
> >The result at that time was that EDID data should be considered to use
> >as this is a format that already exists. I want to come back to this
> >topic since:
> >
> >- EDID data is a binary format and as such quite inconvenient to handle.
> >   There exist several tools to parse EDID data, but I'm not aware of any
> >   (open source) tool which can generate EDID data.
> >- EDID blobs are hard to modify and hard to review in patches.
> >- EDID is designed to describe multiple modes, but fixed displays
> >   usually only support a single fixed mode.
> >
> >There are several ways of describing the mode, for this patch I chose to
> >use the format (and naming) used by the Linux Framebuffer Layer as this
> >is the only description which does not allow for inconsistent modes. I
> >added the most common flags like [v|h]sync_active_high. Others can be
> >added, but these flags are pretty much agreed upon and won't be
> >superseeded with other flags. The mode from the devicetree can be
> >converted to the most common modes used in Linux, struct fb_videomode
> >and struct drm_display_mode.
> >
> >Comments welcome
> 
> 
> I like the general approach and the set of names.  The separators
> inside the names should be hyphen (-) not underscore (_), following
> the usual device tree convention.  The rationale for that convention
> is to follow natural-language usage, not the identifier constraints
> of programming languages based on algebraic expressions.

Good to hear that this is the convention for devicetree, I like
hyphens better aswell. Will change.

Sascha

-- 
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, RFC] displaymodes in devicetree
From: Mitch Bradley @ 2012-06-27 16:16 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart
In-Reply-To: <20120627124313.GI1623-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

On 6/27/2012 2:43 AM, Sascha Hauer wrote:
> Hi All,
>
> I'd like to have a possibility to describe fixed display modes in the
> devicetree. This topic has been discussed before here:
>
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2010-February/080683.html
>
> The result at that time was that EDID data should be considered to use
> as this is a format that already exists. I want to come back to this
> topic since:
>
> - EDID data is a binary format and as such quite inconvenient to handle.
>    There exist several tools to parse EDID data, but I'm not aware of any
>    (open source) tool which can generate EDID data.
> - EDID blobs are hard to modify and hard to review in patches.
> - EDID is designed to describe multiple modes, but fixed displays
>    usually only support a single fixed mode.
>
> There are several ways of describing the mode, for this patch I chose to
> use the format (and naming) used by the Linux Framebuffer Layer as this
> is the only description which does not allow for inconsistent modes. I
> added the most common flags like [v|h]sync_active_high. Others can be
> added, but these flags are pretty much agreed upon and won't be
> superseeded with other flags. The mode from the devicetree can be
> converted to the most common modes used in Linux, struct fb_videomode
> and struct drm_display_mode.
>
> Comments welcome


I like the general approach and the set of names.  The separators inside 
the names should be hyphen (-) not underscore (_), following the usual 
device tree convention.  The rationale for that convention is to follow 
natural-language usage, not the identifier constraints of programming 
languages based on algebraic expressions.


>
> Sascha
>
>
> 8<--------------------------------------------
>
> of: Add videomode helper
>
> This patch adds a helper function for parsing videomodes from the devicetree.
> The videomode is returned either as a struct drm_display_mode or a
> struct fb_videomode.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>   .../devicetree/bindings/video/displaymode          |   40 ++++++++
>   drivers/of/Kconfig                                 |    5 +
>   drivers/of/Makefile                                |    1 +
>   drivers/of/of_videomode.c                          |  108 ++++++++++++++++++++
>   include/linux/of_videomode.h                       |   19 ++++
>   5 files changed, 173 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/video/displaymode
>   create mode 100644 drivers/of/of_videomode.c
>   create mode 100644 include/linux/of_videomode.h
>
> diff --git a/Documentation/devicetree/bindings/video/displaymode b/Documentation/devicetree/bindings/video/displaymode
> new file mode 100644
> index 0000000..303cfc8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/displaymode
> @@ -0,0 +1,40 @@
> +videomode bindings
> +=========
> +
> +Required properties:
> + - xres, yres: Display resolution
> + - left_margin, right_margin, hsync_len: Horizontal Display timing parameters
> +   in pixels
> +   upper_margin, lower_margin, vsync_len: Vertical display timing parameters in
> +   lines
> + - clock: displayclock in Hz
> +
> +Optional properties:
> + - width_mm, height_mm: Display dimensions in mm
> + - hsync_active_high (bool): Hsync pulse is active high
> + - vsync_active_high (bool): Vsync pulse is active high
> + - interlaced (bool): This is an interlaced mode
> + - doublescan (bool): This is a doublescan mode
> +
> +There are different ways of describing a display mode. The devicetree representation
> +corresponds to the one used by the Linux Framebuffer framework described here in
> +Documentation/fb/framebuffer.txt. This representation has been chosen because it's
> +the only format which does not allow for inconsistent parameters.Unlike the Framebuffer
> +framework the devicetree has the clock in Hz instead of ps.
> +
> +Example:
> +
> +	display@0 {
> +		/* 1920x1080p24 */
> +		clock = <52000000>;
> +		xres = <1920>;
> +		yres = <1080>;
> +		left_margin = <25>;
> +		right_margin = <25>;
> +		hsync_len = <25>;
> +		lower_margin = <2>;
> +		upper_margin = <2>;
> +		vsync_len = <2>;
> +		hsync_active_high;
> +	};
> +
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index dfba3e6..a3acaa3 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -83,4 +83,9 @@ config OF_MTD
>   	depends on MTD
>   	def_bool y
>
> +config OF_VIDEOMODE
> +	def_bool y
> +	help
> +	  helper to parse videomodes from the devicetree
> +
>   endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index e027f44..80e6db3 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO)	+= of_mdio.o
>   obj-$(CONFIG_OF_PCI)	+= of_pci.o
>   obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
>   obj-$(CONFIG_OF_MTD)	+= of_mtd.o
> +obj-$(CONFIG_OF_VIDEOMODE)	+= of_videomode.o
> diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
> new file mode 100644
> index 0000000..859aefb
> --- /dev/null
> +++ b/drivers/of/of_videomode.c
> @@ -0,0 +1,108 @@
> +/*
> + * OF helpers for parsing display modes
> + *
> + * Copyright (c) 2012 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
> + *
> + * This file is released under the GPLv2
> + */
> +#include <linux/of.h>
> +#include <linux/fb.h>
> +#include <linux/export.h>
> +#include <drm/drmP.h>
> +#include <drm/drm_crtc.h>
> +
> +int of_get_video_mode(struct device_node *np, struct drm_display_mode *dmode,
> +		struct fb_videomode *fbmode)
> +{
> +	int ret = 0;
> +	u32 left_margin, xres, right_margin, hsync_len;
> +	u32 upper_margin, yres, lower_margin, vsync_len;
> +	u32 width_mm = 0, height_mm = 0;
> +	u32 clock;
> +	bool hah = false, vah = false, interlaced = false, doublescan = false;
> +
> +	if (!np)
> +		return -EINVAL;
> +
> +	ret |= of_property_read_u32(np, "left_margin", &left_margin);
> +	ret |= of_property_read_u32(np, "xres", &xres);
> +	ret |= of_property_read_u32(np, "right_margin", &right_margin);
> +	ret |= of_property_read_u32(np, "hsync_len", &hsync_len);
> +	ret |= of_property_read_u32(np, "upper_margin", &upper_margin);
> +	ret |= of_property_read_u32(np, "yres", &yres);
> +	ret |= of_property_read_u32(np, "lower_margin", &lower_margin);
> +	ret |= of_property_read_u32(np, "vsync_len", &vsync_len);
> +	ret |= of_property_read_u32(np, "clock", &clock);
> +	if (ret)
> +		return -EINVAL;
> +
> +	of_property_read_u32(np, "width_mm", &width_mm);
> +	of_property_read_u32(np, "height_mm", &height_mm);
> +
> +	hah = of_property_read_bool(np, "hsync_active_high");
> +	vah = of_property_read_bool(np, "vsync_active_high");
> +	interlaced = of_property_read_bool(np, "interlaced");
> +	doublescan = of_property_read_bool(np, "doublescan");
> +
> +	if (dmode) {
> +		memset(dmode, 0, sizeof(*dmode));
> +
> +		dmode->hdisplay = xres;
> +		dmode->hsync_start = xres + right_margin;
> +		dmode->hsync_end = xres + right_margin + hsync_len;
> +		dmode->htotal = xres + right_margin + hsync_len + left_margin;
> +
> +		dmode->vdisplay = yres;
> +		dmode->vsync_start = yres + lower_margin;
> +		dmode->vsync_end = yres + lower_margin + vsync_len;
> +		dmode->vtotal = yres + lower_margin + vsync_len + upper_margin;
> +
> +		dmode->width_mm = width_mm;
> +		dmode->height_mm = height_mm;
> +
> +		dmode->clock = clock / 1000;
> +
> +		if (hah)
> +			dmode->flags |= DRM_MODE_FLAG_PHSYNC;
> +		else
> +			dmode->flags |= DRM_MODE_FLAG_NHSYNC;
> +		if (vah)
> +			dmode->flags |= DRM_MODE_FLAG_PVSYNC;
> +		else
> +			dmode->flags |= DRM_MODE_FLAG_NVSYNC;
> +		if (interlaced)
> +			dmode->flags |= DRM_MODE_FLAG_INTERLACE;
> +		if (doublescan)
> +			dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
> +
> +		drm_mode_set_name(dmode);
> +	}
> +
> +	if (fbmode) {
> +		memset(fbmode, 0, sizeof(*fbmode));
> +
> +		fbmode->xres = xres;
> +		fbmode->left_margin = left_margin;
> +		fbmode->right_margin = right_margin;
> +		fbmode->hsync_len = hsync_len;
> +
> +		fbmode->yres = yres;
> +		fbmode->upper_margin = upper_margin;
> +		fbmode->lower_margin = lower_margin;
> +		fbmode->vsync_len = vsync_len;
> +
> +		fbmode->pixclock = KHZ2PICOS(clock / 1000);
> +
> +		if (hah)
> +			fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> +		if (vah)
> +			fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> +		if (interlaced)
> +			fbmode->vmode |= FB_VMODE_INTERLACED;
> +		if (doublescan)
> +			fbmode->vmode |= FB_VMODE_DOUBLE;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_get_video_mode);
> diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
> new file mode 100644
> index 0000000..a988429
> --- /dev/null
> +++ b/include/linux/of_videomode.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright 2012 Sascha Hauer <s.hauer@pengutronix.de>
> + *
> + * OF helpers for videomodes.
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_OF_VIDEOMODE_H
> +#define __LINUX_OF_VIDEOMODE_H
> +
> +struct device_node;
> +struct fb_videomode;
> +struct drm_display_mode;
> +
> +int of_get_video_mode(struct device_node *np, struct drm_display_mode *dmode,
> +		struct fb_videomode *fbmode);
> +
> +#endif /* __LINUX_OF_VIDEOMODE_H */
>


^ permalink raw reply

* Re: [PATCH 1/2] OMAPDSS: Use PM notifiers for system suspend
From: Jassi Brar @ 2012-06-27 15:32 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1340807697-24847-1-git-send-email-tomi.valkeinen@ti.com>

On 27 June 2012 20:04, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> The current way how omapdss handles system suspend and resume is that
> omapdss device (a platform device, which is not part of the device
> hierarchy of the DSS HW devices, like DISPC and DSI, or panels.) uses
> the suspend and resume callbacks from platform_driver to handle system
> suspend. It does this by disabling all enabled panels on suspend, and
> resuming the previously disabled panels on resume.
>
> This presents a few problems.
>
> One is that as omapdss device is not related to the panel devices or the
> DSS HW devices, there's no ordering in the suspend process. This means
> that suspend could be first ran for DSS HW devices and panels, and only
> then for omapdss device. Currently this is not a problem, as DSS HW
> devices and panels do not handle suspend.
>
> Another, more pressing problem, is that when suspending or resuming, the
> runtime PM functions return -EACCES as runtime PM is disabled during
> system suspend. This causes the driver to print warnings, and operations
> to fail as they think that they failed to bring up the HW.
>
> This patch changes the omapdss suspend handling to use PM notifiers,
> which are called before suspend and after resume. This way we have a
> normally functioning system when we are suspending and resuming the
> panels.
>
> This patch, I believe, creates a problem that somebody could enable or
> disable a panel between PM_SUSPEND_PREPARE and the system suspend, and
> similarly the other way around in resume. I choose to ignore the problem
> for now, as it sounds rather unlikely, and if it happens, it's not
> fatal.
>
> In the long run the system suspend handling of omapdss and panels should
> be thought out properly. The current approach feels rather hacky.
> Perhaps the panel drivers should handle system suspend, or the users of
> omapdss (omapfb, omapdrm) should handle system suspend.
>
> Note that after this patch we could probably revert
> 0eaf9f52e94f756147dbfe1faf1f77a02378dbf9 (OMAPDSS: use sync versions of
> pm_runtime_put).
>
I would think only DISPC should need the sync version.

> But as I said, this patch may be temporary, so let's
> leave the sync version still in place.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Reported-by: Jassi Brar <jaswinder.singh@linaro.org>
>
Please feel free to add

  Tested-by: Jassi Brar <jaswinder.singh@linaro.org>

^ permalink raw reply

* Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state
From: Jassi Brar @ 2012-06-27 14:56 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne
In-Reply-To: <1340784821.2649.17.camel@deskari>

On 27 June 2012 13:43, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> I don't like it at all that omapdss disables and enables the panels in
> omapdss's suspend/resume hooks. But I'm not sure how this should work...
> Should panel drivers each have their own suspend/resume hooks, and
> handle it themselves? Or should the call to suspend/resume come from
> upper layers, like omapfb or omapdrm.
>
> I made a prototype patch a few weeks ago to move the suspend to omapfb,
> and it feels better than the current one, but I'm still not sure...
>
IIUC, I have similar opinion.
Each panel having its own suspend/resume sounds like inter-dependency trouble.
I too would prefer suspend/resume propagating from omap-fb/drm, which
imho fits better with the notion of a linux device(omapdss is only
backend). Though I don't have strong feelings about how core then take
various panels up/down optimally.

BR.

^ permalink raw reply

* [PATCH 2/2] OMAPDSS: fix warnings if CONFIG_PM_RUNTIME=n
From: Tomi Valkeinen @ 2012-06-27 14:34 UTC (permalink / raw)
  To: linux-omap, linux-fbdev, Jassi Brar; +Cc: Tomi Valkeinen, Grazvydas Ignotas
In-Reply-To: <1340807697-24847-1-git-send-email-tomi.valkeinen@ti.com>

If runtime PM is not enabled in the kernel config, pm_runtime_get_sync()
will always return 1 and pm_runtime_put_sync() will always return
-ENOSYS. pm_runtime_get_sync() returning 1 presents no problem to the
driver, but -ENOSYS from pm_runtime_put_sync() causes the driver to
print a warning.

One option would be to ignore errors returned by pm_runtime_put_sync()
totally, as they only say that the call was unable to put the hardware
into suspend mode.

However, I chose to ignore the returned -ENOSYS explicitly, and print a
warning for other errors, as I think we should get notified if the HW
failed to go to suspend properly.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Jassi Brar <jaswinder.singh@linaro.org>
Cc: Grazvydas Ignotas <notasas@gmail.com>
---
 drivers/video/omap2/dss/dispc.c |    2 +-
 drivers/video/omap2/dss/dsi.c   |    2 +-
 drivers/video/omap2/dss/dss.c   |    2 +-
 drivers/video/omap2/dss/hdmi.c  |    2 +-
 drivers/video/omap2/dss/rfbi.c  |    2 +-
 drivers/video/omap2/dss/venc.c  |    2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 4749ac3..397d4ee 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -384,7 +384,7 @@ void dispc_runtime_put(void)
 	DSSDBG("dispc_runtime_put\n");
 
 	r = pm_runtime_put_sync(&dispc.pdev->dev);
-	WARN_ON(r < 0);
+	WARN_ON(r < 0 && r != -ENOSYS);
 }
 
 static inline bool dispc_mgr_is_lcd(enum omap_channel channel)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 2a0055e..084906c 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -1075,7 +1075,7 @@ void dsi_runtime_put(struct platform_device *dsidev)
 	DSSDBG("dsi_runtime_put\n");
 
 	r = pm_runtime_put_sync(&dsi->pdev->dev);
-	WARN_ON(r < 0);
+	WARN_ON(r < 0 && r != -ENOSYS);
 }
 
 /* source clock for DSI PLL. this could also be PCLKFREE */
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 7706323..d2b5719 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -731,7 +731,7 @@ static void dss_runtime_put(void)
 	DSSDBG("dss_runtime_put\n");
 
 	r = pm_runtime_put_sync(&dss.pdev->dev);
-	WARN_ON(r < 0 && r != -EBUSY);
+	WARN_ON(r < 0 && r != -ENOSYS && r != -EBUSY);
 }
 
 /* DEBUGFS */
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index de6920b..ae1688d 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -138,7 +138,7 @@ static void hdmi_runtime_put(void)
 	DSSDBG("hdmi_runtime_put\n");
 
 	r = pm_runtime_put_sync(&hdmi.pdev->dev);
-	WARN_ON(r < 0);
+	WARN_ON(r < 0 && r != -ENOSYS);
 }
 
 static int __init hdmi_init_display(struct omap_dss_device *dssdev)
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index 3d8c206..7985fa1 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -141,7 +141,7 @@ static void rfbi_runtime_put(void)
 	DSSDBG("rfbi_runtime_put\n");
 
 	r = pm_runtime_put_sync(&rfbi.pdev->dev);
-	WARN_ON(r < 0);
+	WARN_ON(r < 0 && r != -ENOSYS);
 }
 
 void rfbi_bus_lock(void)
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index fd37440..416d478 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -402,7 +402,7 @@ static void venc_runtime_put(void)
 	DSSDBG("venc_runtime_put\n");
 
 	r = pm_runtime_put_sync(&venc.pdev->dev);
-	WARN_ON(r < 0);
+	WARN_ON(r < 0 && r != -ENOSYS);
 }
 
 static const struct venc_config *venc_timings_to_config(
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 1/2] OMAPDSS: Use PM notifiers for system suspend
From: Tomi Valkeinen @ 2012-06-27 14:34 UTC (permalink / raw)
  To: linux-omap, linux-fbdev, Jassi Brar; +Cc: Tomi Valkeinen

The current way how omapdss handles system suspend and resume is that
omapdss device (a platform device, which is not part of the device
hierarchy of the DSS HW devices, like DISPC and DSI, or panels.) uses
the suspend and resume callbacks from platform_driver to handle system
suspend. It does this by disabling all enabled panels on suspend, and
resuming the previously disabled panels on resume.

This presents a few problems.

One is that as omapdss device is not related to the panel devices or the
DSS HW devices, there's no ordering in the suspend process. This means
that suspend could be first ran for DSS HW devices and panels, and only
then for omapdss device. Currently this is not a problem, as DSS HW
devices and panels do not handle suspend.

Another, more pressing problem, is that when suspending or resuming, the
runtime PM functions return -EACCES as runtime PM is disabled during
system suspend. This causes the driver to print warnings, and operations
to fail as they think that they failed to bring up the HW.

This patch changes the omapdss suspend handling to use PM notifiers,
which are called before suspend and after resume. This way we have a
normally functioning system when we are suspending and resuming the
panels.

This patch, I believe, creates a problem that somebody could enable or
disable a panel between PM_SUSPEND_PREPARE and the system suspend, and
similarly the other way around in resume. I choose to ignore the problem
for now, as it sounds rather unlikely, and if it happens, it's not
fatal.

In the long run the system suspend handling of omapdss and panels should
be thought out properly. The current approach feels rather hacky.
Perhaps the panel drivers should handle system suspend, or the users of
omapdss (omapfb, omapdrm) should handle system suspend.

Note that after this patch we could probably revert
0eaf9f52e94f756147dbfe1faf1f77a02378dbf9 (OMAPDSS: use sync versions of
pm_runtime_put). But as I said, this patch may be temporary, so let's
leave the sync version still in place.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reported-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 drivers/video/omap2/dss/core.c |   45 ++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index d9b0bf3..e4fadf9 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -32,6 +32,7 @@
 #include <linux/io.h>
 #include <linux/device.h>
 #include <linux/regulator/consumer.h>
+#include <linux/suspend.h>
 
 #include <video/omapdss.h>
 
@@ -207,6 +208,30 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
 #endif /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUG_SUPPORT */
 
 /* PLATFORM DEVICE */
+static int omap_dss_pm_notif(struct notifier_block *b, unsigned long v, void *d)
+{
+	DSSDBG("pm notif %lu\n", v);
+
+	switch (v)
+	{
+	case PM_SUSPEND_PREPARE:
+		DSSDBG("suspending displays\n");
+		return dss_suspend_all_devices();
+
+	case PM_POST_SUSPEND:
+		DSSDBG("resuming displays\n");
+		return dss_resume_all_devices();
+
+	default:
+		return 0;
+	}
+}
+
+static struct notifier_block omap_dss_pm_notif_block +{
+	.notifier_call = omap_dss_pm_notif,
+};
+
 static int __init omap_dss_probe(struct platform_device *pdev)
 {
 	struct omap_dss_board_info *pdata = pdev->dev.platform_data;
@@ -230,6 +255,8 @@ static int __init omap_dss_probe(struct platform_device *pdev)
 	else if (pdata->default_device)
 		core.default_display_name = pdata->default_device->name;
 
+	register_pm_notifier(&omap_dss_pm_notif_block);
+
 	return 0;
 
 err_debugfs:
@@ -239,6 +266,8 @@ err_debugfs:
 
 static int omap_dss_remove(struct platform_device *pdev)
 {
+	unregister_pm_notifier(&omap_dss_pm_notif_block);
+
 	dss_uninitialize_debugfs();
 
 	dss_uninit_overlays(pdev);
@@ -253,25 +282,9 @@ static void omap_dss_shutdown(struct platform_device *pdev)
 	dss_disable_all_devices();
 }
 
-static int omap_dss_suspend(struct platform_device *pdev, pm_message_t state)
-{
-	DSSDBG("suspend %d\n", state.event);
-
-	return dss_suspend_all_devices();
-}
-
-static int omap_dss_resume(struct platform_device *pdev)
-{
-	DSSDBG("resume\n");
-
-	return dss_resume_all_devices();
-}
-
 static struct platform_driver omap_dss_driver = {
 	.remove         = omap_dss_remove,
 	.shutdown	= omap_dss_shutdown,
-	.suspend	= omap_dss_suspend,
-	.resume		= omap_dss_resume,
 	.driver         = {
 		.name   = "omapdss",
 		.owner  = THIS_MODULE,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
From: jaswinder.singh @ 2012-06-27 14:17 UTC (permalink / raw)
  To: tomi.valkeinen, mythripk
  Cc: linux-omap, linux-fbdev, andy.green, n-dechesne, patches,
	Jassi Brar

From: Jassi Brar <jaswinder.singh@linaro.org>

We can easily keep track of latest EDID from the display and hence avoid
expensive EDID re-reads over I2C.
This could also help some cheapo displays that provide EDID reliably only
immediately after asserting HPD and not later.
Even with good displays, there is something in OMAPDSS that apparantly
messes up DDC occasionally while EDID is being read, giving the
  "operation stopped when reading edid" error.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 drivers/video/omap2/dss/hdmi.c            |    1 +
 drivers/video/omap2/dss/ti_hdmi.h         |    2 ++
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |   15 +++++++++++++--
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 0738090..9853621 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -758,6 +758,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
 	hdmi.ip_data.core_av_offset = HDMI_CORE_AV;
 	hdmi.ip_data.pll_offset = HDMI_PLLCTRL;
 	hdmi.ip_data.phy_offset = HDMI_PHY;
+	hdmi.ip_data.edid_len = 0; /* Invalidate EDID Cache */
 	mutex_init(&hdmi.ip_data.lock);
 
 	hdmi_panel_init();
diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
index cc292b8..4735860 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++ b/drivers/video/omap2/dss/ti_hdmi.h
@@ -178,6 +178,8 @@ struct hdmi_ip_data {
 	/* ti_hdmi_4xxx_ip private data. These should be in a separate struct */
 	int hpd_gpio;
 	struct mutex lock;
+	u8 edid_cached[256];
+	unsigned edid_len;
 };
 int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
 void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index 04acca9..b5c3dc4 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
 
 	hpd = gpio_get_value(ip_data->hpd_gpio);
 
-	if (hpd)
+	if (hpd) {
 		r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
-	else
+	} else {
+		/* Invalidate EDID Cache */
+		ip_data->edid_len = 0;
 		r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
+	}
 
 	if (r) {
 		DSSERR("Failed to %s PHY TX power\n",
@@ -454,6 +457,11 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
 {
 	int r, l;
 
+	if (ip_data->edid_len) {
+		memcpy(edid, ip_data->edid_cached, ip_data->edid_len);
+		return ip_data->edid_len;
+	}
+
 	if (len < 128)
 		return -EINVAL;
 
@@ -474,6 +482,9 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
 		l += 128;
 	}
 
+	ip_data->edid_len = l;
+	memcpy(ip_data->edid_cached, edid, l);
+
 	return l;
 }
 
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 2/3] OMAPDSS: HDMI: Replace spinlock with mutex in hdmi_check_hpd_state
From: jaswinder.singh @ 2012-06-27 14:16 UTC (permalink / raw)
  To: tomi.valkeinen, mythripk
  Cc: linux-omap, linux-fbdev, andy.green, n-dechesne, patches,
	Jassi Brar

From: Jassi Brar <jaswinder.singh@linaro.org>

State change of HDMI PHY could potentially take many millisecs, we can do
better by protecting things in hdmi_set_phy_pwr() with a mutex rather than
a spin_lock_irqsave.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 drivers/video/omap2/dss/hdmi.c            |    1 +
 drivers/video/omap2/dss/ti_hdmi.h         |    1 +
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |    6 ++----
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 8195c71..0738090 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -758,6 +758,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
 	hdmi.ip_data.core_av_offset = HDMI_CORE_AV;
 	hdmi.ip_data.pll_offset = HDMI_PLLCTRL;
 	hdmi.ip_data.phy_offset = HDMI_PHY;
+	mutex_init(&hdmi.ip_data.lock);
 
 	hdmi_panel_init();
 
diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
index d174ca1..cc292b8 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++ b/drivers/video/omap2/dss/ti_hdmi.h
@@ -177,6 +177,7 @@ struct hdmi_ip_data {
 
 	/* ti_hdmi_4xxx_ip private data. These should be in a separate struct */
 	int hpd_gpio;
+	struct mutex lock;
 };
 int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
 void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index 3fa3d98..04acca9 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -238,10 +238,8 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
 	unsigned long flags;
 	bool hpd;
 	int r;
-	/* this should be in ti_hdmi_4xxx_ip private data */
-	static DEFINE_SPINLOCK(phy_tx_lock);
 
-	spin_lock_irqsave(&phy_tx_lock, flags);
+	mutex_lock(&ip_data->lock);
 
 	hpd = gpio_get_value(ip_data->hpd_gpio);
 
@@ -257,7 +255,7 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
 	}
 
 err:
-	spin_unlock_irqrestore(&phy_tx_lock, flags);
+	mutex_unlock(&ip_data->lock);
 	return r;
 }
 
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 1/3] OMAPDSS: HDMI: Discard phy_tx_enabled member
From: jaswinder.singh @ 2012-06-27 14:16 UTC (permalink / raw)
  To: tomi.valkeinen, mythripk
  Cc: linux-omap, linux-fbdev, andy.green, n-dechesne, patches,
	Jassi Brar

From: Jassi Brar <jaswinder.singh@linaro.org>

It is simpler to read the current status from a register as compared
to maintaining a state variable to hold the information.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 drivers/video/omap2/dss/ti_hdmi.h         |    1 -
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |   11 ++++-------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
index e734cb4..d174ca1 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++ b/drivers/video/omap2/dss/ti_hdmi.h
@@ -177,7 +177,6 @@ struct hdmi_ip_data {
 
 	/* ti_hdmi_4xxx_ip private data. These should be in a separate struct */
 	int hpd_gpio;
-	bool phy_tx_enabled;
 };
 int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
 void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index 4dae1b2..3fa3d98 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -157,6 +157,10 @@ static int hdmi_pll_init(struct hdmi_ip_data *ip_data)
 /* PHY_PWR_CMD */
 static int hdmi_set_phy_pwr(struct hdmi_ip_data *ip_data, enum hdmi_phy_pwr val)
 {
+	/* Return if already the state */
+	if (REG_GET(hdmi_wp_base(ip_data), HDMI_WP_PWR_CTRL, 5, 4) = val)
+		return 0;
+
 	/* Command for power control of HDMI PHY */
 	REG_FLD_MOD(hdmi_wp_base(ip_data), HDMI_WP_PWR_CTRL, val, 7, 6);
 
@@ -241,11 +245,6 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
 
 	hpd = gpio_get_value(ip_data->hpd_gpio);
 
-	if (hpd = ip_data->phy_tx_enabled) {
-		spin_unlock_irqrestore(&phy_tx_lock, flags);
-		return 0;
-	}
-
 	if (hpd)
 		r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
 	else
@@ -257,7 +256,6 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)
 		goto err;
 	}
 
-	ip_data->phy_tx_enabled = hpd;
 err:
 	spin_unlock_irqrestore(&phy_tx_lock, flags);
 	return r;
@@ -327,7 +325,6 @@ void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data)
 	free_irq(gpio_to_irq(ip_data->hpd_gpio), ip_data);
 
 	hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_OFF);
-	ip_data->phy_tx_enabled = false;
 }
 
 static int hdmi_core_ddc_init(struct hdmi_ip_data *ip_data)
-- 
1.7.4.1


^ permalink raw reply related

* Re: [PATCH 0/2] simple device tree support for mxsfb
From: Shawn Guo @ 2012-06-27 13:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1340701379-8619-1-git-send-email-shawn.guo@linaro.org>

Hi Florian,

I have a mxs platform patch depends on these two patches to covert
a few board files over to device tree.  I would like to have the patches
go via arm-soc tree if they look good.  Please let me know if you have
problem with the patches go through arm-soc tree.  Thanks.

Regards,
Shawn

On Tue, Jun 26, 2012 at 05:02:57PM +0800, Shawn Guo wrote:
> It adds a simple device tree support for mxsfb driver.
> 
> Shawn Guo (2):
>   video: mxsfb: move mxsfb.h into include/linux
>   video: mxsfb: add simple device tree probe
> 
>  Documentation/devicetree/bindings/fb/mxsfb.txt     |   21 +++++++
>  arch/arm/mach-mxs/devices-mx23.h                   |    2 +-
>  arch/arm/mach-mxs/devices-mx28.h                   |    2 +-
>  arch/arm/mach-mxs/devices/platform-mxsfb.c         |    2 +-
>  drivers/video/mxsfb.c                              |   62 +++++++++++++++-----
>  .../include/mach => include/linux}/mxsfb.h         |    6 +-
>  6 files changed, 75 insertions(+), 20 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/fb/mxsfb.txt
>  rename {arch/arm/mach-mxs/include/mach => include/linux}/mxsfb.h (95%)
> 
> -- 
> 1.7.5.4
> 
> 


^ permalink raw reply

* Re: [PATCH 05/17] OMAPDSS: Add some new fields to omap_video_timings
From: Tomi Valkeinen @ 2012-06-27 13:02 UTC (permalink / raw)
  To: Archit Taneja; +Cc: Archit Taneja, rob, linux-fbdev, linux-omap
In-Reply-To: <4FEB00CB.4070502@ti.com>

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

On Wed, 2012-06-27 at 18:17 +0530, Archit Taneja wrote:
> On Wednesday 27 June 2012 06:12 PM, Tomi Valkeinen wrote:
> > On Wed, 2012-06-27 at 17:56 +0530, Archit Taneja wrote:
> >> On Wednesday 27 June 2012 05:18 PM, Tomi Valkeinen wrote:
> >>> On Tue, 2012-06-26 at 15:06 +0530, Archit Taneja wrote:
> >>>> Some panel timing related fields are contained in omap_panel_config in the form
> >>>> of flags. The fields are:
> >>>>
> >>>> - Hsync logic level
> >>>> - Vsync logic level
> >>>> - Data driven on rising/falling edge of pixel clock
> >>>> - Output enable/Data enable logic level
> >>>> - HSYNC/VSYNC driven on rising/falling edge of pixel clock
> >>>>
> >>>> Out of these parameters, Hsync and Vsync logic levels are a part of the timings
> >>>> in the Xorg modeline configuration. So it makes sense to move the to
> >>>> omap_video_timings. The rest aren't a part of modeline, but it still makes
> >>>> sense to move these since they are related to panel timings.
> >>>>
> >>>> These fields stored in omap_panel_config in dssdev are configured for LCD
> >>>> panels, and the corresponding LCD managers in the DISPC_POL_FREQo registers.
> >>>>
> >>>> Add the above fields in omap_video_timings. Represent their state via new enums.
> >>>> The parameter pclk_edge is configured via omap_dss_signal_level, however it
> >>>> actually configures whether data is driven on the rising or falling edge. This
> >>>> is a bit unclean, but it prevents us from creating another enum.
> >>>
> >>> Hmm, why can't omap_dss_signal_edge be used for pclk_edge? I think it'd
> >>> fit fine, except OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES would be an illegal
> >>> value for it.
> >>
> >> I think my paragraph is a bit misleading. The issue is more about the
> >> default value. For hsync_vsync_edge(which programs ONOFF and RF), the
> >> default value is OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES, and is the case for
> >> most panels. But for pclk_edge, the value for most panels is
> >> OMAPDSS_DRIVE_SIG_ACTIVE_HIGH.
> >>
> >> So if I use the same enum for both, I'll need to populate either
> >> pclk_edge or hsync_vsync_edge for almost every panel. With my approach,
> >> I only need to populate these fields for panels having the non-default
> >> requirements. The negative thing is that it's a bit misleading to
> >> represent pclk_edge with the omap_dss_signal_level enum.
> >
> > Ah, I see.
> >
> > I really think using the ACTIVE_LOW/HIGH for pclk_edge is quite ugly =).
> >
> > Well, one option is to add new entry for the enums, "UNDEFINED" or
> > perhaps "DEFAULT", which would have value 0. Then omapdss would know
> > that it should use the default value, whatever that is.
> >
> > Another option is to have all panels define the pclk_edge, so that
> > there's no default value needed.
> 
> I think I'll go with this.

Ok. But if you do this, then perhaps you should fill all the other
properties also. It looks a bit strange to require only one particular
to be always entered, while the others have default values.

But I guess that's not a bad thing at all. If the panel doesn't define
what kind of signal it wants, and trusts that the defaults are always
the same, it may break later or perhaps with other platforms.

So I think the panel drivers should define all the signal properties,
and not leave them to default values. That's the safest option, although
a bit verbose.

 Tomi


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

^ permalink raw reply

* Re: [PATCH 05/17] OMAPDSS: Add some new fields to omap_video_timings
From: Archit Taneja @ 2012-06-27 12:59 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Archit Taneja, rob, linux-fbdev, linux-omap
In-Reply-To: <1340800933.2649.72.camel@deskari>

On Wednesday 27 June 2012 06:12 PM, Tomi Valkeinen wrote:
> On Wed, 2012-06-27 at 17:56 +0530, Archit Taneja wrote:
>> On Wednesday 27 June 2012 05:18 PM, Tomi Valkeinen wrote:
>>> On Tue, 2012-06-26 at 15:06 +0530, Archit Taneja wrote:
>>>> Some panel timing related fields are contained in omap_panel_config in the form
>>>> of flags. The fields are:
>>>>
>>>> - Hsync logic level
>>>> - Vsync logic level
>>>> - Data driven on rising/falling edge of pixel clock
>>>> - Output enable/Data enable logic level
>>>> - HSYNC/VSYNC driven on rising/falling edge of pixel clock
>>>>
>>>> Out of these parameters, Hsync and Vsync logic levels are a part of the timings
>>>> in the Xorg modeline configuration. So it makes sense to move the to
>>>> omap_video_timings. The rest aren't a part of modeline, but it still makes
>>>> sense to move these since they are related to panel timings.
>>>>
>>>> These fields stored in omap_panel_config in dssdev are configured for LCD
>>>> panels, and the corresponding LCD managers in the DISPC_POL_FREQo registers.
>>>>
>>>> Add the above fields in omap_video_timings. Represent their state via new enums.
>>>> The parameter pclk_edge is configured via omap_dss_signal_level, however it
>>>> actually configures whether data is driven on the rising or falling edge. This
>>>> is a bit unclean, but it prevents us from creating another enum.
>>>
>>> Hmm, why can't omap_dss_signal_edge be used for pclk_edge? I think it'd
>>> fit fine, except OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES would be an illegal
>>> value for it.
>>
>> I think my paragraph is a bit misleading. The issue is more about the
>> default value. For hsync_vsync_edge(which programs ONOFF and RF), the
>> default value is OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES, and is the case for
>> most panels. But for pclk_edge, the value for most panels is
>> OMAPDSS_DRIVE_SIG_ACTIVE_HIGH.
>>
>> So if I use the same enum for both, I'll need to populate either
>> pclk_edge or hsync_vsync_edge for almost every panel. With my approach,
>> I only need to populate these fields for panels having the non-default
>> requirements. The negative thing is that it's a bit misleading to
>> represent pclk_edge with the omap_dss_signal_level enum.
>
> Ah, I see.
>
> I really think using the ACTIVE_LOW/HIGH for pclk_edge is quite ugly =).
>
> Well, one option is to add new entry for the enums, "UNDEFINED" or
> perhaps "DEFAULT", which would have value 0. Then omapdss would know
> that it should use the default value, whatever that is.
>
> Another option is to have all panels define the pclk_edge, so that
> there's no default value needed.

I think I'll go with this.

>
> Also, related thing, I see you're writing enum values directly to the
> registers. If you do that, it'd be good to explicitly set the number
> value of the enums. Otherwise it's quite easy to add a new enum value
> later between the old ones, breaking everything (perhaps not a problem
> here, though).

Yes, that's true. I'll set the numbers anyway, for clarity.

Archit

^ permalink raw reply

* [PATCH, RFC] displaymodes in devicetree
From: Sascha Hauer @ 2012-06-27 12:43 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ

Hi All,

I'd like to have a possibility to describe fixed display modes in the
devicetree. This topic has been discussed before here:

https://lists.ozlabs.org/pipermail/linuxppc-dev/2010-February/080683.html

The result at that time was that EDID data should be considered to use
as this is a format that already exists. I want to come back to this
topic since:

- EDID data is a binary format and as such quite inconvenient to handle.
  There exist several tools to parse EDID data, but I'm not aware of any
  (open source) tool which can generate EDID data.
- EDID blobs are hard to modify and hard to review in patches.
- EDID is designed to describe multiple modes, but fixed displays
  usually only support a single fixed mode.

There are several ways of describing the mode, for this patch I chose to
use the format (and naming) used by the Linux Framebuffer Layer as this
is the only description which does not allow for inconsistent modes. I
added the most common flags like [v|h]sync_active_high. Others can be
added, but these flags are pretty much agreed upon and won't be
superseeded with other flags. The mode from the devicetree can be
converted to the most common modes used in Linux, struct fb_videomode
and struct drm_display_mode.

Comments welcome

Sascha


8<--------------------------------------------

of: Add videomode helper

This patch adds a helper function for parsing videomodes from the devicetree.
The videomode is returned either as a struct drm_display_mode or a
struct fb_videomode.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../devicetree/bindings/video/displaymode          |   40 ++++++++
 drivers/of/Kconfig                                 |    5 +
 drivers/of/Makefile                                |    1 +
 drivers/of/of_videomode.c                          |  108 ++++++++++++++++++++
 include/linux/of_videomode.h                       |   19 ++++
 5 files changed, 173 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/displaymode
 create mode 100644 drivers/of/of_videomode.c
 create mode 100644 include/linux/of_videomode.h

diff --git a/Documentation/devicetree/bindings/video/displaymode b/Documentation/devicetree/bindings/video/displaymode
new file mode 100644
index 0000000..303cfc8
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/displaymode
@@ -0,0 +1,40 @@
+videomode bindings
+=========
+
+Required properties:
+ - xres, yres: Display resolution
+ - left_margin, right_margin, hsync_len: Horizontal Display timing parameters
+   in pixels
+   upper_margin, lower_margin, vsync_len: Vertical display timing parameters in
+   lines
+ - clock: displayclock in Hz
+
+Optional properties:
+ - width_mm, height_mm: Display dimensions in mm
+ - hsync_active_high (bool): Hsync pulse is active high
+ - vsync_active_high (bool): Vsync pulse is active high
+ - interlaced (bool): This is an interlaced mode
+ - doublescan (bool): This is a doublescan mode
+
+There are different ways of describing a display mode. The devicetree representation
+corresponds to the one used by the Linux Framebuffer framework described here in
+Documentation/fb/framebuffer.txt. This representation has been chosen because it's
+the only format which does not allow for inconsistent parameters.Unlike the Framebuffer
+framework the devicetree has the clock in Hz instead of ps.
+
+Example:
+
+	display@0 {
+		/* 1920x1080p24 */
+		clock = <52000000>;
+		xres = <1920>;
+		yres = <1080>;
+		left_margin = <25>;
+		right_margin = <25>;
+		hsync_len = <25>;
+		lower_margin = <2>;
+		upper_margin = <2>;
+		vsync_len = <2>;
+		hsync_active_high;
+	};
+
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index dfba3e6..a3acaa3 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -83,4 +83,9 @@ config OF_MTD
 	depends on MTD
 	def_bool y
 
+config OF_VIDEOMODE
+	def_bool y
+	help
+	  helper to parse videomodes from the devicetree
+
 endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index e027f44..80e6db3 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO)	+= of_mdio.o
 obj-$(CONFIG_OF_PCI)	+= of_pci.o
 obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
 obj-$(CONFIG_OF_MTD)	+= of_mtd.o
+obj-$(CONFIG_OF_VIDEOMODE)	+= of_videomode.o
diff --git a/drivers/of/of_videomode.c b/drivers/of/of_videomode.c
new file mode 100644
index 0000000..859aefb
--- /dev/null
+++ b/drivers/of/of_videomode.c
@@ -0,0 +1,108 @@
+/*
+ * OF helpers for parsing display modes
+ *
+ * Copyright (c) 2012 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/of.h>
+#include <linux/fb.h>
+#include <linux/export.h>
+#include <drm/drmP.h>
+#include <drm/drm_crtc.h>
+
+int of_get_video_mode(struct device_node *np, struct drm_display_mode *dmode,
+		struct fb_videomode *fbmode)
+{
+	int ret = 0;
+	u32 left_margin, xres, right_margin, hsync_len;
+	u32 upper_margin, yres, lower_margin, vsync_len;
+	u32 width_mm = 0, height_mm = 0;
+	u32 clock;
+	bool hah = false, vah = false, interlaced = false, doublescan = false;
+
+	if (!np)
+		return -EINVAL;
+
+	ret |= of_property_read_u32(np, "left_margin", &left_margin);
+	ret |= of_property_read_u32(np, "xres", &xres);
+	ret |= of_property_read_u32(np, "right_margin", &right_margin);
+	ret |= of_property_read_u32(np, "hsync_len", &hsync_len);
+	ret |= of_property_read_u32(np, "upper_margin", &upper_margin);
+	ret |= of_property_read_u32(np, "yres", &yres);
+	ret |= of_property_read_u32(np, "lower_margin", &lower_margin);
+	ret |= of_property_read_u32(np, "vsync_len", &vsync_len);
+	ret |= of_property_read_u32(np, "clock", &clock);
+	if (ret)
+		return -EINVAL;
+
+	of_property_read_u32(np, "width_mm", &width_mm);
+	of_property_read_u32(np, "height_mm", &height_mm);
+
+	hah = of_property_read_bool(np, "hsync_active_high");
+	vah = of_property_read_bool(np, "vsync_active_high");
+	interlaced = of_property_read_bool(np, "interlaced");
+	doublescan = of_property_read_bool(np, "doublescan");
+
+	if (dmode) {
+		memset(dmode, 0, sizeof(*dmode));
+
+		dmode->hdisplay = xres;
+		dmode->hsync_start = xres + right_margin;
+		dmode->hsync_end = xres + right_margin + hsync_len;
+		dmode->htotal = xres + right_margin + hsync_len + left_margin;
+
+		dmode->vdisplay = yres;
+		dmode->vsync_start = yres + lower_margin;
+		dmode->vsync_end = yres + lower_margin + vsync_len;
+		dmode->vtotal = yres + lower_margin + vsync_len + upper_margin;
+
+		dmode->width_mm = width_mm;
+		dmode->height_mm = height_mm;
+
+		dmode->clock = clock / 1000;
+
+		if (hah)
+			dmode->flags |= DRM_MODE_FLAG_PHSYNC;
+		else
+			dmode->flags |= DRM_MODE_FLAG_NHSYNC;
+		if (vah)
+			dmode->flags |= DRM_MODE_FLAG_PVSYNC;
+		else
+			dmode->flags |= DRM_MODE_FLAG_NVSYNC;
+		if (interlaced)
+			dmode->flags |= DRM_MODE_FLAG_INTERLACE;
+		if (doublescan)
+			dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
+
+		drm_mode_set_name(dmode);
+	}
+
+	if (fbmode) {
+		memset(fbmode, 0, sizeof(*fbmode));
+
+		fbmode->xres = xres;
+		fbmode->left_margin = left_margin;
+		fbmode->right_margin = right_margin;
+		fbmode->hsync_len = hsync_len;
+
+		fbmode->yres = yres;
+		fbmode->upper_margin = upper_margin;
+		fbmode->lower_margin = lower_margin;
+		fbmode->vsync_len = vsync_len;
+
+		fbmode->pixclock = KHZ2PICOS(clock / 1000);
+
+		if (hah)
+			fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
+		if (vah)
+			fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+		if (interlaced)
+			fbmode->vmode |= FB_VMODE_INTERLACED;
+		if (doublescan)
+			fbmode->vmode |= FB_VMODE_DOUBLE;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_video_mode);
diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
new file mode 100644
index 0000000..a988429
--- /dev/null
+++ b/include/linux/of_videomode.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright 2012 Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * OF helpers for videomodes.
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_VIDEOMODE_H
+#define __LINUX_OF_VIDEOMODE_H
+
+struct device_node;
+struct fb_videomode;
+struct drm_display_mode;
+
+int of_get_video_mode(struct device_node *np, struct drm_display_mode *dmode,
+		struct fb_videomode *fbmode);
+
+#endif /* __LINUX_OF_VIDEOMODE_H */
-- 
1.7.10

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

* Re: [PATCH 05/17] OMAPDSS: Add some new fields to omap_video_timings
From: Tomi Valkeinen @ 2012-06-27 12:42 UTC (permalink / raw)
  To: Archit Taneja; +Cc: Archit Taneja, rob, linux-fbdev, linux-omap
In-Reply-To: <4FEAFC02.2070006@ti.com>

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

On Wed, 2012-06-27 at 17:56 +0530, Archit Taneja wrote:
> On Wednesday 27 June 2012 05:18 PM, Tomi Valkeinen wrote:
> > On Tue, 2012-06-26 at 15:06 +0530, Archit Taneja wrote:
> >> Some panel timing related fields are contained in omap_panel_config in the form
> >> of flags. The fields are:
> >>
> >> - Hsync logic level
> >> - Vsync logic level
> >> - Data driven on rising/falling edge of pixel clock
> >> - Output enable/Data enable logic level
> >> - HSYNC/VSYNC driven on rising/falling edge of pixel clock
> >>
> >> Out of these parameters, Hsync and Vsync logic levels are a part of the timings
> >> in the Xorg modeline configuration. So it makes sense to move the to
> >> omap_video_timings. The rest aren't a part of modeline, but it still makes
> >> sense to move these since they are related to panel timings.
> >>
> >> These fields stored in omap_panel_config in dssdev are configured for LCD
> >> panels, and the corresponding LCD managers in the DISPC_POL_FREQo registers.
> >>
> >> Add the above fields in omap_video_timings. Represent their state via new enums.
> >> The parameter pclk_edge is configured via omap_dss_signal_level, however it
> >> actually configures whether data is driven on the rising or falling edge. This
> >> is a bit unclean, but it prevents us from creating another enum.
> >
> > Hmm, why can't omap_dss_signal_edge be used for pclk_edge? I think it'd
> > fit fine, except OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES would be an illegal
> > value for it.
> 
> I think my paragraph is a bit misleading. The issue is more about the 
> default value. For hsync_vsync_edge(which programs ONOFF and RF), the 
> default value is OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES, and is the case for 
> most panels. But for pclk_edge, the value for most panels is 
> OMAPDSS_DRIVE_SIG_ACTIVE_HIGH.
> 
> So if I use the same enum for both, I'll need to populate either 
> pclk_edge or hsync_vsync_edge for almost every panel. With my approach, 
> I only need to populate these fields for panels having the non-default 
> requirements. The negative thing is that it's a bit misleading to 
> represent pclk_edge with the omap_dss_signal_level enum.

Ah, I see.

I really think using the ACTIVE_LOW/HIGH for pclk_edge is quite ugly =).

Well, one option is to add new entry for the enums, "UNDEFINED" or
perhaps "DEFAULT", which would have value 0. Then omapdss would know
that it should use the default value, whatever that is.

Another option is to have all panels define the pclk_edge, so that
there's no default value needed.

Also, related thing, I see you're writing enum values directly to the
registers. If you do that, it'd be good to explicitly set the number
value of the enums. Otherwise it's quite easy to add a new enum value
later between the old ones, breaking everything (perhaps not a problem
here, though).

 Tomi


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

^ 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