[PATCH 0/9] OMAPDSS: minor fixes & cleanups

[PATCH 0/9] OMAPDSS: minor fixes & cleanups

[Tomi Valkeinen]

Hi,

This series contains minor cleanups and fixes for omapdss.

 Tomi

Tomi Valkeinen (9):
  OMAPDSS: DSI: fix dsi_get_dsidev_from_id()
  OMAPDSS: fix registering the vsync isr in apply
  OMAPDSS: DISPC: constify function parameters
  OMAPDSS: combine LCD related config into one func
  OMAPDSS: remove declarations for non-existing funcs
  OMAPDSS: DISPC: remove struct omap_overlay use
  OMAPDSS: DISPC: cleanup lcd and digit enable
  OMAPDSS: DISPC: add dispc_mgr_get_sync_lost_irq()
  OMAPDSS: DISPC: cleanup lcd/digit enable/disable

 drivers/video/omap2/dss/apply.c |   27 ++---
 drivers/video/omap2/dss/dispc.c |  235 ++++++++++++++++++++++++---------------
 drivers/video/omap2/dss/dsi.c   |   13 ++-
 drivers/video/omap2/dss/dss.h   |   21 ++--
 4 files changed, 174 insertions(+), 122 deletions(-)

-- 
1.7.9.5

[PATCH 1/9] OMAPDSS: DSI: fix dsi_get_dsidev_from_id()

[Tomi Valkeinen]

If dsi_get_dsidev_from_id() is called with a DSI module id that does not
exist on the board, the function will crash as omap_dss_get_output()
will return NULL.

This happens on omap3 boards when dumping DSI clocks, as the dumping
code will try to get the dsidev for DSI modules 0 and 1, but omap3 only
has DSI module 0.

Also clean up the id -> output mapping, so that if the function is
called with invalid module ID it will return NULL.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index d64ac38..bee9284 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -365,11 +365,20 @@ struct platform_device *dsi_get_dsidev_from_id(int module)
 	struct omap_dss_output *out;
 	enum omap_dss_output_id	id;
 
-	id = module = 0 ? OMAP_DSS_OUTPUT_DSI1 : OMAP_DSS_OUTPUT_DSI2;
+	switch (module) {
+	case 0:
+		id = OMAP_DSS_OUTPUT_DSI1;
+		break;
+	case 1:
+		id = OMAP_DSS_OUTPUT_DSI2;
+		break;
+	default:
+		return NULL;
+	}
 
 	out = omap_dss_get_output(id);
 
-	return out->pdev;
+	return out ? out->pdev : NULL;
 }
 
 static inline void dsi_write_reg(struct platform_device *dsidev,
-- 
1.7.9.5

[PATCH 2/9] OMAPDSS: fix registering the vsync isr in apply

[Tomi Valkeinen]

When we enable an output we don't check if we need to register the vsync
isr. This causes us to miss vsync interrupts until somebody changes the
configuration of an overlay or an overlay manager.

Add the registration to dss_mgr_enable to fix the problem.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/apply.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 19d66f4..29ce5a8 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -1035,6 +1035,9 @@ int dss_mgr_enable(struct omap_overlay_manager *mgr)
 	if (!mgr_manual_update(mgr))
 		mp->updating = true;
 
+	if (!dss_data.irq_enabled && need_isr())
+		dss_register_vsync_isr();
+
 	spin_unlock_irqrestore(&data_lock, flags);
 
 	if (!mgr_manual_update(mgr))
-- 
1.7.9.5

[PATCH 3/9] OMAPDSS: DISPC: constify function parameters

[Tomi Valkeinen]

Add consts to dispc function parameters which do not modify the passed
structs.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   10 +++++-----
 drivers/video/omap2/dss/dss.h   |    6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index b43477a..4dfc90a 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1076,7 +1076,7 @@ static void dispc_mgr_enable_cpr(enum omap_channel channel, bool enable)
 }
 
 static void dispc_mgr_set_cpr_coef(enum omap_channel channel,
-		struct omap_dss_cpr_coefs *coefs)
+		const struct omap_dss_cpr_coefs *coefs)
 {
 	u32 coef_r, coef_g, coef_b;
 
@@ -2505,7 +2505,7 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
 		bool mem_to_mem)
 {
 	int r;
-	struct omap_overlay *ovl = omap_dss_get_overlay(plane);
+	const struct omap_overlay *ovl = omap_dss_get_overlay(plane);
 	enum omap_channel channel;
 
 	channel = dispc_ovl_get_channel_out(plane);
@@ -2842,7 +2842,7 @@ static void dispc_mgr_enable_alpha_fixed_zorder(enum omap_channel ch,
 }
 
 void dispc_mgr_setup(enum omap_channel channel,
-		struct omap_overlay_manager_info *info)
+		const struct omap_overlay_manager_info *info)
 {
 	dispc_mgr_set_default_color(channel, info->default_color);
 	dispc_mgr_set_trans_key(channel, info->trans_key_type, info->trans_key);
@@ -3012,7 +3012,7 @@ static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
 
 /* change name to mode? */
 void dispc_mgr_set_timings(enum omap_channel channel,
-		struct omap_video_timings *timings)
+		const struct omap_video_timings *timings)
 {
 	unsigned xtot, ytot;
 	unsigned long ht, vt;
@@ -3533,7 +3533,7 @@ int dispc_calc_clock_rates(unsigned long dispc_fclk_rate,
 }
 
 void dispc_mgr_set_clock_div(enum omap_channel channel,
-		struct dispc_clock_info *cinfo)
+		const struct dispc_clock_info *cinfo)
 {
 	DSSDBG("lck = %lu (%u)\n", cinfo->lck, cinfo->lck_div);
 	DSSDBG("pck = %lu (%u)\n", cinfo->pck, cinfo->pck_div);
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 6728892..eaf0856 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -475,16 +475,16 @@ void dispc_mgr_enable_stallmode(enum omap_channel channel, bool enable);
 void dispc_mgr_set_tft_data_lines(enum omap_channel channel, u8 data_lines);
 void dispc_mgr_set_lcd_type_tft(enum omap_channel channel);
 void dispc_mgr_set_timings(enum omap_channel channel,
-		struct omap_video_timings *timings);
+		const struct omap_video_timings *timings);
 unsigned long dispc_mgr_lclk_rate(enum omap_channel channel);
 unsigned long dispc_mgr_pclk_rate(enum omap_channel channel);
 unsigned long dispc_core_clk_rate(void);
 void dispc_mgr_set_clock_div(enum omap_channel channel,
-		struct dispc_clock_info *cinfo);
+		const struct dispc_clock_info *cinfo);
 int dispc_mgr_get_clock_div(enum omap_channel channel,
 		struct dispc_clock_info *cinfo);
 void dispc_mgr_setup(enum omap_channel channel,
-		struct omap_overlay_manager_info *info);
+		const struct omap_overlay_manager_info *info);
 
 u32 dispc_wb_get_framedone_irq(void);
 bool dispc_wb_go_busy(void);
-- 
1.7.9.5

[PATCH 4/9] OMAPDSS: combine LCD related config into one func

[Tomi Valkeinen]

Dispc has a bunch of functions used to configure output related
parameters:

- dispc_mgr_set_io_pad_mode
- dispc_mgr_enable_stallmode
- dispc_mgr_enable_fifohandcheck
- dispc_mgr_set_clock_div
- dispc_mgr_set_tft_data_lines
- dispc_lcd_enable_signal_polarity
- dispc_mgr_set_lcd_type_tft

These are all called together, and the configuration values are taken
from struct dss_lcd_mgr_config.

Instead of exposing those individual dispc functions, create a new one,
dispc_mgr_set_lcd_config(), which is used to configure the above
parameters from values in struct dss_lcd_mgr_config.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/apply.c |   18 ++----------------
 drivers/video/omap2/dss/dispc.c |   29 +++++++++++++++++++++++------
 drivers/video/omap2/dss/dss.h   |    8 ++------
 3 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 29ce5a8..ae9f70d 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -666,22 +666,8 @@ static void dss_mgr_write_regs_extra(struct omap_overlay_manager *mgr)
 	dispc_mgr_set_timings(mgr->id, &mp->timings);
 
 	/* lcd_config parameters */
-	if (dss_mgr_is_lcd(mgr->id)) {
-		dispc_mgr_set_io_pad_mode(mp->lcd_config.io_pad_mode);
-
-		dispc_mgr_enable_stallmode(mgr->id, mp->lcd_config.stallmode);
-		dispc_mgr_enable_fifohandcheck(mgr->id,
-			mp->lcd_config.fifohandcheck);
-
-		dispc_mgr_set_clock_div(mgr->id, &mp->lcd_config.clock_info);
-
-		dispc_mgr_set_tft_data_lines(mgr->id,
-			mp->lcd_config.video_port_width);
-
-		dispc_lcd_enable_signal_polarity(mp->lcd_config.lcden_sig_polarity);
-
-		dispc_mgr_set_lcd_type_tft(mgr->id);
-	}
+	if (dss_mgr_is_lcd(mgr->id))
+		dispc_mgr_set_lcd_config(mgr->id, &mp->lcd_config);
 
 	mp->extra_info_dirty = false;
 	if (mp->updating)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 4dfc90a..d3c58eb 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2769,7 +2769,7 @@ bool dispc_wb_is_enabled(void)
 	return REG_GET(DISPC_OVL_ATTRIBUTES(plane), 0, 0);
 }
 
-void dispc_lcd_enable_signal_polarity(bool act_high)
+static void dispc_lcd_enable_signal_polarity(bool act_high)
 {
 	if (!dss_has_feature(FEAT_LCDENABLEPOL))
 		return;
@@ -2793,13 +2793,13 @@ void dispc_pck_free_enable(bool enable)
 	REG_FLD_MOD(DISPC_CONTROL, enable ? 1 : 0, 27, 27);
 }
 
-void dispc_mgr_enable_fifohandcheck(enum omap_channel channel, bool enable)
+static void dispc_mgr_enable_fifohandcheck(enum omap_channel channel, bool enable)
 {
 	mgr_fld_write(channel, DISPC_MGR_FLD_FIFOHANDCHECK, enable);
 }
 
 
-void dispc_mgr_set_lcd_type_tft(enum omap_channel channel)
+static void dispc_mgr_set_lcd_type_tft(enum omap_channel channel)
 {
 	mgr_fld_write(channel, DISPC_MGR_FLD_STNTFT, 1);
 }
@@ -2855,7 +2855,7 @@ void dispc_mgr_setup(enum omap_channel channel,
 	}
 }
 
-void dispc_mgr_set_tft_data_lines(enum omap_channel channel, u8 data_lines)
+static void dispc_mgr_set_tft_data_lines(enum omap_channel channel, u8 data_lines)
 {
 	int code;
 
@@ -2880,7 +2880,7 @@ void dispc_mgr_set_tft_data_lines(enum omap_channel channel, u8 data_lines)
 	mgr_fld_write(channel, DISPC_MGR_FLD_TFTDATALINES, code);
 }
 
-void dispc_mgr_set_io_pad_mode(enum dss_io_pad_mode mode)
+static void dispc_mgr_set_io_pad_mode(enum dss_io_pad_mode mode)
 {
 	u32 l;
 	int gpout0, gpout1;
@@ -2909,11 +2909,28 @@ void dispc_mgr_set_io_pad_mode(enum dss_io_pad_mode mode)
 	dispc_write_reg(DISPC_CONTROL, l);
 }
 
-void dispc_mgr_enable_stallmode(enum omap_channel channel, bool enable)
+static void dispc_mgr_enable_stallmode(enum omap_channel channel, bool enable)
 {
 	mgr_fld_write(channel, DISPC_MGR_FLD_STALLMODE, enable);
 }
 
+void dispc_mgr_set_lcd_config(enum omap_channel channel,
+		const struct dss_lcd_mgr_config *config)
+{
+	dispc_mgr_set_io_pad_mode(config->io_pad_mode);
+
+	dispc_mgr_enable_stallmode(channel, config->stallmode);
+	dispc_mgr_enable_fifohandcheck(channel, config->fifohandcheck);
+
+	dispc_mgr_set_clock_div(channel, &config->clock_info);
+
+	dispc_mgr_set_tft_data_lines(channel, config->video_port_width);
+
+	dispc_lcd_enable_signal_polarity(config->lcden_sig_polarity);
+
+	dispc_mgr_set_lcd_type_tft(channel);
+}
+
 static bool _dispc_mgr_size_ok(u16 width, u16 height)
 {
 	return width <= dss_feat_get_param_max(FEAT_PARAM_MGR_WIDTH) &&
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index eaf0856..8447871 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -435,7 +435,6 @@ void dispc_runtime_put(void);
 void dispc_enable_sidle(void);
 void dispc_disable_sidle(void);
 
-void dispc_lcd_enable_signal_polarity(bool act_high);
 void dispc_lcd_enable_signal(bool enable);
 void dispc_pck_free_enable(bool enable);
 void dispc_enable_fifomerge(bool enable);
@@ -462,7 +461,6 @@ int dispc_ovl_enable(enum omap_plane plane, bool enable);
 void dispc_ovl_set_channel_out(enum omap_plane plane,
 		enum omap_channel channel);
 
-void dispc_mgr_enable_fifohandcheck(enum omap_channel channel, bool enable);
 u32 dispc_mgr_get_vsync_irq(enum omap_channel channel);
 u32 dispc_mgr_get_framedone_irq(enum omap_channel channel);
 bool dispc_mgr_go_busy(enum omap_channel channel);
@@ -470,10 +468,8 @@ void dispc_mgr_go(enum omap_channel channel);
 bool dispc_mgr_is_enabled(enum omap_channel channel);
 void dispc_mgr_enable(enum omap_channel channel, bool enable);
 bool dispc_mgr_is_channel_enabled(enum omap_channel channel);
-void dispc_mgr_set_io_pad_mode(enum dss_io_pad_mode mode);
-void dispc_mgr_enable_stallmode(enum omap_channel channel, bool enable);
-void dispc_mgr_set_tft_data_lines(enum omap_channel channel, u8 data_lines);
-void dispc_mgr_set_lcd_type_tft(enum omap_channel channel);
+void dispc_mgr_set_lcd_config(enum omap_channel channel,
+		const struct dss_lcd_mgr_config *config);
 void dispc_mgr_set_timings(enum omap_channel channel,
 		const struct omap_video_timings *timings);
 unsigned long dispc_mgr_lclk_rate(enum omap_channel channel);
-- 
1.7.9.5

[PATCH 5/9] OMAPDSS: remove declarations for non-existing funcs

[Tomi Valkeinen]

dss_mgr_set_device and dss_mgr_unset_device are declared in dss.h, but
the functions do not exist. Remove the declarations.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dss.h |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 8447871..373847c 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -217,9 +217,6 @@ int dss_mgr_set_info(struct omap_overlay_manager *mgr,
 		struct omap_overlay_manager_info *info);
 void dss_mgr_get_info(struct omap_overlay_manager *mgr,
 		struct omap_overlay_manager_info *info);
-int dss_mgr_set_device(struct omap_overlay_manager *mgr,
-		struct omap_dss_device *dssdev);
-int dss_mgr_unset_device(struct omap_overlay_manager *mgr);
 int dss_mgr_set_output(struct omap_overlay_manager *mgr,
 		struct omap_dss_output *output);
 int dss_mgr_unset_output(struct omap_overlay_manager *mgr);
-- 
1.7.9.5

[PATCH 6/9] OMAPDSS: DISPC: remove struct omap_overlay use

[Tomi Valkeinen]

dispc_ovl_setup() uses struct omap_overlay to get the caps for the
overlay. We can change the code to get the caps directly from dss
features, thus removing the dependency to struct omap_overlay.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dispc.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index d3c58eb..9f0ce18 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2505,7 +2505,7 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
 		bool mem_to_mem)
 {
 	int r;
-	const struct omap_overlay *ovl = omap_dss_get_overlay(plane);
+	enum omap_overlay_caps caps = dss_feat_get_overlay_caps(plane);
 	enum omap_channel channel;
 
 	channel = dispc_ovl_get_channel_out(plane);
@@ -2516,7 +2516,7 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
 		oi->pos_y, oi->width, oi->height, oi->out_width, oi->out_height,
 		oi->color_mode, oi->rotation, oi->mirror, channel, replication);
 
-	r = dispc_ovl_setup_common(plane, ovl->caps, oi->paddr, oi->p_uv_addr,
+	r = dispc_ovl_setup_common(plane, caps, oi->paddr, oi->p_uv_addr,
 		oi->screen_width, oi->pos_x, oi->pos_y, oi->width, oi->height,
 		oi->out_width, oi->out_height, oi->color_mode, oi->rotation,
 		oi->mirror, oi->zorder, oi->pre_mult_alpha, oi->global_alpha,
-- 
1.7.9.5

[PATCH 7/9] OMAPDSS: DISPC: cleanup lcd and digit enable

[Tomi Valkeinen]

dispc.c's functions to enable LCD and DIGIT outputs can be cleaned up a
bit by using common functions to set the enable bit and to check if the
output is enabled.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 9f0ce18..492740e 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2591,13 +2591,18 @@ static void dispc_disable_isr(void *data, u32 mask)
 	complete(compl);
 }
 
-static void _enable_lcd_out(enum omap_channel channel, bool enable)
+static void _enable_mgr_out(enum omap_channel channel, bool enable)
 {
 	mgr_fld_write(channel, DISPC_MGR_FLD_ENABLE, enable);
 	/* flush posted write */
 	mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
 }
 
+bool dispc_mgr_is_enabled(enum omap_channel channel)
+{
+	return !!mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
+}
+
 static void dispc_mgr_enable_lcd_out(enum omap_channel channel, bool enable)
 {
 	struct completion frame_done_completion;
@@ -2608,7 +2613,7 @@ 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 = mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
+	is_on = dispc_mgr_is_enabled(channel);
 
 	irq = mgr_desc[channel].framedone_irq;
 
@@ -2622,7 +2627,7 @@ static void dispc_mgr_enable_lcd_out(enum omap_channel channel, bool enable)
 			DSSERR("failed to register FRAMEDONE isr\n");
 	}
 
-	_enable_lcd_out(channel, enable);
+	_enable_mgr_out(channel, enable);
 
 	if (!enable && is_on) {
 		if (!wait_for_completion_timeout(&frame_done_completion,
@@ -2637,13 +2642,6 @@ static void dispc_mgr_enable_lcd_out(enum omap_channel channel, bool enable)
 	}
 }
 
-static void _enable_digit_out(bool enable)
-{
-	REG_FLD_MOD(DISPC_CONTROL, enable ? 1 : 0, 1, 1);
-	/* flush posted write */
-	dispc_read_reg(DISPC_CONTROL);
-}
-
 static void dispc_mgr_enable_digit_out(bool enable)
 {
 	struct completion frame_done_completion;
@@ -2652,7 +2650,7 @@ static void dispc_mgr_enable_digit_out(bool enable)
 	u32 irq_mask;
 	int num_irqs;
 
-	if (REG_GET(DISPC_CONTROL, 1, 1) = enable)
+	if (dispc_mgr_is_enabled(OMAP_DSS_CHANNEL_DIGIT) = enable)
 		return;
 
 	src = dss_get_hdmi_venc_clk_source();
@@ -2689,7 +2687,7 @@ static void dispc_mgr_enable_digit_out(bool enable)
 	if (r)
 		DSSERR("failed to register %x isr\n", irq_mask);
 
-	_enable_digit_out(enable);
+	_enable_mgr_out(OMAP_DSS_CHANNEL_DIGIT, enable);
 
 	for (i = 0; i < num_irqs; ++i) {
 		if (!wait_for_completion_timeout(&frame_done_completion,
@@ -2713,11 +2711,6 @@ static void dispc_mgr_enable_digit_out(bool enable)
 	}
 }
 
-bool dispc_mgr_is_enabled(enum omap_channel channel)
-{
-	return !!mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
-}
-
 void dispc_mgr_enable(enum omap_channel channel, bool enable)
 {
 	if (dss_mgr_is_lcd(channel))
-- 
1.7.9.5

[PATCH 8/9] OMAPDSS: DISPC: add dispc_mgr_get_sync_lost_irq()

[Tomi Valkeinen]

Add function that returns the sync lost irq mask for the given channel.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dispc.c |    5 +++++
 drivers/video/omap2/dss/dss.h   |    1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 492740e..82339ad 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -543,6 +543,11 @@ u32 dispc_mgr_get_framedone_irq(enum omap_channel channel)
 	return mgr_desc[channel].framedone_irq;
 }
 
+u32 dispc_mgr_get_sync_lost_irq(enum omap_channel channel)
+{
+	return mgr_desc[channel].sync_lost_irq;
+}
+
 u32 dispc_wb_get_framedone_irq(void)
 {
 	return DISPC_IRQ_FRAMEDONEWB;
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 373847c..b6e3a06 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -460,6 +460,7 @@ void dispc_ovl_set_channel_out(enum omap_plane plane,
 
 u32 dispc_mgr_get_vsync_irq(enum omap_channel channel);
 u32 dispc_mgr_get_framedone_irq(enum omap_channel channel);
+u32 dispc_mgr_get_sync_lost_irq(enum omap_channel channel);
 bool dispc_mgr_go_busy(enum omap_channel channel);
 void dispc_mgr_go(enum omap_channel channel);
 bool dispc_mgr_is_enabled(enum omap_channel channel);
-- 
1.7.9.5

[PATCH 9/9] OMAPDSS: DISPC: cleanup lcd/digit enable/disable

[Tomi Valkeinen]

We currently have a single function to enable and disable the manager
output for LCD and DIGIT. The functions are a bit complex, as handling
both enable and disable require some extra steps to ensure that the
output is enabled or disabled properly without errors before exiting the
function.

The code can be made simpler to understand by splitting the functions
into separate enable and disable functions. We'll also clean up the
comments and some parameter names at the same time.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/apply.c |    6 +-
 drivers/video/omap2/dss/dispc.c |  178 ++++++++++++++++++++++++---------------
 drivers/video/omap2/dss/dss.h   |    3 +-
 3 files changed, 116 insertions(+), 71 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index ae9f70d..4fff8ac 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -772,7 +772,7 @@ void dss_mgr_start_update(struct omap_overlay_manager *mgr)
 	if (!dss_data.irq_enabled && need_isr())
 		dss_register_vsync_isr();
 
-	dispc_mgr_enable(mgr->id, true);
+	dispc_mgr_enable(mgr->id);
 
 	mgr_clear_shadow_dirty(mgr);
 
@@ -1027,7 +1027,7 @@ int dss_mgr_enable(struct omap_overlay_manager *mgr)
 	spin_unlock_irqrestore(&data_lock, flags);
 
 	if (!mgr_manual_update(mgr))
-		dispc_mgr_enable(mgr->id, true);
+		dispc_mgr_enable(mgr->id);
 
 out:
 	mutex_unlock(&apply_lock);
@@ -1052,7 +1052,7 @@ void dss_mgr_disable(struct omap_overlay_manager *mgr)
 		goto out;
 
 	if (!mgr_manual_update(mgr))
-		dispc_mgr_enable(mgr->id, false);
+		dispc_mgr_disable(mgr->id);
 
 	spin_lock_irqsave(&data_lock, flags);
 
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 82339ad..94f393ec 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2590,7 +2590,7 @@ int dispc_ovl_enable(enum omap_plane plane, bool enable)
 	return 0;
 }
 
-static void dispc_disable_isr(void *data, u32 mask)
+static void dispc_mgr_disable_isr(void *data, u32 mask)
 {
 	struct completion *compl = data;
 	complete(compl);
@@ -2608,122 +2608,166 @@ bool dispc_mgr_is_enabled(enum omap_channel channel)
 	return !!mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
 }
 
-static void dispc_mgr_enable_lcd_out(enum omap_channel channel, bool enable)
+static void dispc_mgr_enable_lcd_out(enum omap_channel channel)
 {
-	struct completion frame_done_completion;
-	bool is_on;
+	_enable_mgr_out(channel, true);
+}
+
+static void dispc_mgr_disable_lcd_out(enum omap_channel channel)
+{
+	DECLARE_COMPLETION_ONSTACK(framedone_compl);
 	int r;
 	u32 irq;
 
-	/* 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 = dispc_mgr_is_enabled(channel);
+	if (dispc_mgr_is_enabled(channel) = false)
+		return;
+
+	/*
+	 * When we disable LCD output, we need to wait for FRAMEDONE to know
+	 * that DISPC has finished with the LCD output.
+	 */
 
-	irq = mgr_desc[channel].framedone_irq;
+	irq = dispc_mgr_get_framedone_irq(channel);
 
-	if (!enable && is_on) {
-		init_completion(&frame_done_completion);
+	r = omap_dispc_register_isr(dispc_mgr_disable_isr, &framedone_compl,
+			irq);
+	if (r)
+		DSSERR("failed to register FRAMEDONE isr\n");
 
-		r = omap_dispc_register_isr(dispc_disable_isr,
-				&frame_done_completion, irq);
+	_enable_mgr_out(channel, false);
 
-		if (r)
-			DSSERR("failed to register FRAMEDONE isr\n");
+	/* if we couldn't register for framedone, just sleep and exit */
+	if (r) {
+		msleep(200);
+		return;
 	}
 
-	_enable_mgr_out(channel, enable);
+	if (!wait_for_completion_timeout(&framedone_compl,
+				msecs_to_jiffies(100)))
+		DSSERR("timeout waiting for FRAME DONE\n");
 
-	if (!enable && is_on) {
-		if (!wait_for_completion_timeout(&frame_done_completion,
-					msecs_to_jiffies(100)))
-			DSSERR("timeout waiting for FRAME DONE\n");
+	r = omap_dispc_unregister_isr(dispc_mgr_disable_isr, &framedone_compl,
+			irq);
+	if (r)
+		DSSERR("failed to unregister FRAMEDONE isr\n");
+}
 
-		r = omap_dispc_unregister_isr(dispc_disable_isr,
-				&frame_done_completion, irq);
+static void dispc_digit_out_enable_isr(void *data, u32 mask)
+{
+	struct completion *compl = data;
 
-		if (r)
-			DSSERR("failed to unregister FRAMEDONE isr\n");
+	/* ignore any sync lost interrupts */
+	if (mask & (DISPC_IRQ_EVSYNC_EVEN | DISPC_IRQ_EVSYNC_ODD))
+		complete(compl);
+}
+
+static void dispc_mgr_enable_digit_out(void)
+{
+	DECLARE_COMPLETION_ONSTACK(vsync_compl);
+	int r;
+	u32 irq_mask;
+
+	if (dispc_mgr_is_enabled(OMAP_DSS_CHANNEL_DIGIT) = true)
+		return;
+
+	/*
+	 * Digit output produces some sync lost interrupts during the first
+	 * frame when enabling. Those need to be ignored, so we register for the
+	 * sync lost irq to prevent the error handler from triggering.
+	 */
+
+	irq_mask = dispc_mgr_get_vsync_irq(OMAP_DSS_CHANNEL_DIGIT) |
+		dispc_mgr_get_sync_lost_irq(OMAP_DSS_CHANNEL_DIGIT);
+
+	r = omap_dispc_register_isr(dispc_digit_out_enable_isr, &vsync_compl,
+			irq_mask);
+	if (r) {
+		DSSERR("failed to register %x isr\n", irq_mask);
+		return;
 	}
+
+	_enable_mgr_out(OMAP_DSS_CHANNEL_DIGIT, true);
+
+	/* wait for the first evsync */
+	if (!wait_for_completion_timeout(&vsync_compl, msecs_to_jiffies(100)))
+		DSSERR("timeout waiting for digit out to start\n");
+
+	r = omap_dispc_unregister_isr(dispc_digit_out_enable_isr, &vsync_compl,
+			irq_mask);
+	if (r)
+		DSSERR("failed to unregister %x isr\n", irq_mask);
 }
 
-static void dispc_mgr_enable_digit_out(bool enable)
+static void dispc_mgr_disable_digit_out(void)
 {
-	struct completion frame_done_completion;
+	DECLARE_COMPLETION_ONSTACK(framedone_compl);
 	enum dss_hdmi_venc_clk_source_select src;
 	int r, i;
 	u32 irq_mask;
 	int num_irqs;
 
-	if (dispc_mgr_is_enabled(OMAP_DSS_CHANNEL_DIGIT) = enable)
+	if (dispc_mgr_is_enabled(OMAP_DSS_CHANNEL_DIGIT) = false)
 		return;
 
 	src = dss_get_hdmi_venc_clk_source();
 
-	if (enable) {
-		unsigned long flags;
-		/* When we enable digit output, we'll get an extra digit
-		 * sync lost interrupt, that we need to ignore */
-		spin_lock_irqsave(&dispc.irq_lock, flags);
-		dispc.irq_error_mask &= ~DISPC_IRQ_SYNC_LOST_DIGIT;
-		_omap_dispc_set_irqs();
-		spin_unlock_irqrestore(&dispc.irq_lock, flags);
-	}
-
-	/* When we disable digit output, we need to wait until fields are done.
-	 * Otherwise the DSS is still working, and turning off the clocks
-	 * prevents DSS from going to OFF mode. And when enabling, we need to
-	 * wait for the extra sync losts */
-	init_completion(&frame_done_completion);
+	/*
+	 * When we disable the digit output, we need to wait for FRAMEDONE to
+	 * know that DISPC has finished with the output. For analog tv out we'll
+	 * use vsync, as omap2/3 don't have framedone for TV.
+	 */
 
-	if (src = DSS_HDMI_M_PCLK && enable = false) {
+	if (src = DSS_HDMI_M_PCLK) {
 		irq_mask = DISPC_IRQ_FRAMEDONETV;
 		num_irqs = 1;
 	} else {
-		irq_mask = DISPC_IRQ_EVSYNC_EVEN | DISPC_IRQ_EVSYNC_ODD;
-		/* XXX I understand from TRM that we should only wait for the
-		 * current field to complete. But it seems we have to wait for
-		 * both fields */
+		irq_mask = dispc_mgr_get_vsync_irq(OMAP_DSS_CHANNEL_DIGIT);
+		/*
+		 * We need to wait for both even and odd vsyncs. Note that this
+		 * is not totally reliable, as we could get a vsync interrupt
+		 * before we disable the output, which leads to timeout in the
+		 * wait_for_completion.
+		 */
 		num_irqs = 2;
 	}
 
-	r = omap_dispc_register_isr(dispc_disable_isr, &frame_done_completion,
+	r = omap_dispc_register_isr(dispc_mgr_disable_isr, &framedone_compl,
 			irq_mask);
 	if (r)
 		DSSERR("failed to register %x isr\n", irq_mask);
 
-	_enable_mgr_out(OMAP_DSS_CHANNEL_DIGIT, enable);
+	_enable_mgr_out(OMAP_DSS_CHANNEL_DIGIT, false);
 
 	for (i = 0; i < num_irqs; ++i) {
-		if (!wait_for_completion_timeout(&frame_done_completion,
+		if (!wait_for_completion_timeout(&framedone_compl,
 					msecs_to_jiffies(100)))
-			DSSERR("timeout waiting for digit out to %s\n",
-					enable ? "start" : "stop");
+			DSSERR("timeout waiting for digit out to stop\n");
 	}
 
-	r = omap_dispc_unregister_isr(dispc_disable_isr, &frame_done_completion,
+	r = omap_dispc_unregister_isr(dispc_mgr_disable_isr, &framedone_compl,
 			irq_mask);
 	if (r)
 		DSSERR("failed to unregister %x isr\n", irq_mask);
+}
 
-	if (enable) {
-		unsigned long flags;
-		spin_lock_irqsave(&dispc.irq_lock, flags);
-		dispc.irq_error_mask |= DISPC_IRQ_SYNC_LOST_DIGIT;
-		dispc_write_reg(DISPC_IRQSTATUS, DISPC_IRQ_SYNC_LOST_DIGIT);
-		_omap_dispc_set_irqs();
-		spin_unlock_irqrestore(&dispc.irq_lock, flags);
-	}
+void dispc_mgr_enable(enum omap_channel channel)
+{
+	if (dss_mgr_is_lcd(channel))
+		dispc_mgr_enable_lcd_out(channel);
+	else if (channel = OMAP_DSS_CHANNEL_DIGIT)
+		dispc_mgr_enable_digit_out();
+	else
+		WARN_ON(1);
 }
 
-void dispc_mgr_enable(enum omap_channel channel, bool enable)
+void dispc_mgr_disable(enum omap_channel channel)
 {
 	if (dss_mgr_is_lcd(channel))
-		dispc_mgr_enable_lcd_out(channel, enable);
+		dispc_mgr_disable_lcd_out(channel);
 	else if (channel = OMAP_DSS_CHANNEL_DIGIT)
-		dispc_mgr_enable_digit_out(enable);
+		dispc_mgr_disable_digit_out();
 	else
-		BUG();
+		WARN_ON(1);
 }
 
 void dispc_wb_enable(bool enable)
@@ -2740,7 +2784,7 @@ void dispc_wb_enable(bool enable)
 	if (!enable && is_on) {
 		init_completion(&frame_done_completion);
 
-		r = omap_dispc_register_isr(dispc_disable_isr,
+		r = omap_dispc_register_isr(dispc_mgr_disable_isr,
 				&frame_done_completion, irq);
 		if (r)
 			DSSERR("failed to register FRAMEDONEWB isr\n");
@@ -2753,7 +2797,7 @@ void dispc_wb_enable(bool enable)
 					msecs_to_jiffies(100)))
 			DSSERR("timeout waiting for FRAMEDONEWB\n");
 
-		r = omap_dispc_unregister_isr(dispc_disable_isr,
+		r = omap_dispc_unregister_isr(dispc_mgr_disable_isr,
 				&frame_done_completion, irq);
 		if (r)
 			DSSERR("failed to unregister FRAMEDONEWB isr\n");
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index b6e3a06..e501a77 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -464,7 +464,8 @@ u32 dispc_mgr_get_sync_lost_irq(enum omap_channel channel);
 bool dispc_mgr_go_busy(enum omap_channel channel);
 void dispc_mgr_go(enum omap_channel channel);
 bool dispc_mgr_is_enabled(enum omap_channel channel);
-void dispc_mgr_enable(enum omap_channel channel, bool enable);
+void dispc_mgr_enable(enum omap_channel channel);
+void dispc_mgr_disable(enum omap_channel channel);
 bool dispc_mgr_is_channel_enabled(enum omap_channel channel);
 void dispc_mgr_set_lcd_config(enum omap_channel channel,
 		const struct dss_lcd_mgr_config *config);
-- 
1.7.9.5

Re: [PATCH 9/9] OMAPDSS: DISPC: cleanup lcd/digit enable/disable

[Archit Taneja]

Hi,

On Wednesday 17 October 2012 04:50 PM, Tomi Valkeinen wrote:
> We currently have a single function to enable and disable the manager
> output for LCD and DIGIT. The functions are a bit complex, as handling
> both enable and disable require some extra steps to ensure that the
> output is enabled or disabled properly without errors before exiting the
> function.
>
> The code can be made simpler to understand by splitting the functions
> into separate enable and disable functions. We'll also clean up the
> comments and some parameter names at the same time.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/dss/apply.c |    6 +-
>   drivers/video/omap2/dss/dispc.c |  178 ++++++++++++++++++++++++---------------
>   drivers/video/omap2/dss/dss.h   |    3 +-
>   3 files changed, 116 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
> index ae9f70d..4fff8ac 100644
> --- a/drivers/video/omap2/dss/apply.c
> +++ b/drivers/video/omap2/dss/apply.c
> @@ -772,7 +772,7 @@ void dss_mgr_start_update(struct omap_overlay_manager *mgr)
>   	if (!dss_data.irq_enabled && need_isr())
>   		dss_register_vsync_isr();
>
> -	dispc_mgr_enable(mgr->id, true);
> +	dispc_mgr_enable(mgr->id);
>
>   	mgr_clear_shadow_dirty(mgr);
>
> @@ -1027,7 +1027,7 @@ int dss_mgr_enable(struct omap_overlay_manager *mgr)
>   	spin_unlock_irqrestore(&data_lock, flags);
>
>   	if (!mgr_manual_update(mgr))
> -		dispc_mgr_enable(mgr->id, true);
> +		dispc_mgr_enable(mgr->id);
>
>   out:
>   	mutex_unlock(&apply_lock);
> @@ -1052,7 +1052,7 @@ void dss_mgr_disable(struct omap_overlay_manager *mgr)
>   		goto out;
>
>   	if (!mgr_manual_update(mgr))
> -		dispc_mgr_enable(mgr->id, false);
> +		dispc_mgr_disable(mgr->id);
>
>   	spin_lock_irqsave(&data_lock, flags);
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 82339ad..94f393ec 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -2590,7 +2590,7 @@ int dispc_ovl_enable(enum omap_plane plane, bool enable)
>   	return 0;
>   }
>
> -static void dispc_disable_isr(void *data, u32 mask)
> +static void dispc_mgr_disable_isr(void *data, u32 mask)
>   {
>   	struct completion *compl = data;
>   	complete(compl);
> @@ -2608,122 +2608,166 @@ bool dispc_mgr_is_enabled(enum omap_channel channel)
>   	return !!mgr_fld_read(channel, DISPC_MGR_FLD_ENABLE);
>   }
>
> -static void dispc_mgr_enable_lcd_out(enum omap_channel channel, bool enable)
> +static void dispc_mgr_enable_lcd_out(enum omap_channel channel)
>   {
> -	struct completion frame_done_completion;
> -	bool is_on;
> +	_enable_mgr_out(channel, true);
> +}
> +
> +static void dispc_mgr_disable_lcd_out(enum omap_channel channel)
> +{
> +	DECLARE_COMPLETION_ONSTACK(framedone_compl);
>   	int r;
>   	u32 irq;
>
> -	/* 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 = dispc_mgr_is_enabled(channel);
> +	if (dispc_mgr_is_enabled(channel) = false)
> +		return;
> +
> +	/*
> +	 * When we disable LCD output, we need to wait for FRAMEDONE to know
> +	 * that DISPC has finished with the LCD output.
> +	 */
>
> -	irq = mgr_desc[channel].framedone_irq;
> +	irq = dispc_mgr_get_framedone_irq(channel);
>
> -	if (!enable && is_on) {
> -		init_completion(&frame_done_completion);
> +	r = omap_dispc_register_isr(dispc_mgr_disable_isr, &framedone_compl,
> +			irq);
> +	if (r)
> +		DSSERR("failed to register FRAMEDONE isr\n");
>
> -		r = omap_dispc_register_isr(dispc_disable_isr,
> -				&frame_done_completion, irq);
> +	_enable_mgr_out(channel, false);
>
> -		if (r)
> -			DSSERR("failed to register FRAMEDONE isr\n");
> +	/* if we couldn't register for framedone, just sleep and exit */
> +	if (r) {
> +		msleep(200);

We sleep for 200 ms if we fail to register for framedone. But we just 
wait for 100ms for FRAMEDONE to occur. It seems a bit incorrect, both 
should be kept the same, shouldn't they?

> +		return;
>   	}
>
> -	_enable_mgr_out(channel, enable);
> +	if (!wait_for_completion_timeout(&framedone_compl,
> +				msecs_to_jiffies(100)))
> +		DSSERR("timeout waiting for FRAME DONE\n");
>
> -	if (!enable && is_on) {
> -		if (!wait_for_completion_timeout(&frame_done_completion,
> -					msecs_to_jiffies(100)))
> -			DSSERR("timeout waiting for FRAME DONE\n");
> +	r = omap_dispc_unregister_isr(dispc_mgr_disable_isr, &framedone_compl,
> +			irq);
> +	if (r)
> +		DSSERR("failed to unregister FRAMEDONE isr\n");
> +}
>
> -		r = omap_dispc_unregister_isr(dispc_disable_isr,
> -				&frame_done_completion, irq);
> +static void dispc_digit_out_enable_isr(void *data, u32 mask)
> +{
> +	struct completion *compl = data;
>
> -		if (r)
> -			DSSERR("failed to unregister FRAMEDONE isr\n");
> +	/* ignore any sync lost interrupts */
> +	if (mask & (DISPC_IRQ_EVSYNC_EVEN | DISPC_IRQ_EVSYNC_ODD))
> +		complete(compl);
> +}
> +
> +static void dispc_mgr_enable_digit_out(void)
> +{
> +	DECLARE_COMPLETION_ONSTACK(vsync_compl);
> +	int r;
> +	u32 irq_mask;
> +
> +	if (dispc_mgr_is_enabled(OMAP_DSS_CHANNEL_DIGIT) = true)
> +		return;
> +
> +	/*
> +	 * Digit output produces some sync lost interrupts during the first
> +	 * frame when enabling. Those need to be ignored, so we register for the
> +	 * sync lost irq to prevent the error handler from triggering.
> +	 */
> +
> +	irq_mask = dispc_mgr_get_vsync_irq(OMAP_DSS_CHANNEL_DIGIT) |
> +		dispc_mgr_get_sync_lost_irq(OMAP_DSS_CHANNEL_DIGIT);
> +
> +	r = omap_dispc_register_isr(dispc_digit_out_enable_isr, &vsync_compl,
> +			irq_mask);
> +	if (r) {
> +		DSSERR("failed to register %x isr\n", irq_mask);
> +		return;
>   	}
> +
> +	_enable_mgr_out(OMAP_DSS_CHANNEL_DIGIT, true);
> +
> +	/* wait for the first evsync */
> +	if (!wait_for_completion_timeout(&vsync_compl, msecs_to_jiffies(100)))
> +		DSSERR("timeout waiting for digit out to start\n");
> +
> +	r = omap_dispc_unregister_isr(dispc_digit_out_enable_isr, &vsync_compl,
> +			irq_mask);
> +	if (r)
> +		DSSERR("failed to unregister %x isr\n", irq_mask);
>   }
>
> -static void dispc_mgr_enable_digit_out(bool enable)
> +static void dispc_mgr_disable_digit_out(void)
>   {
> -	struct completion frame_done_completion;
> +	DECLARE_COMPLETION_ONSTACK(framedone_compl);
>   	enum dss_hdmi_venc_clk_source_select src;
>   	int r, i;
>   	u32 irq_mask;
>   	int num_irqs;
>
> -	if (dispc_mgr_is_enabled(OMAP_DSS_CHANNEL_DIGIT) = enable)
> +	if (dispc_mgr_is_enabled(OMAP_DSS_CHANNEL_DIGIT) = false)
>   		return;
>
>   	src = dss_get_hdmi_venc_clk_source();
>
> -	if (enable) {
> -		unsigned long flags;
> -		/* When we enable digit output, we'll get an extra digit
> -		 * sync lost interrupt, that we need to ignore */
> -		spin_lock_irqsave(&dispc.irq_lock, flags);
> -		dispc.irq_error_mask &= ~DISPC_IRQ_SYNC_LOST_DIGIT;
> -		_omap_dispc_set_irqs();
> -		spin_unlock_irqrestore(&dispc.irq_lock, flags);
> -	}
> -
> -	/* When we disable digit output, we need to wait until fields are done.
> -	 * Otherwise the DSS is still working, and turning off the clocks
> -	 * prevents DSS from going to OFF mode. And when enabling, we need to
> -	 * wait for the extra sync losts */
> -	init_completion(&frame_done_completion);
> +	/*
> +	 * When we disable the digit output, we need to wait for FRAMEDONE to
> +	 * know that DISPC has finished with the output. For analog tv out we'll
> +	 * use vsync, as omap2/3 don't have framedone for TV.
> +	 */
>
> -	if (src = DSS_HDMI_M_PCLK && enable = false) {
> +	if (src = DSS_HDMI_M_PCLK) {
>   		irq_mask = DISPC_IRQ_FRAMEDONETV;
>   		num_irqs = 1;
>   	} else {
> -		irq_mask = DISPC_IRQ_EVSYNC_EVEN | DISPC_IRQ_EVSYNC_ODD;
> -		/* XXX I understand from TRM that we should only wait for the
> -		 * current field to complete. But it seems we have to wait for
> -		 * both fields */
> +		irq_mask = dispc_mgr_get_vsync_irq(OMAP_DSS_CHANNEL_DIGIT);
> +		/*
> +		 * We need to wait for both even and odd vsyncs. Note that this
> +		 * is not totally reliable, as we could get a vsync interrupt
> +		 * before we disable the output, which leads to timeout in the
> +		 * wait_for_completion.
> +		 */
>   		num_irqs = 2;
>   	}
>
> -	r = omap_dispc_register_isr(dispc_disable_isr, &frame_done_completion,
> +	r = omap_dispc_register_isr(dispc_mgr_disable_isr, &framedone_compl,
>   			irq_mask);
>   	if (r)
>   		DSSERR("failed to register %x isr\n", irq_mask);

We should probably sleep here too as we did for LCD above.

Archit

Re: [PATCH 9/9] OMAPDSS: DISPC: cleanup lcd/digit enable/disable

[Tomi Valkeinen]

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

On 2012-10-17 17:38, Archit Taneja wrote:
> Hi,
> 
> On Wednesday 17 October 2012 04:50 PM, Tomi Valkeinen wrote:

>> -        if (r)
>> -            DSSERR("failed to register FRAMEDONE isr\n");
>> +    /* if we couldn't register for framedone, just sleep and exit */
>> +    if (r) {
>> +        msleep(200);
> 
> We sleep for 200 ms if we fail to register for framedone. But we just
> wait for 100ms for FRAMEDONE to occur. It seems a bit incorrect, both
> should be kept the same, shouldn't they?

They are just arbitrary numbers, but you're right, it makes more sense
to have same numbers for both.

>> -    r = omap_dispc_register_isr(dispc_disable_isr,
>> &frame_done_completion,
>> +    r = omap_dispc_register_isr(dispc_mgr_disable_isr, &framedone_compl,
>>               irq_mask);
>>       if (r)
>>           DSSERR("failed to register %x isr\n", irq_mask);
> 
> We should probably sleep here too as we did for LCD above.

Yep, good point. I'll add that.

 Tomi




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

Re: [PATCH 0/9] OMAPDSS: minor fixes & cleanups

[Archit Taneja]

On Wednesday 17 October 2012 04:50 PM, Tomi Valkeinen wrote:
> Hi,
>
> This series contains minor cleanups and fixes for omapdss.

This series looks fine to me. I have a related cleanup patch which you 
may consider adding to the series.

Thanks,
Archit

From: Archit Taneja <archit@ti.com>
Date: Thu, 18 Oct 2012 16:51:57 +0530
Subject: [PATCH] OMAPDSS: Remove acb and acbi fields from omap_dss_device

acbi and acb fields were meant for passive matrix panels which omapdss 
doesn't
support any longer. Remove these fields from omap_dss_device struct.

Signed-off-by: Archit Taneja <archit@ti.com>
---
  include/video/omapdss.h |    4 ----
  1 file changed, 4 deletions(-)

diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index 88c8294..f39e6aa 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -621,10 +621,6 @@ struct omap_dss_device {
  	struct {
  		struct omap_video_timings timings;

-		int acbi;	/* ac-bias pin transitions per interrupt */
-		/* Unit: line clocks */
-		int acb;	/* ac-bias pin frequency */
-
  		enum omap_dss_dsi_pixel_format dsi_pix_fmt;
  		enum omap_dss_dsi_mode dsi_mode;
  		struct omap_dss_dsi_videomode_timings dsi_vm_timings;
-- 
1.7.9.5

Re: [PATCH 0/9] OMAPDSS: minor fixes & cleanups

[Tomi Valkeinen]

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

On 2012-10-22 08:48, Archit Taneja wrote:
> On Wednesday 17 October 2012 04:50 PM, Tomi Valkeinen wrote:
>> Hi,
>>
>> This series contains minor cleanups and fixes for omapdss.
> 
> This series looks fine to me. I have a related cleanup patch which you
> may consider adding to the series.
> 
> Thanks,
> Archit
> 
> From: Archit Taneja <archit@ti.com>
> Date: Thu, 18 Oct 2012 16:51:57 +0530
> Subject: [PATCH] OMAPDSS: Remove acb and acbi fields from omap_dss_device
> 
> acbi and acb fields were meant for passive matrix panels which omapdss
> doesn't
> support any longer. Remove these fields from omap_dss_device struct.
> 
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>  include/video/omapdss.h |    4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index 88c8294..f39e6aa 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -621,10 +621,6 @@ struct omap_dss_device {
>      struct {
>          struct omap_video_timings timings;
> 
> -        int acbi;    /* ac-bias pin transitions per interrupt */
> -        /* Unit: line clocks */
> -        int acb;    /* ac-bias pin frequency */
> -
>          enum omap_dss_dsi_pixel_format dsi_pix_fmt;
>          enum omap_dss_dsi_mode dsi_mode;
>          struct omap_dss_dsi_videomode_timings dsi_vm_timings;

Thanks, applied to omapdss tree.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]