Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 05/12] OMAPDSS: DSI: Configure dss_lcd_mgr_config struct with lcd manager parameters
From: Archit Taneja @ 2012-06-28 14:42 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1340893842-10626-1-git-send-email-archit@ti.com>

Create a dss_lcd_mgr_config struct instance in DSI. Fill up all the parameters
of the struct with configurations held by the panel, and the configurations
required by DSI.

Use these to write to the DISPC registers. These direct register writes would be
later replaced by a function which applies the configuration using the shadow
register programming model.

The function dsi_configure_dispc_clocks() is now called in
dsi_display_init_dispc(), this lets all the lcd manager related configurations
happen in the same place. The DISPC_DIVISORo register was written in
dsi_configure_dispc_clock(), now it just fills up the dispc_clock_info parameter
in mgr_config. The clock_info is written later in dsi_display_init_dispc().

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dsi.c |  117 ++++++++++++++++++++++++++++-------------
 1 file changed, 80 insertions(+), 37 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 94ff3aa..df92e24 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -331,6 +331,8 @@ struct dsi_data {
 	unsigned num_lanes_used;
 
 	unsigned scp_clk_refcount;
+
+	struct dss_lcd_mgr_config mgr_config;
 };
 
 struct dsi_packet_sent_handler_data {
@@ -4339,14 +4341,42 @@ EXPORT_SYMBOL(omap_dsi_update);
 
 /* Display funcs */
 
-static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
+static int dsi_configure_dispc_clocks(struct omap_dss_device *dssdev,
+		const struct omapdss_lcd_clock_config *lcd_clks)
 {
+	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
+	struct dispc_clock_info dispc_cinfo;
 	int r;
+	unsigned long long fck;
+
+	fck = dsi_get_pll_hsdiv_dispc_rate(dsidev);
+
+	dispc_cinfo.lck_div = lcd_clks->lck_div;
+	dispc_cinfo.pck_div = lcd_clks->pck_div;
+
+	r = dispc_calc_clock_rates(fck, &dispc_cinfo);
+	if (r) {
+		DSSERR("Failed to calc dispc clocks\n");
+		return r;
+	}
+
+	dsi->mgr_config.clock_info = dispc_cinfo;
+
+	return 0;
+}
+
+static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
+{
+	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	struct omap_video_timings timings;
+	const struct omapdss_clock_config *clks;
+	int r, lcd_id;
+	u32 irq;
 
 	if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_CMD_MODE) {
 		u16 dw, dh;
-		u32 irq;
 
 		dssdev->driver->get_resolution(dssdev, &dw, &dh);
 
@@ -4366,16 +4396,16 @@ static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
 			(void *) dssdev, irq);
 		if (r) {
 			DSSERR("can't get FRAMEDONE irq\n");
-			return r;
+			goto err;
 		}
 
-		dispc_mgr_enable_stallmode(dssdev->manager->id, true);
-		dispc_mgr_enable_fifohandcheck(dssdev->manager->id, 1);
+		dsi->mgr_config.stallmode = true;
+		dsi->mgr_config.fifohandcheck = true;
 	} else {
 		timings = dssdev->panel.timings;
 
-		dispc_mgr_enable_stallmode(dssdev->manager->id, false);
-		dispc_mgr_enable_fifohandcheck(dssdev->manager->id, 0);
+		dsi->mgr_config.stallmode = false;
+		dsi->mgr_config.fifohandcheck = false;
 	}
 
 	/*
@@ -4391,12 +4421,53 @@ static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
 
 	dss_mgr_set_timings(dssdev->manager, &timings);
 
-	dispc_mgr_set_lcd_type_tft(dssdev->manager->id);
+	clks = dss_get_platform_clock_config();
+
+	switch (dssdev->manager->id) {
+	case OMAP_DSS_CHANNEL_LCD:
+		lcd_id = 0;
+		break;
+	case OMAP_DSS_CHANNEL_LCD2:
+		lcd_id = 1;
+		break;
+	default:
+		r = -EINVAL;
+		goto err1;
+	}
+
+	r = dsi_configure_dispc_clocks(dssdev, &clks->lcd[lcd_id]);
+	if (r)
+		goto err1;
+
+	dsi->mgr_config.io_pad_mode = DSS_IO_PAD_MODE_BYPASS;
+	dsi->mgr_config.video_port_width +			dsi_get_pixel_size(dssdev->panel.dsi_pix_fmt);
+	dsi->mgr_config.lcden_sig_polarity = 0;
+
+	dispc_mgr_set_io_pad_mode(dsi->mgr_config.io_pad_mode);
+
+	dispc_mgr_enable_stallmode(dssdev->manager->id,
+			dsi->mgr_config.stallmode);
+	dispc_mgr_enable_fifohandcheck(dssdev->manager->id,
+			dsi->mgr_config.fifohandcheck);
+
+	dispc_mgr_set_clock_div(dssdev->manager->id,
+			&dsi->mgr_config.clock_info);
 
 	dispc_mgr_set_tft_data_lines(dssdev->manager->id,
-		dsi_get_pixel_size(dssdev->panel.dsi_pix_fmt));
+			dsi->mgr_config.video_port_width);
+
+	dispc_lcd_enable_signal_polarity(dsi->mgr_config.lcden_sig_polarity);
+
+	dispc_mgr_set_lcd_type_tft(dssdev->manager->id);
 
 	return 0;
+err1:
+	if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_CMD_MODE)
+		omap_dispc_unregister_isr(dsi_framedone_irq_callback,
+			(void *) dssdev, irq);
+err:
+	return r;
 }
 
 static void dsi_display_uninit_dispc(struct omap_dss_device *dssdev)
@@ -4437,30 +4508,6 @@ static int dsi_configure_dsi_clocks(struct platform_device *dsidev,
 	return 0;
 }
 
-static int dsi_configure_dispc_clocks(struct omap_dss_device *dssdev,
-		const struct omapdss_lcd_clock_config *lcd_clks)
-{
-	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
-	struct dispc_clock_info dispc_cinfo;
-	int r;
-	unsigned long long fck;
-
-	fck = dsi_get_pll_hsdiv_dispc_rate(dsidev);
-
-	dispc_cinfo.lck_div = lcd_clks->lck_div;
-	dispc_cinfo.pck_div = lcd_clks->pck_div;
-
-	r = dispc_calc_clock_rates(fck, &dispc_cinfo);
-	if (r) {
-		DSSERR("Failed to calc dispc clocks\n");
-		return r;
-	}
-
-	dispc_mgr_set_clock_div(dssdev->manager->id, &dispc_cinfo);
-
-	return 0;
-}
-
 static int dsi_display_init_dsi(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
@@ -4499,10 +4546,6 @@ static int dsi_display_init_dsi(struct omap_dss_device *dssdev)
 
 	DSSDBG("PLL OK\n");
 
-	r = dsi_configure_dispc_clocks(dssdev, &clks->lcd[lcd_id]);
-	if (r)
-		goto err2;
-
 	r = dsi_cio_init(dssdev);
 	if (r)
 		goto err2;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 06/12] OMAPDSS: SDI: Configure dss_lcd_mgr_config struct with lcd manager parameters
From: Archit Taneja @ 2012-06-28 14:42 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1340893842-10626-1-git-send-email-archit@ti.com>

Create a dss_lcd_mgr_config struct instance in SDI. Fill up all the parameters
of the struct with configurations held by the panel, and the configurations
required by SDI.

Use these to write to the DISPC registers. These direct register writes would be
later replaced by a function which applies the configuration using the shadow
register programming model.

Create function sdi_config_lcd_manager() which fills the mgr_config parameters
and writes to the DISPC registers

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/sdi.c |   42 +++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
index 0b2659f..f6cd96c 100644
--- a/drivers/video/omap2/dss/sdi.c
+++ b/drivers/video/omap2/dss/sdi.c
@@ -32,18 +32,34 @@
 static struct {
 	bool update_enabled;
 	struct regulator *vdds_sdi_reg;
-} sdi;
 
-static void sdi_basic_init(struct omap_dss_device *dssdev)
+	struct dss_lcd_mgr_config mgr_config;
+} sdi;
 
+static void sdi_config_lcd_manager(struct omap_dss_device *dssdev)
 {
-	dispc_mgr_set_io_pad_mode(DSS_IO_PAD_MODE_BYPASS);
-	dispc_mgr_enable_stallmode(dssdev->manager->id, false);
+	sdi.mgr_config.io_pad_mode = DSS_IO_PAD_MODE_BYPASS;
 
-	dispc_mgr_set_lcd_type_tft(dssdev->manager->id);
+	sdi.mgr_config.stallmode = false;
+	sdi.mgr_config.fifohandcheck = false;
+
+	sdi.mgr_config.video_port_width = 24;
+	sdi.mgr_config.lcden_sig_polarity = 1;
+
+	dispc_mgr_set_io_pad_mode(sdi.mgr_config.io_pad_mode);
+	dispc_mgr_enable_stallmode(dssdev->manager->id,
+			sdi.mgr_config.stallmode);
+	dispc_mgr_enable_fifohandcheck(dssdev->manager->id,
+			sdi.mgr_config.fifohandcheck);
 
-	dispc_mgr_set_tft_data_lines(dssdev->manager->id, 24);
-	dispc_lcd_enable_signal_polarity(1);
+	dispc_mgr_set_clock_div(dssdev->manager->id,
+			&sdi.mgr_config.clock_info);
+
+	dispc_mgr_set_tft_data_lines(dssdev->manager->id,
+			sdi.mgr_config.video_port_width);
+	dispc_lcd_enable_signal_polarity(sdi.mgr_config.lcden_sig_polarity);
+
+	dispc_mgr_set_lcd_type_tft(dssdev->manager->id);
 }
 
 int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
@@ -51,8 +67,6 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
 	struct omap_video_timings *t = &dssdev->panel.timings;
 	struct dss_clock_info dss_cinfo;
 	struct dispc_clock_info dispc_cinfo;
-	u16 lck_div, pck_div;
-	unsigned long fck;
 	unsigned long pck;
 	int r;
 
@@ -75,8 +89,6 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
 	if (r)
 		goto err_get_dispc;
 
-	sdi_basic_init(dssdev);
-
 	/* 15.5.9.1.2 */
 	dssdev->panel.timings.pclk_edge = OMAPDSS_DRIVE_SIG_RISING_EDGE;
 	dssdev->panel.timings.hsync_vsync_edge = OMAPDSS_DRIVE_SIG_RISING_EDGE;
@@ -85,11 +97,9 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
 	if (r)
 		goto err_calc_clock_div;
 
-	fck = dss_cinfo.fck;
-	lck_div = dispc_cinfo.lck_div;
-	pck_div = dispc_cinfo.pck_div;
+	sdi.mgr_config.clock_info = dispc_cinfo;
 
-	pck = fck / lck_div / pck_div / 1000;
+	pck = dss_cinfo.fck / dispc_cinfo.lck_div / dispc_cinfo.pck_div / 1000;
 
 	if (pck != t->pixel_clock) {
 		DSSWARN("Could not find exact pixel clock. Requested %d kHz, "
@@ -106,7 +116,7 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
 	if (r)
 		goto err_set_dss_clock_div;
 
-	dispc_mgr_set_clock_div(dssdev->manager->id, &dispc_cinfo);
+	sdi_config_lcd_manager(dssdev);
 
 	dss_sdi_init(dssdev->phy.sdi.datapairs);
 	r = dss_sdi_enable();
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 07/12] OMAPDSS: APPLY: Remove DISPC writes to manager's lcd parameters in interface drivers
From: Archit Taneja @ 2012-06-28 14:42 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1340893842-10626-1-git-send-email-archit@ti.com>

Replace the DISPC fuctions used to configure LCD channel related manager
parameters with dss_mgr_set_lcd_config() in APPLY. This function ensures that
the DISPC registers are written at the right time by using the shadow register
programming model.

The LCD manager configurations is stored as a private data of manager in APPLY.
It is treated as an extra info as it's the panel drivers which trigger this
apply via interface drivers, and not a DSS2 user like omapfb or omapdrm.

Storing LCD manager related properties in APPLY also prevents the need to refer
to the panel connected to the manager for information. This helps in making the
DSS driver less dependent on panel.

A helper function is added to check whether the manager is LCD or TV. The direct
DISPC register writes are removed from the interface drivers.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c |   65 +++++++++++++++++++++++++++++++++++++++
 drivers/video/omap2/dss/dpi.c   |   16 +---------
 drivers/video/omap2/dss/dsi.c   |   17 +---------
 drivers/video/omap2/dss/dss.h   |   10 ++++++
 drivers/video/omap2/dss/rfbi.c  |   13 +-------
 drivers/video/omap2/dss/sdi.c   |   15 +--------
 6 files changed, 79 insertions(+), 57 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 3ce7a3e..bb4ec63 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -104,6 +104,7 @@ struct mgr_priv_data {
 	bool shadow_extra_info_dirty;
 
 	struct omap_video_timings timings;
+	struct dss_lcd_mgr_config lcd_config;
 };
 
 static struct {
@@ -137,6 +138,7 @@ static struct mgr_priv_data *get_mgr_priv(struct omap_overlay_manager *mgr)
 void dss_apply_init(void)
 {
 	const int num_ovls = dss_feat_get_num_ovls();
+	struct mgr_priv_data *mp;
 	int i;
 
 	spin_lock_init(&data_lock);
@@ -168,6 +170,16 @@ void dss_apply_init(void)
 
 		op->user_info = op->info;
 	}
+
+	/*
+	 * Initialize some of the lcd_config fields for TV manager, this lets
+	 * us prevent checking if the manager is LCD or TV at some places
+	 */
+	mp = &dss_data.mgr_priv_data_array[OMAP_DSS_CHANNEL_DIGIT];
+
+	mp->lcd_config.video_port_width = 24;
+	mp->lcd_config.clock_info.lck_div = 1;
+	mp->lcd_config.clock_info.pck_div = 1;
 }
 
 static bool ovl_manual_update(struct omap_overlay *ovl)
@@ -633,6 +645,24 @@ 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);
+	}
+
 	mp->extra_info_dirty = false;
 	if (mp->updating)
 		mp->shadow_extra_info_dirty = true;
@@ -1292,6 +1322,41 @@ void dss_mgr_set_timings(struct omap_overlay_manager *mgr,
 	mutex_unlock(&apply_lock);
 }
 
+static void dss_apply_mgr_lcd_config(struct omap_overlay_manager *mgr,
+		struct dss_lcd_mgr_config config)
+{
+	struct mgr_priv_data *mp = get_mgr_priv(mgr);
+
+	mp->lcd_config = config;
+	mp->extra_info_dirty = true;
+}
+
+void dss_mgr_set_lcd_config(struct omap_overlay_manager *mgr,
+		struct dss_lcd_mgr_config config)
+{
+	unsigned long flags;
+	struct mgr_priv_data *mp = get_mgr_priv(mgr);
+
+	mutex_lock(&apply_lock);
+
+	if (mp->enabled)
+		goto out;
+
+	spin_lock_irqsave(&data_lock, flags);
+
+	dss_apply_mgr_lcd_config(mgr, config);
+
+	dss_write_regs();
+	dss_set_go_bits();
+
+	spin_unlock_irqrestore(&data_lock, flags);
+
+	wait_pending_extra_info_updates();
+
+out:
+	mutex_unlock(&apply_lock);
+}
+
 int dss_ovl_set_info(struct omap_overlay *ovl,
 		struct omap_overlay_info *info)
 {
diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index a3a012b..c10c0f8 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -179,21 +179,7 @@ static void dpi_config_lcd_manager(struct omap_dss_device *dssdev)
 
 	dpi.mgr_config.lcden_sig_polarity = false;
 
-	dispc_mgr_set_io_pad_mode(dpi.mgr_config.io_pad_mode);
-	dispc_mgr_enable_stallmode(dssdev->manager->id,
-			dpi.mgr_config.stallmode);
-	dispc_mgr_enable_fifohandcheck(dssdev->manager->id,
-			dpi.mgr_config.fifohandcheck);
-
-	dispc_mgr_set_tft_data_lines(dssdev->manager->id,
-			dpi.mgr_config.video_port_width);
-
-	dispc_mgr_set_clock_div(dssdev->manager->id,
-			&dpi.mgr_config.clock_info);
-
-	dispc_lcd_enable_signal_polarity(dpi.mgr_config.lcden_sig_polarity);
-
-	dispc_mgr_set_lcd_type_tft(dssdev->manager->id);
+	dss_mgr_set_lcd_config(dssdev->manager, dpi.mgr_config);
 }
 
 int omapdss_dpi_display_enable(struct omap_dss_device *dssdev)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index df92e24..0ae85e2 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -4444,22 +4444,7 @@ static int dsi_display_init_dispc(struct omap_dss_device *dssdev)
 			dsi_get_pixel_size(dssdev->panel.dsi_pix_fmt);
 	dsi->mgr_config.lcden_sig_polarity = 0;
 
-	dispc_mgr_set_io_pad_mode(dsi->mgr_config.io_pad_mode);
-
-	dispc_mgr_enable_stallmode(dssdev->manager->id,
-			dsi->mgr_config.stallmode);
-	dispc_mgr_enable_fifohandcheck(dssdev->manager->id,
-			dsi->mgr_config.fifohandcheck);
-
-	dispc_mgr_set_clock_div(dssdev->manager->id,
-			&dsi->mgr_config.clock_info);
-
-	dispc_mgr_set_tft_data_lines(dssdev->manager->id,
-			dsi->mgr_config.video_port_width);
-
-	dispc_lcd_enable_signal_polarity(dsi->mgr_config.lcden_sig_polarity);
-
-	dispc_mgr_set_lcd_type_tft(dssdev->manager->id);
+	dss_mgr_set_lcd_config(dssdev->manager, dsi->mgr_config);
 
 	return 0;
 err1:
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 6688eaf..b4c52ec 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -202,6 +202,8 @@ int dss_mgr_set_device(struct omap_overlay_manager *mgr,
 int dss_mgr_unset_device(struct omap_overlay_manager *mgr);
 void dss_mgr_set_timings(struct omap_overlay_manager *mgr,
 		struct omap_video_timings *timings);
+void dss_mgr_set_lcd_config(struct omap_overlay_manager *mgr,
+		struct dss_lcd_mgr_config config);
 const struct omap_video_timings *dss_mgr_get_timings(struct omap_overlay_manager *mgr);
 
 bool dss_ovl_is_enabled(struct omap_overlay *ovl);
@@ -235,6 +237,14 @@ int dss_mgr_check(struct omap_overlay_manager *mgr,
 		const struct omap_video_timings *mgr_timings,
 		struct omap_overlay_info **overlay_infos);
 
+static inline bool dss_mgr_is_lcd(enum omap_channel id)
+{
+	if (id = OMAP_DSS_CHANNEL_LCD || id = OMAP_DSS_CHANNEL_LCD2)
+		return true;
+	else
+		return false;
+}
+
 /* overlay */
 void dss_init_overlays(struct platform_device *pdev);
 void dss_uninit_overlays(struct platform_device *pdev);
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index 847f694..0376056 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -872,18 +872,7 @@ static void rfbi_config_lcd_manager(struct omap_dss_device *dssdev)
 	mgr_config.video_port_width = dssdev->ctrl.pixel_size;
 	mgr_config.lcden_sig_polarity = 0;
 
-	dispc_mgr_set_io_pad_mode(mgr_config.io_pad_mode);
-
-	dispc_mgr_enable_stallmode(dssdev->manager->id, mgr_config.stallmode);
-	dispc_mgr_enable_fifohandcheck(dssdev->manager->id,
-			mgr_config.fifohandcheck);
-
-	dispc_mgr_set_tft_data_lines(dssdev->manager->id,
-			mgr_config.video_port_width);
-
-	dispc_lcd_enable_signal_polarity(mgr_config.lcden_sig_polarity);
-
-	dispc_mgr_set_lcd_type_tft(dssdev->manager->id);
+	dss_mgr_set_lcd_config(dssdev->manager, mgr_config);
 }
 
 int omapdss_rfbi_display_enable(struct omap_dss_device *dssdev)
diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
index f6cd96c..b8dc13c 100644
--- a/drivers/video/omap2/dss/sdi.c
+++ b/drivers/video/omap2/dss/sdi.c
@@ -46,20 +46,7 @@ static void sdi_config_lcd_manager(struct omap_dss_device *dssdev)
 	sdi.mgr_config.video_port_width = 24;
 	sdi.mgr_config.lcden_sig_polarity = 1;
 
-	dispc_mgr_set_io_pad_mode(sdi.mgr_config.io_pad_mode);
-	dispc_mgr_enable_stallmode(dssdev->manager->id,
-			sdi.mgr_config.stallmode);
-	dispc_mgr_enable_fifohandcheck(dssdev->manager->id,
-			sdi.mgr_config.fifohandcheck);
-
-	dispc_mgr_set_clock_div(dssdev->manager->id,
-			&sdi.mgr_config.clock_info);
-
-	dispc_mgr_set_tft_data_lines(dssdev->manager->id,
-			sdi.mgr_config.video_port_width);
-	dispc_lcd_enable_signal_polarity(sdi.mgr_config.lcden_sig_polarity);
-
-	dispc_mgr_set_lcd_type_tft(dssdev->manager->id);
+	dss_mgr_set_lcd_config(dssdev->manager, sdi.mgr_config);
 }
 
 int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 08/12] OMAPDSS: MANAGER: Check LCD related overlay manager parameters
From: Archit Taneja @ 2012-06-28 14:42 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1340893842-10626-1-git-send-email-archit@ti.com>

The LCD related manager configurations are a part of the manager's private data
in APPLY. Pass this to dss_lcd_mgr_config to dss_mgr_check and create a function
to check the validity of some of the configurations.

To check some of the configurations, we require information of interface to
which the manager output is connected. These can be added once interfaces are
represented as an entity.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c   |    2 +-
 drivers/video/omap2/dss/dss.h     |    1 +
 drivers/video/omap2/dss/manager.c |   35 +++++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index bb4ec63..95a6713 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -226,7 +226,7 @@ static int dss_check_settings_low(struct omap_overlay_manager *mgr,
 		ois[ovl->id] = oi;
 	}
 
-	return dss_mgr_check(mgr, mi, &mp->timings, ois);
+	return dss_mgr_check(mgr, mi, &mp->timings, &mp->lcd_config, ois);
 }
 
 /*
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index b4c52ec..cb859e3 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -235,6 +235,7 @@ int dss_mgr_check_timings(struct omap_overlay_manager *mgr,
 int dss_mgr_check(struct omap_overlay_manager *mgr,
 		struct omap_overlay_manager_info *info,
 		const struct omap_video_timings *mgr_timings,
+		const struct dss_lcd_mgr_config *config,
 		struct omap_overlay_info **overlay_infos);
 
 static inline bool dss_mgr_is_lcd(enum omap_channel id)
diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index 0cbcde4..09540bc 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -665,9 +665,40 @@ int dss_mgr_check_timings(struct omap_overlay_manager *mgr,
 	return 0;
 }
 
+static int dss_mgr_check_lcd_config(struct omap_overlay_manager *mgr,
+		const struct dss_lcd_mgr_config *config)
+{
+	struct dispc_clock_info cinfo = config->clock_info;
+	int dl = config->video_port_width;
+	bool stallmode = config->stallmode;
+	bool fifohandcheck = config->fifohandcheck;
+
+	if (cinfo.lck_div < 1 || cinfo.lck_div > 255)
+		return -EINVAL;
+
+	if (cinfo.pck_div < 1 || cinfo.pck_div > 255)
+		return -EINVAL;
+
+	if (dl != 12 && dl != 16 && dl != 18 && dl != 24)
+		return -EINVAL;
+
+	/* fifohandcheck should be used only with stallmode */
+	if (stallmode = false && fifohandcheck = true)
+		return -EINVAL;
+
+	/*
+	 * io pad mode can be only checked by using dssdev connected to the
+	 * manager. Ignore checking these for now, add checks when manager
+	 * is capable of holding information related to the connected interface
+	 */
+
+	return 0;
+}
+
 int dss_mgr_check(struct omap_overlay_manager *mgr,
 		struct omap_overlay_manager_info *info,
 		const struct omap_video_timings *mgr_timings,
+		const struct dss_lcd_mgr_config *lcd_config,
 		struct omap_overlay_info **overlay_infos)
 {
 	struct omap_overlay *ovl;
@@ -683,6 +714,10 @@ int dss_mgr_check(struct omap_overlay_manager *mgr,
 	if (r)
 		return r;
 
+	r = dss_mgr_check_lcd_config(mgr, lcd_config);
+	if (r)
+		return r;
+
 	list_for_each_entry(ovl, &mgr->overlays, list) {
 		struct omap_overlay_info *oi;
 		int r;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 09/12] OMAPDSS: APPLY: Remove usage of omap_dss_device from manual/auto update checks
From: Archit Taneja @ 2012-06-28 14:42 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1340893842-10626-1-git-send-email-archit@ti.com>

APPLY needs to know at certain places whether an overlay manager is in manual
or auto update mode. The caps of the connected omap_dss_device were used to
check that.

A LCD manager is in manual update if stallmode is enabled for that manager. TV
managers for now always auto update.

Return the value of stallmode parameter in the private data 'lcd_confg' in
mgr_manual_update() and ovl_manual_update(), for TV managers stallmode field
will be false by default.

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

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 95a6713..b9c6e75 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -182,14 +182,23 @@ void dss_apply_init(void)
 	mp->lcd_config.clock_info.pck_div = 1;
 }
 
+/*
+ * A LCD manager's stallmode decides whether it is in manual or auto update. TV
+ * manager is always auto update, stallmode field for TV manager is false by
+ * default
+ */
 static bool ovl_manual_update(struct omap_overlay *ovl)
 {
-	return ovl->manager->device->caps & OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE;
+	struct mgr_priv_data *mp = get_mgr_priv(ovl->manager);
+
+	return mp->lcd_config.stallmode;
 }
 
 static bool mgr_manual_update(struct omap_overlay_manager *mgr)
 {
-	return mgr->device->caps & OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE;
+	struct mgr_priv_data *mp = get_mgr_priv(mgr);
+
+	return mp->lcd_config.stallmode;
 }
 
 static int dss_check_settings_low(struct omap_overlay_manager *mgr,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 10/12] OMAPDSS: DISPC: Remove a redundant function
From: Archit Taneja @ 2012-06-28 14:42 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1340893842-10626-1-git-send-email-archit@ti.com>

dss_mgr_is_lcd() available in dss.h does the same thing as dispc_mgr_is_lcd()
in dispc.c. Remove the function from dispc.c and replace it with the one in
dss.h.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index b047fe6..0e0b89c 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -387,15 +387,6 @@ void dispc_runtime_put(void)
 	WARN_ON(r < 0);
 }
 
-static inline bool dispc_mgr_is_lcd(enum omap_channel channel)
-{
-	if (channel = OMAP_DSS_CHANNEL_LCD ||
-			channel = OMAP_DSS_CHANNEL_LCD2)
-		return true;
-	else
-		return false;
-}
-
 u32 dispc_mgr_get_vsync_irq(enum omap_channel channel)
 {
 	switch (channel) {
@@ -430,7 +421,7 @@ bool dispc_mgr_go_busy(enum omap_channel channel)
 {
 	int bit;
 
-	if (dispc_mgr_is_lcd(channel))
+	if (dss_mgr_is_lcd(channel))
 		bit = 5; /* GOLCD */
 	else
 		bit = 6; /* GODIGIT */
@@ -446,7 +437,7 @@ void dispc_mgr_go(enum omap_channel channel)
 	int bit;
 	bool enable_bit, go_bit;
 
-	if (dispc_mgr_is_lcd(channel))
+	if (dss_mgr_is_lcd(channel))
 		bit = 0; /* LCDENABLE */
 	else
 		bit = 1; /* DIGITALENABLE */
@@ -460,7 +451,7 @@ void dispc_mgr_go(enum omap_channel channel)
 	if (!enable_bit)
 		return;
 
-	if (dispc_mgr_is_lcd(channel))
+	if (dss_mgr_is_lcd(channel))
 		bit = 5; /* GOLCD */
 	else
 		bit = 6; /* GODIGIT */
@@ -939,7 +930,7 @@ static void dispc_mgr_set_cpr_coef(enum omap_channel channel,
 {
 	u32 coef_r, coef_g, coef_b;
 
-	if (!dispc_mgr_is_lcd(channel))
+	if (!dss_mgr_is_lcd(channel))
 		return;
 
 	coef_r = FLD_VAL(coefs->rr, 31, 22) | FLD_VAL(coefs->rg, 20, 11) |
@@ -1798,7 +1789,7 @@ static int check_horiz_timing_omap3(enum omap_channel channel,
 
 	nonactive = t->x_res + t->hfp + t->hsw + t->hbp - out_width;
 	pclk = dispc_mgr_pclk_rate(channel);
-	if (dispc_mgr_is_lcd(channel))
+	if (dss_mgr_is_lcd(channel))
 		lclk = dispc_mgr_lclk_rate(channel);
 	else
 		lclk = dispc_fclk_rate();
@@ -2398,7 +2389,7 @@ bool dispc_mgr_is_enabled(enum omap_channel channel)
 
 void dispc_mgr_enable(enum omap_channel channel, bool enable)
 {
-	if (dispc_mgr_is_lcd(channel))
+	if (dss_mgr_is_lcd(channel))
 		dispc_mgr_enable_lcd_out(channel, enable);
 	else if (channel = OMAP_DSS_CHANNEL_DIGIT)
 		dispc_mgr_enable_digit_out(enable);
@@ -2610,7 +2601,7 @@ bool dispc_mgr_timings_ok(enum omap_channel channel,
 
 	timings_ok = _dispc_mgr_size_ok(timings->x_res, timings->y_res);
 
-	if (dispc_mgr_is_lcd(channel))
+	if (dss_mgr_is_lcd(channel))
 		timings_ok =  timings_ok && _dispc_lcd_timings_ok(timings->hsw,
 						timings->hfp, timings->hbp,
 						timings->vsw, timings->vfp,
@@ -2702,7 +2693,7 @@ void dispc_mgr_set_timings(enum omap_channel channel,
 		return;
 	}
 
-	if (dispc_mgr_is_lcd(channel)) {
+	if (dss_mgr_is_lcd(channel)) {
 		_dispc_mgr_set_lcd_timings(channel, t.hsw, t.hfp, t.hbp, t.vsw,
 				t.vfp, t.vbp, t.vsync_level, t.hsync_level,
 				t.pclk_edge, t.de_level, t.hsync_vsync_edge);
@@ -2808,7 +2799,7 @@ unsigned long dispc_mgr_pclk_rate(enum omap_channel channel)
 {
 	unsigned long r;
 
-	if (dispc_mgr_is_lcd(channel)) {
+	if (dss_mgr_is_lcd(channel)) {
 		int pcd;
 		u32 l;
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 11/12] OMAPDSS: RFBI: Use dss_mgr_enable to enable the overlay manager
From: Archit Taneja @ 2012-06-28 14:42 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1340893842-10626-1-git-send-email-archit@ti.com>

The RFBI driver uses a direct DISPC register write to enable the overlay
manager. Replace this with dss_mgr_enable() which checks if the connected
overlay and managers are correctly configured, and configure DSS for
fifomerge.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/rfbi.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index 0376056..8c5d5df 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -300,10 +300,11 @@ void omap_rfbi_write_pixels(const void __iomem *buf, int scr_width,
 }
 EXPORT_SYMBOL(omap_rfbi_write_pixels);
 
-static void rfbi_transfer_area(struct omap_dss_device *dssdev, u16 width,
+static int rfbi_transfer_area(struct omap_dss_device *dssdev, u16 width,
 		u16 height, void (*callback)(void *data), void *data)
 {
 	u32 l;
+	int r;
 	struct omap_video_timings timings = {
 		.hsw		= 1,
 		.hfp		= 1,
@@ -322,7 +323,9 @@ static void rfbi_transfer_area(struct omap_dss_device *dssdev, u16 width,
 
 	dss_mgr_set_timings(dssdev->manager, &timings);
 
-	dispc_mgr_enable(dssdev->manager->id, true);
+	r = dss_mgr_enable(dssdev->manager);
+	if (r)
+		return r;
 
 	rfbi.framedone_callback = callback;
 	rfbi.framedone_callback_data = data;
@@ -335,6 +338,8 @@ static void rfbi_transfer_area(struct omap_dss_device *dssdev, u16 width,
 		l = FLD_MOD(l, 1, 4, 4); /* ITE */
 
 	rfbi_write_reg(RFBI_CONTROL, l);
+
+	return 0;
 }
 
 static void framedone_callback(void *data, u32 mask)
@@ -814,8 +819,11 @@ int omap_rfbi_update(struct omap_dss_device *dssdev,
 		u16 x, u16 y, u16 w, u16 h,
 		void (*callback)(void *), void *data)
 {
-	rfbi_transfer_area(dssdev, w, h, callback, data);
-	return 0;
+	int r;
+
+	r = rfbi_transfer_area(dssdev, w, h, callback, data);
+
+	return r;
 }
 EXPORT_SYMBOL(omap_rfbi_update);
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 12/12] OMAPDSS: OVERLAY: Clean up replication checking
From: Archit Taneja @ 2012-06-28 14:42 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1340893842-10626-1-git-send-email-archit@ti.com>

Replication logic for an overlay depends on the color mode in which it is
configured and the video port width of the manager it is connected to.

video port width now held in dss_lcd_mgr_config in the manager's private
data in APPLY. Use this instead of referring to the omap_dss_device connected to
the manager.

Replication is enabled in the case of TV manager, the video_port_width is set to
a default value of 24 for TV manager.

Make the replication checking an overlay function since it's more of an overlay
characteristic than a display characteristic.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/apply.c   |    2 +-
 drivers/video/omap2/dss/display.c |   34 ----------------------------------
 drivers/video/omap2/dss/dss.h     |    4 ++--
 drivers/video/omap2/dss/overlay.c |   13 +++++++++++++
 4 files changed, 16 insertions(+), 37 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index b9c6e75..89ee677 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -571,7 +571,7 @@ static void dss_ovl_write_regs(struct omap_overlay *ovl)
 
 	mp = get_mgr_priv(ovl->manager);
 
-	replication = dss_use_replication(ovl->manager->device, oi->color_mode);
+	replication = dss_ovl_use_replication(mp->lcd_config, oi->color_mode);
 
 	r = dispc_ovl_setup(ovl->id, oi, replication, &mp->timings);
 	if (r) {
diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
index 1d8198e..9433b92 100644
--- a/drivers/video/omap2/dss/display.c
+++ b/drivers/video/omap2/dss/display.c
@@ -316,40 +316,6 @@ void omapdss_default_get_timings(struct omap_dss_device *dssdev,
 }
 EXPORT_SYMBOL(omapdss_default_get_timings);
 
-/* Checks if replication logic should be used. Only use for active matrix,
- * when overlay is in RGB12U or RGB16 mode, and LCD interface is
- * 18bpp or 24bpp */
-bool dss_use_replication(struct omap_dss_device *dssdev,
-		enum omap_color_mode mode)
-{
-	int bpp;
-
-	if (mode != OMAP_DSS_COLOR_RGB12U && mode != OMAP_DSS_COLOR_RGB16)
-		return false;
-
-	switch (dssdev->type) {
-	case OMAP_DISPLAY_TYPE_DPI:
-		bpp = dssdev->phy.dpi.data_lines;
-		break;
-	case OMAP_DISPLAY_TYPE_HDMI:
-	case OMAP_DISPLAY_TYPE_VENC:
-	case OMAP_DISPLAY_TYPE_SDI:
-		bpp = 24;
-		break;
-	case OMAP_DISPLAY_TYPE_DBI:
-		bpp = dssdev->ctrl.pixel_size;
-		break;
-	case OMAP_DISPLAY_TYPE_DSI:
-		bpp = dsi_get_pixel_size(dssdev->panel.dsi_pix_fmt);
-		break;
-	default:
-		BUG();
-		return false;
-	}
-
-	return bpp > 16;
-}
-
 void dss_init_device(struct platform_device *pdev,
 		struct omap_dss_device *dssdev)
 {
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index cb859e3..018b27c 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -222,8 +222,6 @@ void dss_init_device(struct platform_device *pdev,
 		struct omap_dss_device *dssdev);
 void dss_uninit_device(struct platform_device *pdev,
 		struct omap_dss_device *dssdev);
-bool dss_use_replication(struct omap_dss_device *dssdev,
-		enum omap_color_mode mode);
 
 /* manager */
 int dss_init_overlay_managers(struct platform_device *pdev);
@@ -255,6 +253,8 @@ int dss_ovl_simple_check(struct omap_overlay *ovl,
 		const struct omap_overlay_info *info);
 int dss_ovl_check(struct omap_overlay *ovl, struct omap_overlay_info *info,
 		const struct omap_video_timings *mgr_timings);
+bool dss_ovl_use_replication(struct dss_lcd_mgr_config config,
+		enum omap_color_mode mode);
 
 /* DSS */
 int dss_init_platform_driver(void) __init;
diff --git a/drivers/video/omap2/dss/overlay.c b/drivers/video/omap2/dss/overlay.c
index c492bb0..0a129dc 100644
--- a/drivers/video/omap2/dss/overlay.c
+++ b/drivers/video/omap2/dss/overlay.c
@@ -677,3 +677,16 @@ int dss_ovl_check(struct omap_overlay *ovl, struct omap_overlay_info *info,
 
 	return 0;
 }
+
+/*
+ * Checks if replication logic should be used. Only use when overlay is in
+ * RGB12U or RGB16 mode, and video port width interface is 18bpp or 24bpp
+ */
+bool dss_ovl_use_replication(struct dss_lcd_mgr_config config,
+		enum omap_color_mode mode)
+{
+	if (mode != OMAP_DSS_COLOR_RGB12U && mode != OMAP_DSS_COLOR_RGB16)
+		return false;
+
+	return config.video_port_width > 16;
+}
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
From: Tomi Valkeinen @ 2012-06-28 15:14 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne,
	patches
In-Reply-To: <CAJe_Zhdkhu4TciiwGJB2Kz8oZp2RQNZfESSr5Cfv0R1MNn=r9A@mail.gmail.com>

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

On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:

> A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
> probe and disable during remove.
> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
> HDMI_PHY on/off.
> The user selecting "Autodetect and Configure" option would then equate
> to "(un)loading" of the HDMI driver.
> Not to mean a trivial job.

One more thing I realized while thinking about this:

While it could be argued that the power draw from having the tpd12s015
always enabled is very small, I think it could matter. If you consider a
phone with HDMI output, it's likely that the phone is locked 99% of the
time. When the phone is locked, there's no need to keep the HDMI HPD
enabled. So this could add to a considerable amount of power wasted, if
the HPD was always enabled.

At least I can't figure out a reason why one would want the HPD to work
when the phone is locked. Also, I have never used the HDMI output on my
phone, so I'd be glad if it was totally powered off if it gave me more
standby hours =).

 Tomi


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

^ permalink raw reply

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

On 28 June 2012 19:01, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:
>> On 28 June 2012 17:33, Andy Green <andy.green@linaro.org> wrote:
>
>> > If Jassi's alright with it we might have a go at implementing this, but can
>> > you define a bit more about how we logically tell DSS that we want to, eg,
>> > disable HDMI totally?
>> >
>> A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
>> probe and disable during remove.
>
> The problem with this is a feature of omapdss: we can have multiple
> displays for the same output, of which only one can be enabled at the
> same time. What this means is that you shouldn't (and in some cases
> can't) allocate or enable resources in probe that may be shared, because
> then the driver for both displays would try to allocate the same
> resource.
>
> Sure, this is not a problem for the HDMI configuration we are using now,
> but it's still against the panel model we have. Thus we should allocate
> resources only when the panel device is turned on, and release them when
> it's disabled.
>
> I do think the model is slightly broken, but that's what we have now.
> And I'm also not even sure how it should be fixed...
>
I won't press further with my Utopian ideas, but I think we need to
segregate 5V/HPD enabling from PHY enabling somehow. Because that is
already failing slow but otherwise perfectly legit displays (like
Andy's "HPD taking 700ms" TV)


> And also, as I said earlier, if you keep it enabled all the time, it'll
> eat power even if the user is never going to use HDMI.
>
> On a desktop I guess the power consumption wouldn't be an issue, but I
> do feel a bit uneasy about it on an embedded device.
>
As I said, it should be platform dependent. If a device doesn't have
HDMI port, the board file would not even have platform_enable. And if
it has, some user action should enable it while 'making the device
ready for new display'.
IOW, how do you envision an OMAP4 based tablet with HDMI port react to
display connections ?


>> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
>> HDMI_PHY on/off.
>> The user selecting "Autodetect and Configure" option would then equate
>> to "(un)loading" of the HDMI driver.
>
> HDMI cannot be currently compiled as a separate module. Although I think
> you can detach a device and a driver, achieving the same. Is that what
> you meant with unloading?
>
Yeah, I meant something to the effect of bringing HDMI driver to life.


> By the way, when the device is in system suspend, we surely won't detect
> the HPD even if we kept the HPD always enabled. So there we'll miss the
> HPD interrupt anyway, and the EDID cache would be invalid.
>
If omapdss already handles the possibility of display changed during
suspend, I think we should be good :)

^ permalink raw reply

* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
From: Tomi Valkeinen @ 2012-06-28 15:27 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne,
	patches
In-Reply-To: <CAJe_ZheBbj3qoECDokcxHsB4UOx354G5t_MfYiZ3UEm9xK=tqw@mail.gmail.com>

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

On Thu, 2012-06-28 at 20:44 +0530, Jassi Brar wrote:

> I won't press further with my Utopian ideas, but I think we need to
> segregate 5V/HPD enabling from PHY enabling somehow. Because that is
> already failing slow but otherwise perfectly legit displays (like
> Andy's "HPD taking 700ms" TV)

Yes, I agree. That's what the four power states I suggested in the other
mail were about. The second power state would have HPD enabled, and the
third would enable the PHY. Or at least enough to do i2c transfers, I'm
not sure if the PHY is needed for that.

> > And also, as I said earlier, if you keep it enabled all the time, it'll
> > eat power even if the user is never going to use HDMI.
> >
> > On a desktop I guess the power consumption wouldn't be an issue, but I
> > do feel a bit uneasy about it on an embedded device.
> >
> As I said, it should be platform dependent. If a device doesn't have
> HDMI port, the board file would not even have platform_enable. And if

Not that it's relevant here, but I have a patch series where I remove
the platform_enable stuff for HDMI, and move the work to the hdmi
driver. That needs to be done for device tree stuff, when we don't have
board files.

> it has, some user action should enable it while 'making the device
> ready for new display'.
> IOW, how do you envision an OMAP4 based tablet with HDMI port react to
> display connections ?

I guess this was covered in my mail about the phone's HDMI. If the
tablet is unlocked, and I plug in a HDMI cable, I expect the device to
do something. Either clone the display, or perhaps ask me what I want to
do.

So yes, HPD would be always enabled, when the tablet is active
(unlocked).

> >> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
> >> HDMI_PHY on/off.
> >> The user selecting "Autodetect and Configure" option would then equate
> >> to "(un)loading" of the HDMI driver.
> >
> > HDMI cannot be currently compiled as a separate module. Although I think
> > you can detach a device and a driver, achieving the same. Is that what
> > you meant with unloading?
> >
> Yeah, I meant something to the effect of bringing HDMI driver to life.
> 
> 
> > By the way, when the device is in system suspend, we surely won't detect
> > the HPD even if we kept the HPD always enabled. So there we'll miss the
> > HPD interrupt anyway, and the EDID cache would be invalid.
> >
> If omapdss already handles the possibility of display changed during
> suspend, I think we should be good :)

Hmm I'm not sure I understand what you mean. I was referring to your
patch, which invalidated the EDID cache only on HPD interrupt when the
cable is unplugged. And we'd miss that interrupt when the board is in
system suspend, even if we otherwise kept the HPD interrupt always
enabled.

 Tomi


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

^ permalink raw reply

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

On 28 June 2012 20:44, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:
>
>> A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
>> probe and disable during remove.
>> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
>> HDMI_PHY on/off.
>> The user selecting "Autodetect and Configure" option would then equate
>> to "(un)loading" of the HDMI driver.
>> Not to mean a trivial job.
>
> One more thing I realized while thinking about this:
>
> While it could be argued that the power draw from having the tpd12s015
> always enabled is very small, I think it could matter. If you consider a
> phone with HDMI output, it's likely that the phone is locked 99% of the
> time. When the phone is locked, there's no need to keep the HDMI HPD
> enabled. So this could add to a considerable amount of power wasted, if
> the HPD was always enabled.
>
> At least I can't figure out a reason why one would want the HPD to work
> when the phone is locked. Also, I have never used the HDMI output on my
> phone, so I'd be glad if it was totally powered off if it gave me more
> standby hours =).
>
Of course, I don't suggest imposing any hard rule here.
All I suggest is make it platform dependent and provide a way from
user-space too to enable/disable HPD.

^ permalink raw reply

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

On 28 June 2012 20:57, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-06-28 at 20:44 +0530, Jassi Brar wrote:

>> it has, some user action should enable it while 'making the device
>> ready for new display'.
>> IOW, how do you envision an OMAP4 based tablet with HDMI port react to
>> display connections ?
>
> I guess this was covered in my mail about the phone's HDMI. If the
> tablet is unlocked, and I plug in a HDMI cable, I expect the device to
> do something. Either clone the display, or perhaps ask me what I want to
> do.
>
> So yes, HPD would be always enabled, when the tablet is active
> (unlocked).
>
OK, somehow I was under impression you didn't wanna spare even the 5V+ floating.
Though there could also be some option in settings to enable 5/HPD
only when the user is about the connect a display... so that the
activate window is even narrowed down. Anyway... I am glad we are in
sync.

>> > By the way, when the device is in system suspend, we surely won't detect
>> > the HPD even if we kept the HPD always enabled. So there we'll miss the
>> > HPD interrupt anyway, and the EDID cache would be invalid.
>> >
>> If omapdss already handles the possibility of display changed during
>> suspend, I think we should be good :)
>
> Hmm I'm not sure I understand what you mean. I was referring to your
> patch, which invalidated the EDID cache only on HPD interrupt when the
> cable is unplugged. And we'd miss that interrupt when the board is in
> system suspend, even if we otherwise kept the HPD interrupt always
> enabled.
>
I meant before stale-edid, we face potential problem of omapdss
behaving badly to the displays switched during suspend ?

^ permalink raw reply

* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
From: Jassi Brar @ 2012-06-28 16:32 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne,
	patches
In-Reply-To: <CAJe_Zhdnc8TOY8hDiuZfjto9fTWnT8piGpjczApzfLVMnCcE7w@mail.gmail.com>

On 28 June 2012 21:18, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> On 28 June 2012 20:57, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

>>> > By the way, when the device is in system suspend, we surely won't detect
>>> > the HPD even if we kept the HPD always enabled. So there we'll miss the
>>> > HPD interrupt anyway, and the EDID cache would be invalid.
>>> >
>>> If omapdss already handles the possibility of display changed during
>>> suspend, I think we should be good :)
>>
>> Hmm I'm not sure I understand what you mean. I was referring to your
>> patch, which invalidated the EDID cache only on HPD interrupt when the
>> cable is unplugged. And we'd miss that interrupt when the board is in
>> system suspend, even if we otherwise kept the HPD interrupt always
>> enabled.
>>
> I meant before stale-edid, we face potential problem of omapdss
> behaving badly to the displays switched during suspend ?
>
OK, I think I get now what you mean. We do need to invalidate
edid-cache in the suspend path, irrespective of how omapdss behaves.

Thanks.

^ permalink raw reply

* Re: [PATCH 2/3] OMAPDSS: HDMI: Replace spinlock with mutex in hdmi_check_hpd_state
From: Tomi Valkeinen @ 2012-06-29  6:45 UTC (permalink / raw)
  To: jaswinder.singh
  Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne,
	patches
In-Reply-To: <1340805896-28731-1-git-send-email-jaswinder.singh@linaro.org>

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

On Wed, 2012-06-27 at 19:34 +0530, jaswinder.singh@linaro.org wrote:
> 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;

This introduced a new warning:

drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c: In function 'hdmi_check_hpd_state':
drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c:238:16: warning: unused variable 'flags'

I fixed that.

I'll apply this and the first patch, as I don't think there are any
questions about these.

 Tomi


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

^ permalink raw reply

* Re: [PATCH 07/12] OMAPDSS: APPLY: Remove DISPC writes to manager's lcd parameters in interface drive
From: Tomi Valkeinen @ 2012-06-29 10:12 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-fbdev, linux-omap
In-Reply-To: <1340893842-10626-9-git-send-email-archit@ti.com>

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

On Thu, 2012-06-28 at 20:00 +0530, Archit Taneja wrote:
> Replace the DISPC fuctions used to configure LCD channel related manager
> parameters with dss_mgr_set_lcd_config() in APPLY. This function ensures that
> the DISPC registers are written at the right time by using the shadow register
> programming model.
> 
> The LCD manager configurations is stored as a private data of manager in APPLY.
> It is treated as an extra info as it's the panel drivers which trigger this
> apply via interface drivers, and not a DSS2 user like omapfb or omapdrm.
> 
> Storing LCD manager related properties in APPLY also prevents the need to refer
> to the panel connected to the manager for information. This helps in making the
> DSS driver less dependent on panel.
> 
> A helper function is added to check whether the manager is LCD or TV. The direct
> DISPC register writes are removed from the interface drivers.


 
> +static void dss_apply_mgr_lcd_config(struct omap_overlay_manager *mgr,
> +		struct dss_lcd_mgr_config config)
> +{

This one should take a pointer to the config, not a copy (and const).

> +	struct mgr_priv_data *mp = get_mgr_priv(mgr);
> +
> +	mp->lcd_config = config;
> +	mp->extra_info_dirty = true;
> +}
> +
> +void dss_mgr_set_lcd_config(struct omap_overlay_manager *mgr,
> +		struct dss_lcd_mgr_config config)

And this.

> +{
> +	unsigned long flags;
> +	struct mgr_priv_data *mp = get_mgr_priv(mgr);
> +
> +	mutex_lock(&apply_lock);
> +
> +	if (mp->enabled)
> +		goto out;

Hmm. Should we print a warning or such here? Isn't it a bug in the
interface driver, if it tries to set the lcd config when the output is
enabled?

 Tomi


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

^ permalink raw reply

* Re: [PATCH 07/12] OMAPDSS: APPLY: Remove DISPC writes to manager's lcd parameters in interface drive
From: Archit Taneja @ 2012-06-29 10:40 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap
In-Reply-To: <1340964745.1866.26.camel@lappyti>

On Friday 29 June 2012 03:42 PM, Tomi Valkeinen wrote:
> On Thu, 2012-06-28 at 20:00 +0530, Archit Taneja wrote:
>> Replace the DISPC fuctions used to configure LCD channel related manager
>> parameters with dss_mgr_set_lcd_config() in APPLY. This function ensures that
>> the DISPC registers are written at the right time by using the shadow register
>> programming model.
>>
>> The LCD manager configurations is stored as a private data of manager in APPLY.
>> It is treated as an extra info as it's the panel drivers which trigger this
>> apply via interface drivers, and not a DSS2 user like omapfb or omapdrm.
>>
>> Storing LCD manager related properties in APPLY also prevents the need to refer
>> to the panel connected to the manager for information. This helps in making the
>> DSS driver less dependent on panel.
>>
>> A helper function is added to check whether the manager is LCD or TV. The direct
>> DISPC register writes are removed from the interface drivers.
>
>
>
>> +static void dss_apply_mgr_lcd_config(struct omap_overlay_manager *mgr,
>> +		struct dss_lcd_mgr_config config)
>> +{
>
> This one should take a pointer to the config, not a copy (and const).
>
>> +	struct mgr_priv_data *mp = get_mgr_priv(mgr);
>> +
>> +	mp->lcd_config = config;
>> +	mp->extra_info_dirty = true;
>> +}
>> +
>> +void dss_mgr_set_lcd_config(struct omap_overlay_manager *mgr,
>> +		struct dss_lcd_mgr_config config)
>
> And this.

Will fix these.

>
>> +{
>> +	unsigned long flags;
>> +	struct mgr_priv_data *mp = get_mgr_priv(mgr);
>> +
>> +	mutex_lock(&apply_lock);
>> +
>> +	if (mp->enabled)
>> +		goto out;
>
> Hmm. Should we print a warning or such here? Isn't it a bug in the
> interface driver, if it tries to set the lcd config when the output is
> enabled?

Yes, we should add an error here. Will add it.

Archit

^ permalink raw reply

* [PATCH] pwm-backlight: add regulator and GPIO support
From: Alexandre Courbot @ 2012-06-29 13:22 UTC (permalink / raw)
  To: Thierry Reding; +Cc: linux-tegra, linux-kernel, linux-fbdev, Alexandre Courbot

Add support for an optional power regulator and enable/disable GPIO.
This scheme is commonly used in embedded systems.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 .../bindings/video/backlight/pwm-backlight         |  4 +
 drivers/video/backlight/pwm_bl.c                   | 86 ++++++++++++++++++----
 include/linux/pwm_backlight.h                      |  5 ++
 3 files changed, 79 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight b/Documentation/devicetree/bindings/video/backlight/pwm-backlight
index 1e4fc72..85cbb7b 100644
--- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight
+++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight
@@ -14,6 +14,8 @@ Required properties:
 Optional properties:
   - pwm-names: a list of names for the PWM devices specified in the
                "pwms" property (see PWM binding[0])
+  - power-supply: a reference to a regulator used to control the backlight power
+  - enable-gpios: a reference to a GPIO used to enable/disable the backlight
 
 [0]: Documentation/devicetree/bindings/pwm/pwm.txt
 
@@ -25,4 +27,6 @@ Example:
 
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <6>;
+		power-supply = <&backlight_reg>;
+		enable-gpios = <&gpio 6 0>;
 	};
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 057389d..821e03e 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -20,6 +20,8 @@
 #include <linux/pwm.h>
 #include <linux/pwm_backlight.h>
 #include <linux/slab.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
 
 struct pwm_bl_data {
 	struct pwm_device	*pwm;
@@ -27,6 +29,9 @@ struct pwm_bl_data {
 	unsigned int		period;
 	unsigned int		lth_brightness;
 	unsigned int		*levels;
+	bool			enabled;
+	struct regulator	*power_reg;
+	int			enable_gpio;
 	int			(*notify)(struct device *,
 					  int brightness);
 	void			(*notify_after)(struct device *,
@@ -35,6 +40,40 @@ struct pwm_bl_data {
 	void			(*exit)(struct device *);
 };
 
+static void pwm_backlight_off(struct pwm_bl_data *pb)
+{
+	if (!pb->enabled)
+		return;
+
+	if (gpio_is_valid(pb->enable_gpio))
+		gpio_set_value_cansleep(pb->enable_gpio, 0);
+
+	if (pb->power_reg)
+		regulator_disable(pb->power_reg);
+
+	pwm_config(pb->pwm, 0, pb->period);
+	pwm_disable(pb->pwm);
+
+	pb->enabled = false;
+}
+
+static void pwm_backlight_on(struct pwm_bl_data *pb, int brightness)
+{
+	pwm_config(pb->pwm, brightness, pb->period);
+	pwm_enable(pb->pwm);
+
+	if (pb->enabled)
+		return;
+
+	if (pb->power_reg)
+		regulator_enable(pb->power_reg);
+
+	if (gpio_is_valid(pb->enable_gpio))
+		gpio_set_value_cansleep(pb->enable_gpio, 1);
+
+	pb->enabled = true;
+}
+
 static int pwm_backlight_update_status(struct backlight_device *bl)
 {
 	struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
@@ -51,8 +90,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
 		brightness = pb->notify(pb->dev, brightness);
 
 	if (brightness = 0) {
-		pwm_config(pb->pwm, 0, pb->period);
-		pwm_disable(pb->pwm);
+		pwm_backlight_off(pb);
 	} else {
 		if (pb->levels) {
 			brightness = pb->levels[brightness];
@@ -61,8 +99,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
 
 		brightness = pb->lth_brightness +
 			(brightness * (pb->period - pb->lth_brightness) / max);
-		pwm_config(pb->pwm, brightness, pb->period);
-		pwm_enable(pb->pwm);
+		pwm_backlight_on(pb, brightness);
 	}
 
 	if (pb->notify_after)
@@ -141,11 +178,14 @@ static int pwm_backlight_parse_dt(struct device *dev,
 		data->max_brightness--;
 	}
 
-	/*
-	 * TODO: Most users of this driver use a number of GPIOs to control
-	 *       backlight power. Support for specifying these needs to be
-	 *       added.
-	 */
+	ret = of_get_named_gpio(node, "enable-gpios", 0);
+	if (ret >= 0) {
+		data->enable_gpio = of_get_named_gpio(node, "enable-gpios", 0);
+		data->use_enable_gpio = true;
+	} else if (ret != -ENOENT) {
+		/* GPIO is optional, so ENOENT is not an error here */
+		return ret;
+	}
 
 	return 0;
 }
@@ -176,7 +216,9 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 
 	if (!data) {
 		ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
-		if (ret < 0) {
+		if (ret = -EPROBE_DEFER) {
+			return ret;
+		} else if (ret < 0) {
 			dev_err(&pdev->dev, "failed to find platform data\n");
 			return ret;
 		}
@@ -221,8 +263,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 		}
 	}
 
-	dev_dbg(&pdev->dev, "got pwm for backlight\n");
-
 	/*
 	 * The DT case will set the pwm_period_ns field to 0 and store the
 	 * period, parsed from the DT, in the PWM device. For the non-DT case,
@@ -231,6 +271,22 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 	if (data->pwm_period_ns > 0)
 		pwm_set_period(pb->pwm, data->pwm_period_ns);
 
+
+	pb->power_reg = devm_regulator_get(&pdev->dev, "power");
+	if (IS_ERR(pb->power_reg))
+		return PTR_ERR(pb->power_reg);
+
+	pb->enable_gpio = -EINVAL;
+	if (data->use_enable_gpio) {
+		ret = devm_gpio_request_one(&pdev->dev, data->enable_gpio,
+				GPIOF_OUT_INIT_HIGH, "backlight_enable");
+		if (ret)
+			dev_warn(&pdev->dev,
+				"error %d requesting control gpio\n", ret);
+		else
+			pb->enable_gpio = data->enable_gpio;
+	}
+
 	pb->period = pwm_get_period(pb->pwm);
 	pb->lth_brightness = data->lth_brightness * (pb->period / max);
 
@@ -265,8 +321,7 @@ static int pwm_backlight_remove(struct platform_device *pdev)
 	struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
 
 	backlight_device_unregister(bl);
-	pwm_config(pb->pwm, 0, pb->period);
-	pwm_disable(pb->pwm);
+	pwm_backlight_off(pb);
 	pwm_put(pb->pwm);
 	if (pb->exit)
 		pb->exit(&pdev->dev);
@@ -281,8 +336,7 @@ static int pwm_backlight_suspend(struct device *dev)
 
 	if (pb->notify)
 		pb->notify(pb->dev, 0);
-	pwm_config(pb->pwm, 0, pb->period);
-	pwm_disable(pb->pwm);
+	pwm_backlight_off(pb);
 	if (pb->notify_after)
 		pb->notify_after(pb->dev, 0);
 	return 0;
diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h
index 56f4a86..5ae2cd0 100644
--- a/include/linux/pwm_backlight.h
+++ b/include/linux/pwm_backlight.h
@@ -18,6 +18,11 @@ struct platform_pwm_backlight_data {
 	void (*notify_after)(struct device *dev, int brightness);
 	void (*exit)(struct device *dev);
 	int (*check_fb)(struct device *dev, struct fb_info *info);
+	/* optional GPIO that enables/disables the backlight */
+	int enable_gpio;
+	/* 0 (default initialization value) is a valid GPIO number. Make use of
+	 * control gpio explicit to avoid bad surprises. */
+	bool use_enable_gpio;
 };
 
 #endif
-- 
1.7.11.1


^ permalink raw reply related

* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Stephen Warren @ 2012-06-29 16:04 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Thierry Reding, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1340976167-27298-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

On 06/29/2012 07:22 AM, Alexandre Courbot wrote:
> Add support for an optional power regulator and enable/disable GPIO.
> This scheme is commonly used in embedded systems.

> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c

> -	dev_dbg(&pdev->dev, "got pwm for backlight\n");
> -

That seems like an unrelated change?

> @@ -231,6 +271,22 @@ static int pwm_backlight_probe(struct platform_device *pdev)
>  	if (data->pwm_period_ns > 0)
>  		pwm_set_period(pb->pwm, data->pwm_period_ns);
>  
> +
> +	pb->power_reg = devm_regulator_get(&pdev->dev, "power");

There's an extra blank line there.

> +	if (IS_ERR(pb->power_reg))
> +		return PTR_ERR(pb->power_reg);
> +
> +	pb->enable_gpio = -EINVAL;
> +	if (data->use_enable_gpio) {
> +		ret = devm_gpio_request_one(&pdev->dev, data->enable_gpio,
> +				GPIOF_OUT_INIT_HIGH, "backlight_enable");
> +		if (ret)
> +			dev_warn(&pdev->dev,
> +				"error %d requesting control gpio\n", ret);

Shouldn't that be a hard error? If the user specified that some GPIO be
used, and the GPIO could not be requested, shouldn't the driver fail to
initialize?

> +		else
> +			pb->enable_gpio = data->enable_gpio;

Aside from that, this looks good to me.

^ permalink raw reply

* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Alex Courbot @ 2012-06-30  3:54 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Thierry Reding,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <4FEDD222.7050905-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

On 06/30/2012 01:04 AM, Stephen Warren wrote:
 >> -	dev_dbg(&pdev->dev, "got pwm for backlight\n");
 >> -
 >
 > That seems like an unrelated change?

Dammit, I hoped that would have gone unnoticed. ;)

 >> +	pb->enable_gpio = -EINVAL;
 >> +	if (data->use_enable_gpio) {
 >> +		ret = devm_gpio_request_one(&pdev->dev, data->enable_gpio,
 >> +				GPIOF_OUT_INIT_HIGH, "backlight_enable");
 >> +		if (ret)
 >> +			dev_warn(&pdev->dev,
 >> +				"error %d requesting control gpio\n", ret);
 >
 > Shouldn't that be a hard error? If the user specified that some GPIO be
 > used, and the GPIO could not be requested, shouldn't the driver fail to
 > initialize?

Yes, you are right.

Thanks,
Alex.


^ permalink raw reply

* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Thierry Reding @ 2012-06-30 18:37 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1340976167-27298-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

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

On Fri, Jun 29, 2012 at 10:22:47PM +0900, Alexandre Courbot wrote:
> Add support for an optional power regulator and enable/disable GPIO.
> This scheme is commonly used in embedded systems.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>

I've added some comments in addition to those by Stephen.

[...]
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 057389d..821e03e 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
[...]
> @@ -141,11 +178,14 @@ static int pwm_backlight_parse_dt(struct device *dev,
>  		data->max_brightness--;
>  	}
>  
> -	/*
> -	 * TODO: Most users of this driver use a number of GPIOs to control
> -	 *       backlight power. Support for specifying these needs to be
> -	 *       added.
> -	 */
> +	ret = of_get_named_gpio(node, "enable-gpios", 0);
> +	if (ret >= 0) {
> +		data->enable_gpio = of_get_named_gpio(node, "enable-gpios", 0);

Can't you just reuse the value of ret here?

[...]
> @@ -231,6 +271,22 @@ static int pwm_backlight_probe(struct platform_device *pdev)
>  	if (data->pwm_period_ns > 0)
>  		pwm_set_period(pb->pwm, data->pwm_period_ns);
>  
> +
> +	pb->power_reg = devm_regulator_get(&pdev->dev, "power");
> +	if (IS_ERR(pb->power_reg))
> +		return PTR_ERR(pb->power_reg);
> +
> +	pb->enable_gpio = -EINVAL;

Perhaps initialize this to -1? Assigning standard error codes to a GPIO
doesn't make much sense.

[...]
> diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h
> index 56f4a86..5ae2cd0 100644
> --- a/include/linux/pwm_backlight.h
> +++ b/include/linux/pwm_backlight.h
> @@ -18,6 +18,11 @@ struct platform_pwm_backlight_data {
>  	void (*notify_after)(struct device *dev, int brightness);
>  	void (*exit)(struct device *dev);
>  	int (*check_fb)(struct device *dev, struct fb_info *info);
> +	/* optional GPIO that enables/disables the backlight */
> +	int enable_gpio;
> +	/* 0 (default initialization value) is a valid GPIO number. Make use of
> +	 * control gpio explicit to avoid bad surprises. */
> +	bool use_enable_gpio;

It's a shame we have to add workarounds like this...

Also the canonical form to write multi-line comments would be:

	/*
	 * 0 (default ...
	 * ... surprises.
	 */

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [PATCH] s3fb: Add Virge/MX (86C260)
From: Ondrej Zary @ 2012-07-01 19:23 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Ondrej Zajicek, linux-fbdev, Kernel development list

Add support for Virge/MX (86C260) chip. Although this is a laptop chip,
there's an AGP card with this chip too.

Tested with AGP card, will probably not work correctly with laptops.
DDC does not work on this card (even in DOS or Windows).

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

--- a/drivers/video/s3fb.c
+++ b/drivers/video/s3fb.c
@@ -84,7 +84,7 @@ static const char * const s3_names[] = {"S3 Unknown", "S3 Trio32", "S3 Trio64",
 			"S3 Virge/VX", "S3 Virge/DX", "S3 Virge/GX",
 			"S3 Virge/GX2", "S3 Virge/GX2+", "",
 			"S3 Trio3D/1X", "S3 Trio3D/2X", "S3 Trio3D/2X",
-			"S3 Trio3D"};
+			"S3 Trio3D", "S3 Virge/MX"};
 
 #define CHIP_UNKNOWN		0x00
 #define CHIP_732_TRIO32		0x01
@@ -105,6 +105,7 @@ static const char * const s3_names[] = {"S3 Unknown", "S3 Trio32", "S3 Trio64",
 #define CHIP_362_TRIO3D_2X	0x11
 #define CHIP_368_TRIO3D_2X	0x12
 #define CHIP_365_TRIO3D		0x13
+#define CHIP_260_VIRGE_MX	0x14
 
 #define CHIP_XXX_TRIO		0x80
 #define CHIP_XXX_TRIO64V2_DXGX	0x81
@@ -280,7 +281,8 @@ static int __devinit s3fb_setup_ddc_bus(struct fb_info *info)
 	 */
 /*	vga_wseq(par->state.vgabase, 0x08, 0x06); - not needed, already unlocked */
 	if (par->chip = CHIP_357_VIRGE_GX2 ||
-	    par->chip = CHIP_359_VIRGE_GX2P)
+	    par->chip = CHIP_359_VIRGE_GX2P ||
+	    par->chip = CHIP_260_VIRGE_MX)
 		svga_wseq_mask(par->state.vgabase, 0x0d, 0x01, 0x03);
 	else
 		svga_wseq_mask(par->state.vgabase, 0x0d, 0x00, 0x03);
@@ -487,7 +489,8 @@ static void s3_set_pixclock(struct fb_info *info, u32 pixclock)
 	    par->chip = CHIP_359_VIRGE_GX2P ||
 	    par->chip = CHIP_360_TRIO3D_1X ||
 	    par->chip = CHIP_362_TRIO3D_2X ||
-	    par->chip = CHIP_368_TRIO3D_2X) {
+	    par->chip = CHIP_368_TRIO3D_2X ||
+	    par->chip = CHIP_260_VIRGE_MX) {
 		vga_wseq(par->state.vgabase, 0x12, (n - 2) | ((r & 3) << 6));	/* n and two bits of r */
 		vga_wseq(par->state.vgabase, 0x29, r >> 2); /* remaining highest bit of r */
 	} else
@@ -690,7 +693,8 @@ static int s3fb_set_par(struct fb_info *info)
 	    par->chip != CHIP_359_VIRGE_GX2P &&
 	    par->chip != CHIP_360_TRIO3D_1X &&
 	    par->chip != CHIP_362_TRIO3D_2X &&
-	    par->chip != CHIP_368_TRIO3D_2X) {
+	    par->chip != CHIP_368_TRIO3D_2X &&
+	    par->chip != CHIP_260_VIRGE_MX) {
 		vga_wcrt(par->state.vgabase, 0x54, 0x18); /* M parameter */
 		vga_wcrt(par->state.vgabase, 0x60, 0xff); /* N parameter */
 		vga_wcrt(par->state.vgabase, 0x61, 0xff); /* L parameter */
@@ -739,7 +743,8 @@ static int s3fb_set_par(struct fb_info *info)
 	    par->chip = CHIP_368_TRIO3D_2X ||
 	    par->chip = CHIP_365_TRIO3D    ||
 	    par->chip = CHIP_375_VIRGE_DX  ||
-	    par->chip = CHIP_385_VIRGE_GX) {
+	    par->chip = CHIP_385_VIRGE_GX  ||
+	    par->chip = CHIP_260_VIRGE_MX) {
 		dbytes = info->var.xres * ((bpp+7)/8);
 		vga_wcrt(par->state.vgabase, 0x91, (dbytes + 7) / 8);
 		vga_wcrt(par->state.vgabase, 0x90, (((dbytes + 7) / 8) >> 8) | 0x80);
@@ -751,7 +756,8 @@ static int s3fb_set_par(struct fb_info *info)
 	    par->chip = CHIP_359_VIRGE_GX2P ||
 	    par->chip = CHIP_360_TRIO3D_1X ||
 	    par->chip = CHIP_362_TRIO3D_2X ||
-	    par->chip = CHIP_368_TRIO3D_2X)
+	    par->chip = CHIP_368_TRIO3D_2X ||
+	    par->chip = CHIP_260_VIRGE_MX)
 		vga_wcrt(par->state.vgabase, 0x34, 0x00);
 	else	/* enable Data Transfer Position Control (DTPC) */
 		vga_wcrt(par->state.vgabase, 0x34, 0x10);
@@ -807,7 +813,8 @@ static int s3fb_set_par(struct fb_info *info)
 		    par->chip = CHIP_359_VIRGE_GX2P ||
 		    par->chip = CHIP_360_TRIO3D_1X ||
 		    par->chip = CHIP_362_TRIO3D_2X ||
-		    par->chip = CHIP_368_TRIO3D_2X)
+		    par->chip = CHIP_368_TRIO3D_2X ||
+		    par->chip = CHIP_260_VIRGE_MX)
 			svga_wcrt_mask(par->state.vgabase, 0x67, 0x00, 0xF0);
 		else {
 			svga_wcrt_mask(par->state.vgabase, 0x67, 0x10, 0xF0);
@@ -837,7 +844,8 @@ static int s3fb_set_par(struct fb_info *info)
 			    par->chip != CHIP_359_VIRGE_GX2P &&
 			    par->chip != CHIP_360_TRIO3D_1X &&
 			    par->chip != CHIP_362_TRIO3D_2X &&
-			    par->chip != CHIP_368_TRIO3D_2X)
+			    par->chip != CHIP_368_TRIO3D_2X &&
+			    par->chip != CHIP_260_VIRGE_MX)
 				hmul = 2;
 		}
 		break;
@@ -864,7 +872,8 @@ static int s3fb_set_par(struct fb_info *info)
 			    par->chip != CHIP_359_VIRGE_GX2P &&
 			    par->chip != CHIP_360_TRIO3D_1X &&
 			    par->chip != CHIP_362_TRIO3D_2X &&
-			    par->chip != CHIP_368_TRIO3D_2X)
+			    par->chip != CHIP_368_TRIO3D_2X &&
+			    par->chip != CHIP_260_VIRGE_MX)
 				hmul = 2;
 		}
 		break;
@@ -1208,7 +1217,8 @@ static int __devinit s3_pci_probe(struct pci_dev *dev, const struct pci_device_i
 			break;
 		}
 	} else if (par->chip = CHIP_357_VIRGE_GX2 ||
-		   par->chip = CHIP_359_VIRGE_GX2P) {
+		   par->chip = CHIP_359_VIRGE_GX2P ||
+		   par->chip = CHIP_260_VIRGE_MX) {
 		switch ((regval & 0xC0) >> 6) {
 		case 1: /* 4MB */
 			info->screen_size = 4 << 20;
@@ -1515,6 +1525,7 @@ static struct pci_device_id s3_devices[] __devinitdata = {
 	{PCI_DEVICE(PCI_VENDOR_ID_S3, 0x8A12), .driver_data = CHIP_359_VIRGE_GX2P},
 	{PCI_DEVICE(PCI_VENDOR_ID_S3, 0x8A13), .driver_data = CHIP_36X_TRIO3D_1X_2X},
 	{PCI_DEVICE(PCI_VENDOR_ID_S3, 0x8904), .driver_data = CHIP_365_TRIO3D},
+	{PCI_DEVICE(PCI_VENDOR_ID_S3, 0x8C01), .driver_data = CHIP_260_VIRGE_MX},
 
 	{0, 0, 0, 0, 0, 0, 0}
 };

-- 
Ondrej Zary

^ permalink raw reply

* Re: [PATCH 3/3] mx3fb: avoid screen flash when panning with fb_set_var
From: Liu Ying @ 2012-07-02  2:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1339376810-8247-3-git-send-email-Ying.Liu@freescale.com>

Hi, Guennadi,

Could you please review the second(2/3) patch and the third(3/3) patch
of this series?
I have addressed your comments you gave in the previous review cycle.

Thanks.

2012/6/11, Liu Ying <Ying.Liu@freescale.com>:
> Users may call FBIOPUT_VSCREENINFO ioctrl to do pan display.
> This ioctrl relies on fb_set_var() to do the job. fb_set_var()
> calls custom fb_set_par() method and then calls custom
> fb_pan_display() method. The current implementation of mx3fb
> reinitializes IPU display controller every time the custom
> fb_set_par() method is called, which makes the screen flash
> if fb_set_var() is called to do panning frequently. This patch
> compares the new var info with the cached old one to decide
> whether we really need to reinitialize IPU display controller.
> We ignore xoffset and yoffset update because it doesn't require
> to reinitialize the controller. Users may specify activate field
> of var info with FB_ACTIVATE_NOW and FB_ACTIVATE_FORCE to
> reinialize the controller by force.
>
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
>  drivers/video/mx3fb.c |   22 +++++++++++++++++++++-
>  1 files changed, 21 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index 2dd11c4..7d0aa7b 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -721,6 +721,26 @@ static void mx3fb_dma_done(void *arg)
>  	complete(&mx3_fbi->flip_cmpl);
>  }
>
> +static bool mx3fb_must_set_par(struct fb_info *fbi)
> +{
> +	struct mx3fb_info *mx3_fbi = fbi->par;
> +	struct fb_var_screeninfo old_var = mx3_fbi->cur_var;
> +	struct fb_var_screeninfo new_var = fbi->var;
> +
> +	if ((fbi->var.activate & FB_ACTIVATE_FORCE) &&
> +	    (fbi->var.activate & FB_ACTIVATE_MASK) = FB_ACTIVATE_NOW)
> +		return true;
> +
> +	/*
> +	 * Ignore xoffset and yoffset update,
> +	 * because pan display handles this case.
> +	 */
> +	old_var.xoffset = new_var.xoffset;
> +	old_var.yoffset = new_var.yoffset;
> +
> +	return !!memcmp(&old_var, &new_var, sizeof(struct fb_var_screeninfo));
> +}
> +
>  static int __set_par(struct fb_info *fbi, bool lock)
>  {
>  	u32 mem_len, cur_xoffset, cur_yoffset;
> @@ -844,7 +864,7 @@ static int mx3fb_set_par(struct fb_info *fbi)
>
>  	mutex_lock(&mx3_fbi->mutex);
>
> -	ret = __set_par(fbi, true);
> +	ret = mx3fb_must_set_par(fbi) ? __set_par(fbi, true) : 0;
>
>  	mutex_unlock(&mx3_fbi->mutex);
>
> --
> 1.7.1
>
>
>


-- 
Best Regards,
Liu Ying

^ permalink raw reply

* Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Alexandre Courbot @ 2012-07-02  3:35 UTC (permalink / raw)
  To: Thierry Reding, Stephen Warren
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20120630183742.GE23990-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org>

On 07/01/2012 03:37 AM, Thierry Reding wrote:>> +	ret = 
of_get_named_gpio(node, "enable-gpios", 0);
 >> +	if (ret >= 0) {
 >> +		data->enable_gpio = of_get_named_gpio(node, "enable-gpios", 0);
 >
 > Can't you just reuse the value of ret here?

Yes, definitely.

 >> +	pb->enable_gpio = -EINVAL;
 >
 > Perhaps initialize this to -1? Assigning standard error codes to a GPIO
 > doesn't make much sense.

Documentation/gpio.txt states the following:

"If you want to initialize a structure with an invalid GPIO number, use
some negative number (perhaps "-EINVAL"); that will never be valid."

gpio_is_valid() seems to be happy with any negative value, but -EINVAL 
seems to be a convention here.

 >> +	/* optional GPIO that enables/disables the backlight */
 >> +	int enable_gpio;
 >> +	/* 0 (default initialization value) is a valid GPIO number. Make 
use of
 >> +	 * control gpio explicit to avoid bad surprises. */
 >> +	bool use_enable_gpio;
 >
 > It's a shame we have to add workarounds like this...

Yeah, I hate that too. :/ I see nothing better to do unfortunately.

Other remarks from Stephen made me realize that this patch has two major 
flaws:

1) The GPIO and regulator are fixed, optional entites ; this should 
cover most cases but is not very flexible.
2) Some (most?) backlight specify timings between turning power 
on/enabling PWM/enabling backlight. Even the order of things may be 
different. This patch totally ignores that.

So instead of having fixed "power-supply" and "enable-gpio" properties, 
how about having properties describing the power-on and power-off 
sequences which odd cells alternate between phandles to 
regulators/gpios/pwm and delays in microseconds before continuing the 
sequence. For example:

power-on = <&pwm 2 5000000
	    10000
	    &backlight_reg
	    0
	    &gpio 28 0>;
power-off = <&gpio 28 0
	     0
	     &backlight_reg
	     10000
	     &pwm 2 0>;

Here the power-on sequence would translate to, power on the second PWM 
with a duty-cycle of 5ms, wait 10ms, then enable the backlight regulator 
and GPIO 28 without delay. Power-off is the exact opposite. The nice 
thing with this scheme is that you can reorder the sequence at will and 
support the weirdest setups.

What I don't know (device tree newbie here!) is:
1) Is it legal to refer the same phandle several times in the same node?
2) Is it ok to mix phandles of different types with integer values? The 
DT above compiled, but can you e.g. resolve a regulator phandle in the 
middle of a property?
3) Can you "guess" the type of a phandle before parsing it? Here the 
first phandle is a GPIO, but it could as well be the regulator. Do we 
have means to know that in the driver code?

Sorry for the long explanation, but I really wonder if doing this is 
possible at all. If it is, then I think that's the way to do for 
backlight initialization.

Alex.


^ permalink raw reply

* Re: [PATCH 2/3] mx3fb: support pan display with fb_set_var
From: Florian Tobias Schandinat @ 2012-07-02  6:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1339376810-8247-2-git-send-email-Ying.Liu@freescale.com>

On 06/11/2012 01:06 AM, Liu Ying wrote:
> Users may call FBIOPUT_VSCREENINFO ioctrl to do pan display.
> This ioctrl relies on fb_set_var() to do the job. fb_set_var()
> calls the custom fb_set_par() method and then calls the custom
> fb_pan_display() method. Before calling the custom fb_pan_display()
> method, info->var is already updated from the new *var in fb_set_var().
> And, the custom fb_pan_display() method checks if xoffset and yoffset
> in info->var and the new *var are different before doing actual panning,
> which prevents the panning from happening within fb_set_var() context.
> This patch caches the current var info locally in mx3fb driver so that
> pan display with fb_set_var is supported.
> 
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>

Applied patches 2 and 3 of this series.


Thanks,

Florian Tobias Schandinat

> ---
>  drivers/video/mx3fb.c |   33 ++++++++++++++++++++++++++-------
>  1 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index d53db60..2dd11c4 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -268,7 +268,7 @@ struct mx3fb_info {
>  	dma_cookie_t			cookie;
>  	struct scatterlist		sg[2];
>  
> -	u32				sync;	/* preserve var->sync flags */
> +	struct fb_var_screeninfo	cur_var; /* current var info */
>  };
>  
>  static void mx3fb_dma_done(void *);
> @@ -723,7 +723,7 @@ static void mx3fb_dma_done(void *arg)
>  
>  static int __set_par(struct fb_info *fbi, bool lock)
>  {
> -	u32 mem_len;
> +	u32 mem_len, cur_xoffset, cur_yoffset;
>  	struct ipu_di_signal_cfg sig_cfg;
>  	enum ipu_panel mode = IPU_PANEL_TFT;
>  	struct mx3fb_info *mx3_fbi = fbi->par;
> @@ -805,8 +805,25 @@ static int __set_par(struct fb_info *fbi, bool lock)
>  	video->out_height	= fbi->var.yres;
>  	video->out_stride	= fbi->var.xres_virtual;
>  
> -	if (mx3_fbi->blank = FB_BLANK_UNBLANK)
> +	if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
>  		sdc_enable_channel(mx3_fbi);
> +		/*
> +		 * sg[0] points to fb smem_start address
> +		 * and is actually active in controller.
> +		 */
> +		mx3_fbi->cur_var.xoffset = 0;
> +		mx3_fbi->cur_var.yoffset = 0;
> +	}
> +
> +	/*
> +	 * Preserve xoffset and yoffest in case they are
> +	 * inactive in controller as fb is blanked.
> +	 */
> +	cur_xoffset = mx3_fbi->cur_var.xoffset;
> +	cur_yoffset = mx3_fbi->cur_var.yoffset;
> +	mx3_fbi->cur_var = fbi->var;
> +	mx3_fbi->cur_var.xoffset = cur_xoffset;
> +	mx3_fbi->cur_var.yoffset = cur_yoffset;
>  
>  	return 0;
>  }
> @@ -926,8 +943,8 @@ static int mx3fb_check_var(struct fb_var_screeninfo *var, struct fb_info *fbi)
>  	var->grayscale = 0;
>  
>  	/* Preserve sync flags */
> -	var->sync |= mx3_fbi->sync;
> -	mx3_fbi->sync |= var->sync;
> +	var->sync |= mx3_fbi->cur_var.sync;
> +	mx3_fbi->cur_var.sync |= var->sync;
>  
>  	return 0;
>  }
> @@ -1073,8 +1090,8 @@ static int mx3fb_pan_display(struct fb_var_screeninfo *var,
>  		return -EINVAL;
>  	}
>  
> -	if (fbi->var.xoffset = var->xoffset &&
> -	    fbi->var.yoffset = var->yoffset)
> +	if (mx3_fbi->cur_var.xoffset = var->xoffset &&
> +	    mx3_fbi->cur_var.yoffset = var->yoffset)
>  		return 0;	/* No change, do nothing */
>  
>  	y_bottom = var->yoffset;
> @@ -1157,6 +1174,8 @@ static int mx3fb_pan_display(struct fb_var_screeninfo *var,
>  	else
>  		fbi->var.vmode &= ~FB_VMODE_YWRAP;
>  
> +	mx3_fbi->cur_var = fbi->var;
> +
>  	mutex_unlock(&mx3_fbi->mutex);
>  
>  	dev_dbg(fbi->device, "Update complete\n");


^ 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