Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] OMAPDSS: TPO-TD03MTEA1: Check for errors from regulator_enable()
From: Tomi Valkeinen @ 2012-03-21 12:50 UTC (permalink / raw)
  To: Mark Brown
  Cc: Florian Tobias Schandinat, Grazvydas Ignotas, linux-omap,
	linux-fbdev
In-Reply-To: <1332169352-5076-1-git-send-email-broonie@opensource.wolfsonmicro.com>

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

On Mon, 2012-03-19 at 15:02 +0000, Mark Brown wrote:
> It is possible for regulator_enable() to fail and if it does fail that's
> generally a bad sign for anything we try to do with the hardware afterwards
> so check for and immediately return an error if regulator_enable() fails.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  .../video/omap2/displays/panel-tpo-td043mtea1.c    |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/omap2/displays/panel-tpo-td043mtea1.c b/drivers/video/omap2/displays/panel-tpo-td043mtea1.c
> index 32f3fcd..74c6b87 100644
> --- a/drivers/video/omap2/displays/panel-tpo-td043mtea1.c
> +++ b/drivers/video/omap2/displays/panel-tpo-td043mtea1.c
> @@ -272,11 +272,14 @@ static const struct omap_video_timings tpo_td043_timings = {
>  static int tpo_td043_power_on(struct tpo_td043_device *tpo_td043)
>  {
>  	int nreset_gpio = tpo_td043->nreset_gpio;
> +	int r;
>  
>  	if (tpo_td043->powered_on)
>  		return 0;
>  
> -	regulator_enable(tpo_td043->vcc_reg);
> +	r = regulator_enable(tpo_td043->vcc_reg);
> +	if (r != 0)
> +		return r;
>  
>  	/* wait for regulator to stabilize */
>  	msleep(160);

Thanks, I'll apply this series to omapdss tree.

 Tomi


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

^ permalink raw reply

* Re: [PATCH] OMAPDSS: VENC: Check for errors from regulator_enable()
From: Tomi Valkeinen @ 2012-03-21 12:49 UTC (permalink / raw)
  To: Mark Brown; +Cc: Florian Tobias Schandinat, linux-omap, linux-fbdev
In-Reply-To: <1332168999-4600-1-git-send-email-broonie@opensource.wolfsonmicro.com>

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

On Mon, 2012-03-19 at 14:56 +0000, Mark Brown wrote:
> It is possible for regulator_enable() to fail and if it does fail that's
> generally a bad sign for anything we try to do with the hardware afterwards
> so check for and immediately return an error if regulator_enable() fails.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/video/omap2/dss/venc.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
> index 9c3daf7..abfbd4a 100644
> --- a/drivers/video/omap2/dss/venc.c
> +++ b/drivers/video/omap2/dss/venc.c
> @@ -443,7 +443,9 @@ static int venc_power_on(struct omap_dss_device *dssdev)
>  	dispc_set_digit_size(dssdev->panel.timings.x_res,
>  			dssdev->panel.timings.y_res/2);
>  
> -	regulator_enable(venc.vdda_dac_reg);
> +	r = regulator_enable(venc.vdda_dac_reg);
> +	if (r)
> +		goto err;
>  
>  	if (dssdev->platform_enable)
>  		dssdev->platform_enable(dssdev);

Thanks, I'll apply to omapdss tree.

 Tomi


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

^ permalink raw reply

* Re: [PATCH] OMAP: DSS2: Remove suspicous and unused TAAL regulator API usage
From: Tomi Valkeinen @ 2012-03-21 12:49 UTC (permalink / raw)
  To: Mark Brown; +Cc: Florian Tobias Schandinat, linux-omap, linux-fbdev
In-Reply-To: <1332168646-4391-1-git-send-email-broonie@opensource.wolfsonmicro.com>

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

On Mon, 2012-03-19 at 14:50 +0000, Mark Brown wrote:
> The TAAL driver contains some regulator support which is currently unused
> (the code is there but the one panel supported by the driver doesn't have
> any regulators provided). This code mostly looks like an open coded
> version of the regulator core bulk API.
> 
> The only additional feature is that a voltage range can be set once when
> the device is opened, though this is never varied at runtime. The general
> expectation is that if the device is not actively managing the voltage of
> the device (eg, doing DVFS) then any configuration will be done using the
> constraints rather than by drivers, saving them code and ensuring that
> they work well with systems where the voltage is not configurable.
> 
> If systems are added needing regulator support this can be added back in,
> though it should be based on core features rather than open coding things.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Looks fine to me, I'll apply to omapdss tree.

We had multiple panels in Nokia, almost the same but with slightly
different regulator needs, so we wrote the code removed in this patch.
But as it seems there are no more linux devices coming from Nokia, let's
just remove the regulator code.

 Tomi


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

^ permalink raw reply

* Re: [GIT PULL] OMAP DSS for v3.4
From: Tomi Valkeinen @ 2012-03-21 12:39 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: Arnd Bergmann, linux-omap, linux-fbdev
In-Reply-To: <1332154865.2144.57.camel@deskari>

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

Hi Florian,

On Mon, 2012-03-19 at 13:01 +0200, Tomi Valkeinen wrote:
> Hi Florian, Arnd,
> 
> Here are the changes for OMAP DSS driver for v3.4.
> 
> There's an issue with the dss driver that appears on arm-soc/for-next
> branch, which I'm still solving
> (http://marc.info/?l=linux-omap&m=133214966902577&w=2). I hope to get
> fix for that ready and merged for -rc1, but I'm not sure if I can make
> it in time, so I wanted to sent this pull request already.

Below is a patch that fixes the issue I mentioned above. So briefly, the
problem is that omapdss's driver registration is broken, and it has been
working so far by luck. arm-soc tree contains a patch which changes how
arch/arm/plat-omap/omap_device.c creates the devices, and this causes
the (broken) omapdss driver registration to break, causing a deadlock.

The patch below makes the omapdss driver registration saner. The patch
works with and without the omap_device.c patch from arm-soc.

Florian, if you didn't send the pull request yet, this would be a good
patch to add to it. We can, of course, get it in for -rc2, but avoiding
a kernel deadlock on -rc1 would be nice.

 Tomi


From 849c07b1fd3d9f23e8ed94436b6221f8652462c0 Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
Date: Mon, 19 Mar 2012 15:05:02 +0200
Subject: [PATCH] OMAPDSS: register dss drivers in module init

We do the dss driver registration in a rather strange way: we have the
higher level omapdss driver, and we use that driver's probe function to
register the drivers for the rest of the dss devices.

There doesn't seem to be any reason for that, and additionally the
soon-to-be-merged patch "ARM: OMAP: omap_device: remove
omap_device_parent" will break omapdss initialization with the current
registration model.

This patch changes the registration for all drivers to happen at the
same place, in the init of the module.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/core.c |  135 +++++++++++++++++++++++-----------------
 1 files changed, 77 insertions(+), 58 deletions(-)

diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index 8613f86..e8a1207 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -183,42 +183,6 @@ static int omap_dss_probe(struct platform_device *pdev)
 	dss_init_overlay_managers(pdev);
 	dss_init_overlays(pdev);
 
-	r = dss_init_platform_driver();
-	if (r) {
-		DSSERR("Failed to initialize DSS platform driver\n");
-		goto err_dss;
-	}
-
-	r = dispc_init_platform_driver();
-	if (r) {
-		DSSERR("Failed to initialize dispc platform driver\n");
-		goto err_dispc;
-	}
-
-	r = rfbi_init_platform_driver();
-	if (r) {
-		DSSERR("Failed to initialize rfbi platform driver\n");
-		goto err_rfbi;
-	}
-
-	r = venc_init_platform_driver();
-	if (r) {
-		DSSERR("Failed to initialize venc platform driver\n");
-		goto err_venc;
-	}
-
-	r = dsi_init_platform_driver();
-	if (r) {
-		DSSERR("Failed to initialize DSI platform driver\n");
-		goto err_dsi;
-	}
-
-	r = hdmi_init_platform_driver();
-	if (r) {
-		DSSERR("Failed to initialize hdmi\n");
-		goto err_hdmi;
-	}
-
 	r = dss_initialize_debugfs();
 	if (r)
 		goto err_debugfs;
@@ -246,18 +210,6 @@ static int omap_dss_probe(struct platform_device *pdev)
 err_register:
 	dss_uninitialize_debugfs();
 err_debugfs:
-	hdmi_uninit_platform_driver();
-err_hdmi:
-	dsi_uninit_platform_driver();
-err_dsi:
-	venc_uninit_platform_driver();
-err_venc:
-	dispc_uninit_platform_driver();
-err_dispc:
-	rfbi_uninit_platform_driver();
-err_rfbi:
-	dss_uninit_platform_driver();
-err_dss:
 
 	return r;
 }
@@ -269,13 +221,6 @@ static int omap_dss_remove(struct platform_device *pdev)
 
 	dss_uninitialize_debugfs();
 
-	hdmi_uninit_platform_driver();
-	dsi_uninit_platform_driver();
-	venc_uninit_platform_driver();
-	rfbi_uninit_platform_driver();
-	dispc_uninit_platform_driver();
-	dss_uninit_platform_driver();
-
 	dss_uninit_overlays(pdev);
 	dss_uninit_overlay_managers(pdev);
 
@@ -525,6 +470,80 @@ static int omap_dss_bus_register(void)
 
 /* INIT */
 
+static int __init omap_dss_register_drivers(void)
+{
+	int r;
+
+	r = platform_driver_register(&omap_dss_driver);
+	if (r)
+		return r;
+
+	r = dss_init_platform_driver();
+	if (r) {
+		DSSERR("Failed to initialize DSS platform driver\n");
+		goto err_dss;
+	}
+
+	r = dispc_init_platform_driver();
+	if (r) {
+		DSSERR("Failed to initialize dispc platform driver\n");
+		goto err_dispc;
+	}
+
+	r = rfbi_init_platform_driver();
+	if (r) {
+		DSSERR("Failed to initialize rfbi platform driver\n");
+		goto err_rfbi;
+	}
+
+	r = venc_init_platform_driver();
+	if (r) {
+		DSSERR("Failed to initialize venc platform driver\n");
+		goto err_venc;
+	}
+
+	r = dsi_init_platform_driver();
+	if (r) {
+		DSSERR("Failed to initialize DSI platform driver\n");
+		goto err_dsi;
+	}
+
+	r = hdmi_init_platform_driver();
+	if (r) {
+		DSSERR("Failed to initialize hdmi\n");
+		goto err_hdmi;
+	}
+
+	return 0;
+
+err_hdmi:
+	dsi_uninit_platform_driver();
+err_dsi:
+	venc_uninit_platform_driver();
+err_venc:
+	rfbi_uninit_platform_driver();
+err_rfbi:
+	dispc_uninit_platform_driver();
+err_dispc:
+	dss_uninit_platform_driver();
+err_dss:
+	platform_driver_unregister(&omap_dss_driver);
+
+	return r;
+}
+
+static void __exit omap_dss_unregister_drivers(void)
+{
+	hdmi_uninit_platform_driver();
+	dsi_uninit_platform_driver();
+	venc_uninit_platform_driver();
+	rfbi_uninit_platform_driver();
+	dispc_uninit_platform_driver();
+	dss_uninit_platform_driver();
+
+	platform_driver_unregister(&omap_dss_driver);
+}
+
 #ifdef CONFIG_OMAP2_DSS_MODULE
 static void omap_dss_bus_unregister(void)
 {
@@ -541,7 +560,7 @@ static int __init omap_dss_init(void)
 	if (r)
 		return r;
 
-	r = platform_driver_register(&omap_dss_driver);
+	r = omap_dss_register_drivers();
 	if (r) {
 		omap_dss_bus_unregister();
 		return r;
@@ -562,7 +581,7 @@ static void __exit omap_dss_exit(void)
 		core.vdds_sdi_reg = NULL;
 	}
 
-	platform_driver_unregister(&omap_dss_driver);
+	omap_dss_unregister_drivers();
 
 	omap_dss_bus_unregister();
 }
@@ -577,7 +596,7 @@ static int __init omap_dss_init(void)
 
 static int __init omap_dss_init2(void)
 {
-	return platform_driver_register(&omap_dss_driver);
+	return omap_dss_register_drivers();
 }
 
 core_initcall(omap_dss_init);
-- 
1.7.4.1



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

^ permalink raw reply related

* [PATCH V2 2/3] OMAPDSS: DISPC: Handle synclost errors in OMAP3
From: Chandrabhanu Mahapatra @ 2012-03-21  9:56 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra

In OMAP3 DISPC video overlays suffer from some undocumented horizontal position
and timing related limitations leading to SYNCLOST errors. Whenever the image
window is moved towards the right of the screen SYNCLOST errors become
frequent. Checks have been implemented to see that DISPC driver rejects
configuration exceeding above limitations.

This code was successfully tested on OMAP3. This code is written based on code
written by Ville Syrj채l채 <ville.syrjala@nokia.com> in Linux OMAP kernel. Ville
Syrj채l채 <ville.syrjala@nokia.com> had added checks for video overlay horizontal
timing and DISPC horizontal blanking length limitations.

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

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 5a1963e..d8a1672 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1622,6 +1622,57 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 	}
 }
 
+static int check_horiz_timing(enum omap_channel channel, u16 pos_x,
+		u16 width, u16 height, u16 out_width, u16 out_height)
+{
+	int DS = DIV_ROUND_UP(height, out_height);
+	struct omap_dss_device *dssdev = dispc_mgr_get_device(channel);
+	struct omap_video_timings t = dssdev->panel.timings;
+	unsigned long nonactive, lclk, pclk;
+	static const u8 limits[3] = { 8, 10, 20 };
+	u64 val, blank;
+	int i;
+
+	nonactive = t.x_res + t.hfp + t.hsw + t.hbp - out_width;
+	pclk = dispc_mgr_pclk_rate(channel);
+	lclk = dispc_mgr_lclk_rate(channel);
+
+	i = 0;
+	if (out_height < height)
+		i++;
+	if (out_width < width)
+		i++;
+	blank = div_u64((u64)(t.hbp + t.hsw + t.hfp) * lclk, pclk);
+	DSSDBG("blanking period + ppl = %llu (limit = %u)\n", blank, limits[i]);
+	if (blank <= limits[i])
+		return -EINVAL;
+
+	/*
+	 * Pixel data should be prepared before visible display point starts.
+	 * So, atleast DS-2 lines must have already been fetched by DISPC
+	 * during nonactive - pos_x period.
+	 */
+	val = div_u64((u64)(nonactive - pos_x) * lclk, pclk);
+	DSSDBG("(nonactive - pos_x) * pcd = %llu,"
+		" max(0, DS - 2) * width = %d\n",
+		val, max(0, DS - 2) * width);
+	if (val < max(0, DS - 2) * width)
+		return -EINVAL;
+
+	/*
+	 * All lines need to be refilled during the nonactive period of which
+	 * only one line can be loaded during the active period. So, atleast
+	 * DS - 1 lines should be loaded during nonactive period.
+	 */
+	val =  div_u64((u64)nonactive * lclk, pclk);
+	DSSDBG("nonactive * pcd  = %llu, max(0, DS - 1) * width = %d\n",
+		val, max(0, DS - 1) * width);
+	if (val < max(0, DS - 1) * width)
+		return -EINVAL;
+
+	return 0;
+}
+
 static unsigned long calc_fclk_five_taps(enum omap_channel channel, u16 width,
 		u16 height, u16 out_width, u16 out_height,
 		enum omap_color_mode color_mode)
@@ -1702,7 +1753,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		enum omap_channel channel, u16 width, u16 height,
 		u16 out_width, u16 out_height,
 		enum omap_color_mode color_mode, bool *five_taps,
-		int *x_predecim, int *y_predecim)
+		int *x_predecim, int *y_predecim, u16 pos_x)
 {
 	struct omap_overlay *ovl = omap_dss_get_overlay(plane);
 	const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
@@ -1778,6 +1829,9 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 			fclk = calc_fclk_five_taps(channel, in_width, in_height,
 				out_width, out_height, color_mode);
 
+			error = check_horiz_timing(channel, pos_x, in_width,
+				in_height, out_width, out_height);
+
 			if (in_width > maxsinglelinewidth)
 				if (in_height > out_height &&
 					in_height < out_height * 2)
@@ -1785,7 +1839,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 			if (!*five_taps)
 				fclk = calc_fclk(channel, in_width, in_height,
 					out_width, out_height);
-			error = (in_width > maxsinglelinewidth * 2 ||
+			error = (error || in_width > maxsinglelinewidth * 2 ||
 				(in_width > maxsinglelinewidth && *five_taps) ||
 				!fclk || fclk > dispc_fclk_rate());
 			if (error) {
@@ -1801,6 +1855,12 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		} while (decim_x <= *x_predecim && decim_y <= *y_predecim
 			&& error);
 
+		if (check_horiz_timing(channel, pos_x, width, height,
+			out_width, out_height)){
+				DSSERR("horizontal timing too tight\n");
+				return -EINVAL;
+		}
+
 		if (in_width > (maxsinglelinewidth * 2)) {
 			DSSERR("Cannot setup scaling");
 			DSSERR("width exceeds maximum width possible");
@@ -1901,7 +1961,7 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 
 	r = dispc_ovl_calc_scaling(plane, channel, in_width, in_height,
 			out_width, out_height, oi->color_mode, &five_taps,
-			&x_predecim, &y_predecim);
+			&x_predecim, &y_predecim, oi->pos_x);
 	if (r)
 		return r;
 
@@ -2472,32 +2532,19 @@ unsigned long dispc_fclk_rate(void)
 
 unsigned long dispc_mgr_lclk_rate(enum omap_channel channel)
 {
-	struct platform_device *dsidev;
-	int lcd;
-	unsigned long r;
-	u32 l;
+	unsigned long r = dispc_fclk_rate();
 
-	l = dispc_read_reg(DISPC_DIVISORo(channel));
+	if (dispc_mgr_is_lcd(channel)) {
+		u32 l;
+		int lcd;
 
-	lcd = FLD_GET(l, 23, 16);
+		l = dispc_read_reg(DISPC_DIVISORo(channel));
+		lcd = FLD_GET(l, 23, 16);
 
-	switch (dss_get_lcd_clk_source(channel)) {
-	case OMAP_DSS_CLK_SRC_FCK:
-		r = clk_get_rate(dispc.dss_clk);
-		break;
-	case OMAP_DSS_CLK_SRC_DSI_PLL_HSDIV_DISPC:
-		dsidev = dsi_get_dsidev_from_id(0);
-		r = dsi_get_pll_hsdiv_dispc_rate(dsidev);
-		break;
-	case OMAP_DSS_CLK_SRC_DSI2_PLL_HSDIV_DISPC:
-		dsidev = dsi_get_dsidev_from_id(1);
-		r = dsi_get_pll_hsdiv_dispc_rate(dsidev);
-		break;
-	default:
-		BUG();
+		return r / lcd;
+	} else {
+		return r;
 	}
-
-	return r / lcd;
 }
 
 unsigned long dispc_mgr_pclk_rate(enum omap_channel channel)
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH V2 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage
From: Chandrabhanu Mahapatra @ 2012-03-21  9:53 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra

DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
divisor LCD.

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

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index d8a1672..1fdac73 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1761,6 +1761,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
 	const int max_decim_limit = 16;
 	unsigned long fclk = 0;
+	unsigned long dispc_core_clk = dispc_mgr_lclk_rate(channel);
 	int decim_x, decim_y, error, min_factor;
 	u16 in_width, in_height, in_width_max = 0;
 
@@ -1803,7 +1804,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 			fclk = calc_fclk(channel, in_width, in_height,
 					out_width, out_height);
 			error = (in_width > maxsinglelinewidth || !fclk ||
-				fclk > dispc_fclk_rate());
+				fclk > dispc_core_clk);
 			if (error) {
 				if (decim_x = decim_y) {
 					decim_x = min_factor;
@@ -1841,7 +1842,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 					out_width, out_height);
 			error = (error || in_width > maxsinglelinewidth * 2 ||
 				(in_width > maxsinglelinewidth && *five_taps) ||
-				!fclk || fclk > dispc_fclk_rate());
+				!fclk || fclk > dispc_core_clk);
 			if (error) {
 				if (decim_x = decim_y) {
 					decim_x = min_factor;
@@ -1873,7 +1874,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		}
 	} else {
 		in_height = DIV_ROUND_UP(height, decim_y);
-		in_width_max = dispc_fclk_rate() /
+		in_width_max = dispc_core_clk /
 				DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
 						out_width);
 		decim_x = DIV_ROUND_UP(width, in_width_max);
@@ -1894,13 +1895,13 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	}
 
 	DSSDBG("required fclk rate = %lu Hz\n", fclk);
-	DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
+	DSSDBG("current fclk rate = %lu Hz\n", dispc_core_clk);
 
-	if (!fclk || fclk > dispc_fclk_rate()) {
+	if (!fclk || fclk > dispc_core_clk) {
 		DSSERR("failed to set up scaling, "
 			"required fclk rate = %lu Hz, "
 			"current fclk rate = %lu Hz\n",
-			fclk, dispc_fclk_rate());
+			fclk, dispc_core_clk);
 		return -EINVAL;
 	}
 
-- 
1.7.1


^ permalink raw reply related

* [PATCH V2 1/3] OMAPDSS: DISPC: Enable predecimation
From: Chandrabhanu Mahapatra @ 2012-03-21  9:52 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra

In OMAP3 and OMAP4, the DISPC Scaler can downscale an image up to 4 times, and
up to 2 times in OMAP2. However, with predecimation, the image can be reduced
to 16 times by fetching only the necessary pixels in memory. Then this
predecimated image can be downscaled further by the DISPC scaler.

The pipeline is configured to use a burst of size 8 * 128 bits which consists
of 8 mini bursts of 16 bytes each. So, horizontal predecimation more than 16
can lead to complete discarding of such mini bursts. L3 interconnect may
handover the bus to some other initiator and inturn delay the fetching of
pixels leading to underflows. So, maximum predecimation limit is fixed at 16.

Based on the downscaling required, a prior calculation of predecimation values
for width and height of an image is done. Since, Predecimation reduces quality
of an image higher priorty is given to DISPC Scaler for downscaling.

This code was successfully tested on OMAP2, OMAP3 and OMAP4. Horizontal and
vertical predecimation worked fine except for some synclost errors due to
undocumented errata in OMAP3 which are fixed later and skewed images were seen
on OMAP2 and OMAP3 during horizontal predecimation which will be addressed in
the future patches.

This code is based on code written by Lajos Molnar <lajos@ti.com> who had added
predecimation support for NV12/YUV/rotated/SDMA buffers.

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

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index e1626a1..5a1963e 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1389,7 +1389,7 @@ static void calc_vrfb_rotation_offset(u8 rotation, bool mirror,
 		enum omap_color_mode color_mode, bool fieldmode,
 		unsigned int field_offset,
 		unsigned *offset0, unsigned *offset1,
-		s32 *row_inc, s32 *pix_inc)
+		s32 *row_inc, s32 *pix_inc, int x_predecim, int y_predecim)
 {
 	u8 ps;
 
@@ -1435,10 +1435,10 @@ static void calc_vrfb_rotation_offset(u8 rotation, bool mirror,
 		else
 			*offset0 = 0;
 
-		*row_inc = pixinc(1 + (screen_width - width) +
-				(fieldmode ? screen_width : 0),
-				ps);
-		*pix_inc = pixinc(1, ps);
+		*row_inc = pixinc(1 +
+			(y_predecim * screen_width - x_predecim * width) +
+			(fieldmode ? screen_width : 0), ps);
+		*pix_inc = pixinc(x_predecim, ps);
 		break;
 
 	case OMAP_DSS_ROT_0 + 4:
@@ -1456,10 +1456,10 @@ static void calc_vrfb_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = field_offset * screen_width * ps;
 		else
 			*offset0 = 0;
-		*row_inc = pixinc(1 - (screen_width + width) -
-				(fieldmode ? screen_width : 0),
-				ps);
-		*pix_inc = pixinc(1, ps);
+		*row_inc = pixinc(1 -
+			(y_predecim * screen_width + x_predecim * width) -
+			(fieldmode ? screen_width : 0), ps);
+		*pix_inc = pixinc(x_predecim, ps);
 		break;
 
 	default:
@@ -1473,7 +1473,7 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 		enum omap_color_mode color_mode, bool fieldmode,
 		unsigned int field_offset,
 		unsigned *offset0, unsigned *offset1,
-		s32 *row_inc, s32 *pix_inc)
+		s32 *row_inc, s32 *pix_inc, int x_predecim, int y_predecim)
 {
 	u8 ps;
 	u16 fbw, fbh;
@@ -1515,10 +1515,14 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 + field_offset * screen_width * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(1 + (screen_width - fbw) +
-				(fieldmode ? screen_width : 0),
-				ps);
-		*pix_inc = pixinc(1, ps);
+		*row_inc = pixinc(1 +
+			(y_predecim * screen_width - fbw * x_predecim) +
+			(fieldmode ? screen_width : 0),	ps);
+		if (color_mode = OMAP_DSS_COLOR_YUV2 ||
+			color_mode = OMAP_DSS_COLOR_UYVY)
+			*pix_inc = pixinc(x_predecim, 2 * ps);
+		else
+			*pix_inc = pixinc(x_predecim, ps);
 		break;
 	case OMAP_DSS_ROT_90:
 		*offset1 = screen_width * (fbh - 1) * ps;
@@ -1526,9 +1530,9 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 + field_offset * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(screen_width * (fbh - 1) + 1 +
-				(fieldmode ? 1 : 0), ps);
-		*pix_inc = pixinc(-screen_width, ps);
+		*row_inc = pixinc(screen_width * (fbh * x_predecim - 1) +
+				y_predecim + (fieldmode ? 1 : 0), ps);
+		*pix_inc = pixinc(-x_predecim * screen_width, ps);
 		break;
 	case OMAP_DSS_ROT_180:
 		*offset1 = (screen_width * (fbh - 1) + fbw - 1) * ps;
@@ -1537,10 +1541,13 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 		else
 			*offset0 = *offset1;
 		*row_inc = pixinc(-1 -
-				(screen_width - fbw) -
-				(fieldmode ? screen_width : 0),
-				ps);
-		*pix_inc = pixinc(-1, ps);
+			(y_predecim * screen_width - fbw * x_predecim) -
+			(fieldmode ? screen_width : 0),	ps);
+		if (color_mode = OMAP_DSS_COLOR_YUV2 ||
+			color_mode = OMAP_DSS_COLOR_UYVY)
+			*pix_inc = pixinc(-x_predecim, 2 * ps);
+		else
+			*pix_inc = pixinc(-x_predecim, ps);
 		break;
 	case OMAP_DSS_ROT_270:
 		*offset1 = (fbw - 1) * ps;
@@ -1548,9 +1555,9 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 - field_offset * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(-screen_width * (fbh - 1) - 1 -
-				(fieldmode ? 1 : 0), ps);
-		*pix_inc = pixinc(screen_width, ps);
+		*row_inc = pixinc(-screen_width * (fbh * x_predecim - 1) -
+				y_predecim - (fieldmode ? 1 : 0), ps);
+		*pix_inc = pixinc(x_predecim * screen_width, ps);
 		break;
 
 	/* mirroring */
@@ -1560,10 +1567,14 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 + field_offset * screen_width * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(screen_width * 2 - 1 +
+		*row_inc = pixinc(y_predecim * screen_width * 2 - 1 +
 				(fieldmode ? screen_width : 0),
 				ps);
-		*pix_inc = pixinc(-1, ps);
+		if (color_mode = OMAP_DSS_COLOR_YUV2 ||
+			color_mode = OMAP_DSS_COLOR_UYVY)
+			*pix_inc = pixinc(-x_predecim, 2 * ps);
+		else
+			*pix_inc = pixinc(-x_predecim, ps);
 		break;
 
 	case OMAP_DSS_ROT_90 + 4:
@@ -1572,10 +1583,10 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 + field_offset * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(-screen_width * (fbh - 1) + 1 +
-				(fieldmode ? 1 : 0),
+		*row_inc = pixinc(-screen_width * (fbh * x_predecim - 1) +
+				y_predecim + (fieldmode ? 1 : 0),
 				ps);
-		*pix_inc = pixinc(screen_width, ps);
+		*pix_inc = pixinc(x_predecim * screen_width, ps);
 		break;
 
 	case OMAP_DSS_ROT_180 + 4:
@@ -1584,10 +1595,14 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 - field_offset * screen_width * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(1 - screen_width * 2 -
+		*row_inc = pixinc(1 - y_predecim * screen_width * 2 -
 				(fieldmode ? screen_width : 0),
 				ps);
-		*pix_inc = pixinc(1, ps);
+		if (color_mode = OMAP_DSS_COLOR_YUV2 ||
+			color_mode = OMAP_DSS_COLOR_UYVY)
+			*pix_inc = pixinc(x_predecim, 2 * ps);
+		else
+			*pix_inc = pixinc(x_predecim, ps);
 		break;
 
 	case OMAP_DSS_ROT_270 + 4:
@@ -1596,10 +1611,10 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
 			*offset0 = *offset1 - field_offset * ps;
 		else
 			*offset0 = *offset1;
-		*row_inc = pixinc(screen_width * (fbh - 1) - 1 -
-				(fieldmode ? 1 : 0),
+		*row_inc = pixinc(screen_width * (fbh * x_predecim - 1) -
+				y_predecim - (fieldmode ? 1 : 0),
 				ps);
-		*pix_inc = pixinc(-screen_width, ps);
+		*pix_inc = pixinc(-x_predecim * screen_width, ps);
 		break;
 
 	default:
@@ -1686,13 +1701,17 @@ static unsigned long calc_fclk(enum omap_channel channel, u16 width,
 static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		enum omap_channel channel, u16 width, u16 height,
 		u16 out_width, u16 out_height,
-		enum omap_color_mode color_mode, bool *five_taps)
+		enum omap_color_mode color_mode, bool *five_taps,
+		int *x_predecim, int *y_predecim)
 {
 	struct omap_overlay *ovl = omap_dss_get_overlay(plane);
 	const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
 	const int maxsinglelinewidth  				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+	const int max_decim_limit = 16;
 	unsigned long fclk = 0;
+	int decim_x, decim_y, error, min_factor;
+	u16 in_width, in_height, in_width_max = 0;
 
 	if (width = out_width && height = out_height)
 		return 0;
@@ -1700,45 +1719,117 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	if ((ovl->caps & OMAP_DSS_OVL_CAP_SCALE) = 0)
 		return -EINVAL;
 
-	if (out_width < width / maxdownscale ||
-			out_width > width * 8)
+	*x_predecim = max_decim_limit;
+	*y_predecim = max_decim_limit;
+
+	if (color_mode = OMAP_DSS_COLOR_CLUT1 ||
+	    color_mode = OMAP_DSS_COLOR_CLUT2 ||
+	    color_mode = OMAP_DSS_COLOR_CLUT4 ||
+	    color_mode = OMAP_DSS_COLOR_CLUT8) {
+		*x_predecim = 1;
+		*y_predecim = 1;
+		*five_taps = false;
+		return 0;
+	}
+
+	decim_x = DIV_ROUND_UP(DIV_ROUND_UP(width, out_width), maxdownscale);
+	decim_y = DIV_ROUND_UP(DIV_ROUND_UP(height, out_height), maxdownscale);
+
+	min_factor = min(decim_x, decim_y);
+
+	if (decim_x > *x_predecim || out_width > width * 8)
 		return -EINVAL;
 
-	if (out_height < height / maxdownscale ||
-			out_height > height * 8)
+	if (decim_y > *y_predecim || out_height > height * 8)
 		return -EINVAL;
 
 	if (cpu_is_omap24xx()) {
-		if (width > maxsinglelinewidth)
-			DSSERR("Cannot scale max input width exceeded");
 		*five_taps = false;
-		fclk = calc_fclk(channel, width, height, out_width,
-								out_height);
+
+		do {
+			in_height = DIV_ROUND_UP(height, decim_y);
+			in_width = DIV_ROUND_UP(width, decim_x);
+			fclk = calc_fclk(channel, in_width, in_height,
+					out_width, out_height);
+			error = (in_width > maxsinglelinewidth || !fclk ||
+				fclk > dispc_fclk_rate());
+			if (error) {
+				if (decim_x = decim_y) {
+					decim_x = min_factor;
+					decim_y++;
+				} else {
+					swap(decim_x, decim_y);
+					if (decim_x < decim_y)
+						decim_x++;
+				}
+			}
+		} while (decim_x <= *x_predecim && decim_y <= *y_predecim &&
+				error);
+
+		if (in_width > maxsinglelinewidth) {
+			DSSERR("Cannot scale max input width exceeded");
+			return -EINVAL;
+		}
 	} else if (cpu_is_omap34xx()) {
-		if (width > (maxsinglelinewidth * 2)) {
+
+		do {
+			in_height = DIV_ROUND_UP(height, decim_y);
+			in_width = DIV_ROUND_UP(width, decim_x);
+			fclk = calc_fclk_five_taps(channel, in_width, in_height,
+				out_width, out_height, color_mode);
+
+			if (in_width > maxsinglelinewidth)
+				if (in_height > out_height &&
+					in_height < out_height * 2)
+					*five_taps = false;
+			if (!*five_taps)
+				fclk = calc_fclk(channel, in_width, in_height,
+					out_width, out_height);
+			error = (in_width > maxsinglelinewidth * 2 ||
+				(in_width > maxsinglelinewidth && *five_taps) ||
+				!fclk || fclk > dispc_fclk_rate());
+			if (error) {
+				if (decim_x = decim_y) {
+					decim_x = min_factor;
+					decim_y++;
+				} else {
+					swap(decim_x, decim_y);
+					if (decim_x < decim_y)
+						decim_x++;
+				}
+			}
+		} while (decim_x <= *x_predecim && decim_y <= *y_predecim
+			&& error);
+
+		if (in_width > (maxsinglelinewidth * 2)) {
 			DSSERR("Cannot setup scaling");
 			DSSERR("width exceeds maximum width possible");
 			return -EINVAL;
 		}
-		fclk = calc_fclk_five_taps(channel, width, height, out_width,
-						out_height, color_mode);
-		if (width > maxsinglelinewidth) {
-			if (height > out_height && height < out_height * 2)
-				*five_taps = false;
-			else {
-				DSSERR("cannot setup scaling with five taps");
-				return -EINVAL;
-			}
+
+		if (in_width > maxsinglelinewidth && *five_taps) {
+			DSSERR("cannot setup scaling with five taps");
+			return -EINVAL;
 		}
-		if (!*five_taps)
-			fclk = calc_fclk(channel, width, height, out_width,
-					out_height);
 	} else {
-		if (width > maxsinglelinewidth) {
+		in_height = DIV_ROUND_UP(height, decim_y);
+		in_width_max = dispc_fclk_rate() /
+				DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
+						out_width);
+		decim_x = DIV_ROUND_UP(width, in_width_max);
+		if (decim_x > *x_predecim)
+			return -EINVAL;
+		do {
+			in_width = DIV_ROUND_UP(width, decim_x);
+		} while (decim_x <= *x_predecim &&
+				in_width > maxsinglelinewidth && decim_x++);
+
+		if (in_width > maxsinglelinewidth) {
 			DSSERR("Cannot scale width exceeds max line width");
 			return -EINVAL;
 		}
-		fclk = calc_fclk(channel, width, height, out_width,
+
+		fclk = calc_fclk(channel, in_width, in_height, out_width,
 				out_height);
 	}
 
@@ -1753,6 +1844,8 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		return -EINVAL;
 	}
 
+	*x_predecim = decim_x;
+	*y_predecim = decim_y;
 	return 0;
 }
 
@@ -1768,8 +1861,11 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 	s32 pix_inc;
 	u16 frame_height = oi->height;
 	unsigned int field_offset = 0;
-	u16 outw, outh;
+	u16 in_height = oi->height;
+	u16 in_width = oi->width;
+	u16 out_width, out_height;
 	enum omap_channel channel;
+	int x_predecim = 1, y_predecim = 1;
 
 	channel = dispc_ovl_get_channel_out(plane);
 
@@ -1783,32 +1879,35 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 	if (oi->paddr = 0)
 		return -EINVAL;
 
-	outw = oi->out_width = 0 ? oi->width : oi->out_width;
-	outh = oi->out_height = 0 ? oi->height : oi->out_height;
+	out_width = oi->out_width = 0 ? oi->width : oi->out_width;
+	out_height = oi->out_height = 0 ? oi->height : oi->out_height;
 
-	if (ilace && oi->height = outh)
+	if (ilace && oi->height = out_height)
 		fieldmode = 1;
 
 	if (ilace) {
 		if (fieldmode)
-			oi->height /= 2;
+			in_height /= 2;
 		oi->pos_y /= 2;
-		outh /= 2;
+		out_height /= 2;
 
 		DSSDBG("adjusting for ilace: height %d, pos_y %d, "
 				"out_height %d\n",
-				oi->height, oi->pos_y, outh);
+				in_height, oi->pos_y, out_height);
 	}
 
 	if (!dss_feat_color_mode_supported(plane, oi->color_mode))
 		return -EINVAL;
 
-	r = dispc_ovl_calc_scaling(plane, channel, oi->width, oi->height,
-			outw, outh, oi->color_mode,
-			&five_taps);
+	r = dispc_ovl_calc_scaling(plane, channel, in_width, in_height,
+			out_width, out_height, oi->color_mode, &five_taps,
+			&x_predecim, &y_predecim);
 	if (r)
 		return r;
 
+	in_width = DIV_ROUND_UP(in_width, x_predecim);
+	in_height = DIV_ROUND_UP(in_height, y_predecim);
+
 	if (oi->color_mode = OMAP_DSS_COLOR_YUV2 ||
 			oi->color_mode = OMAP_DSS_COLOR_UYVY ||
 			oi->color_mode = OMAP_DSS_COLOR_NV12)
@@ -1822,10 +1921,10 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 		 * so the integer part must be added to the base address of the
 		 * bottom field.
 		 */
-		if (!oi->height || oi->height = outh)
+		if (!in_height || in_height = out_height)
 			field_offset = 0;
 		else
-			field_offset = oi->height / outh / 2;
+			field_offset = in_height / out_height / 2;
 	}
 
 	/* Fields are independent but interleaved in memory. */
@@ -1834,14 +1933,16 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 
 	if (oi->rotation_type = OMAP_DSS_ROT_DMA)
 		calc_dma_rotation_offset(oi->rotation, oi->mirror,
-				oi->screen_width, oi->width, frame_height,
+				oi->screen_width, in_width, frame_height,
 				oi->color_mode, fieldmode, field_offset,
-				&offset0, &offset1, &row_inc, &pix_inc);
+				&offset0, &offset1, &row_inc, &pix_inc,
+				x_predecim, y_predecim);
 	else
 		calc_vrfb_rotation_offset(oi->rotation, oi->mirror,
-				oi->screen_width, oi->width, frame_height,
+				oi->screen_width, in_width, frame_height,
 				oi->color_mode, fieldmode, field_offset,
-				&offset0, &offset1, &row_inc, &pix_inc);
+				&offset0, &offset1, &row_inc, &pix_inc,
+				x_predecim, y_predecim);
 
 	DSSDBG("offset0 %u, offset1 %u, row_inc %d, pix_inc %d\n",
 			offset0, offset1, row_inc, pix_inc);
@@ -1860,19 +1961,18 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
 	dispc_ovl_set_row_inc(plane, row_inc);
 	dispc_ovl_set_pix_inc(plane, pix_inc);
 
-	DSSDBG("%d,%d %dx%d -> %dx%d\n", oi->pos_x, oi->pos_y, oi->width,
-			oi->height, outw, outh);
+	DSSDBG("%d,%d %dx%d -> %dx%d\n", oi->pos_x, oi->pos_y, in_width,
+			in_height, out_width, out_height);
 
 	dispc_ovl_set_pos(plane, oi->pos_x, oi->pos_y);
 
-	dispc_ovl_set_pic_size(plane, oi->width, oi->height);
+	dispc_ovl_set_pic_size(plane, in_width, in_height);
 
 	if (ovl->caps & OMAP_DSS_OVL_CAP_SCALE) {
-		dispc_ovl_set_scaling(plane, oi->width, oi->height,
-				   outw, outh,
-				   ilace, five_taps, fieldmode,
+		dispc_ovl_set_scaling(plane, in_width, in_height, out_width,
+				   out_height, ilace, five_taps, fieldmode,
 				   oi->color_mode, oi->rotation);
-		dispc_ovl_set_vid_size(plane, outw, outh);
+		dispc_ovl_set_vid_size(plane, out_width, out_height);
 		dispc_ovl_set_vid_color_conv(plane, cconv);
 	}
 
-- 
1.7.1


^ permalink raw reply related

* [PATCH V2 0/3] OMAPDSS: DISPC: Enable predecimation for DMA and VRFB
From: Chandrabhanu Mahapatra @ 2012-03-21  9:52 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra

Hi everyone,
the following patch set directs to enable predecimation for DMA and VRFB
which consists of two pacthes.

The first patch is based on code written by Lajos Molnar <lajos@ti.com> in
Android Kernel, which updates the code with predecimation logic thereby
increasing the downscaling ability of the DISPC module.
In version 2 patch description has been modified include reason for maximum
predecimation limit of 16.

The second patch is based on code written by Ville Syrjälä
<ville.syrjala@nokia.com> which aims to avoid synclost errors occurring
in OMAP3 due to some undocumented horizontal position and timing related
limitations which I faced during testing of the previous patch.
In version 2 the pcd factor has been replaced lclk / pclk to support HDMI and
tv output as pointed out by Ville Syrjälä.

The third patch corrects the usage of dispc fclk in scaling checks by
replacing it with dispc_core_clk as per suggestions of Ville Syrjälä.

I have tested these patches successfully on OMAP2, OMAP3 AND OMAP4 on the
mainline kernel v3.3. Horizontal and vertical predecimation worked fine
but skewed images were seen on OMAP2 and OMAP3 on HDMI tv during horizontal
predecimation which will be addressed in the future patches.

All your comments and suggestions are welcome.

Regards,
Chandrabhanu

Chandrabhanu Mahapatra (3):
  OMAPDSS: DISPC: Enable predecimation
  OMAPDSS: DISPC: Handle synclost errors in OMAP3
  OMAPDSS: DISPC: Correct DISPC functional clock usage

 drivers/video/omap2/dss/dispc.c |  360 +++++++++++++++++++++++++++------------
 1 files changed, 254 insertions(+), 106 deletions(-)


^ permalink raw reply

* Re: [PATCH v2] x86: export 'pcibios_enabled' as GPL
From: Alan Cox @ 2012-03-21  9:29 UTC (permalink / raw)
  To: Wang YanQing
  Cc: Randy Dunlap, Stephen Rothwell, linux-next, LKML,
	Michal Januszewski, Florian Tobias Schandinat, linux-fbdev, x86,
	Andrew Morton, tglx, mingo, hpa
In-Reply-To: <20120321043721.GA927@udknight>

On Wed, 21 Mar 2012 12:37:21 +0800
Wang YanQing <udknight@gmail.com> wrote:

> On Sun, Mar 18, 2012 at 06:03:41PM -0700, Randy Dunlap wrote:
> > From: Randy Dunlap <rdunlap@xenotime.net>
> > 
> > Export 'pcibios_enabled' so that when uvesafb is built as a
> > loadable module (on X86_32), the build will succeed.
> > 
> > ERROR: "pcibios_enabled" [drivers/video/uvesafb.ko] undefined!
> > 
> > Patch v2 uses EXPORT_SYMBOL_GPL() as requested.
> > 
> > Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
> > Cc:	Michal Januszewski <spock@gentoo.org>
> > Cc:	Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> > Cc:	linux-fbdev@vger.kernel.org
> > Cc:	x86@kernel.org
> > ---
> >  arch/x86/pci/pcbios.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > --- linux-next-20120313.orig/arch/x86/pci/pcbios.c
> > +++ linux-next-20120313/arch/x86/pci/pcbios.c
> > @@ -27,6 +27,7 @@
> >  #define PCIBIOS_HW_TYPE2_SPEC		0x20
> >  
> >  int pcibios_enabled;
> > +EXPORT_SYMBOL_GPL(pcibios_enabled);
> >  
> >  /* According to the BIOS specification at:
> >   * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could
> Acked-by: Wang YanQing <udknight@gmail.com>

NAKked again by Alan Cox <alan@linux.intel.com>

The fb code shouldn't be playing guessing games with thsi variable.
I've pointed out both how to check if NX is enabled and how to set pages
NX/non-NX.

Even if you wanted to check the pci bios state we should export a helper
method.

So NAK.

Alan

^ permalink raw reply

* Re: [PATCH v2] x86: export 'pcibios_enabled' as GPL
From: Wang YanQing @ 2012-03-21  4:37 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Stephen Rothwell, linux-next, LKML, Michal Januszewski,
	Florian Tobias Schandinat, linux-fbdev, x86, Andrew Morton, tglx,
	mingo, hpa
In-Reply-To: <4F6685ED.7040000@xenotime.net>

On Sun, Mar 18, 2012 at 06:03:41PM -0700, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@xenotime.net>
> 
> Export 'pcibios_enabled' so that when uvesafb is built as a
> loadable module (on X86_32), the build will succeed.
> 
> ERROR: "pcibios_enabled" [drivers/video/uvesafb.ko] undefined!
> 
> Patch v2 uses EXPORT_SYMBOL_GPL() as requested.
> 
> Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
> Cc:	Michal Januszewski <spock@gentoo.org>
> Cc:	Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc:	linux-fbdev@vger.kernel.org
> Cc:	x86@kernel.org
> ---
>  arch/x86/pci/pcbios.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- linux-next-20120313.orig/arch/x86/pci/pcbios.c
> +++ linux-next-20120313/arch/x86/pci/pcbios.c
> @@ -27,6 +27,7 @@
>  #define PCIBIOS_HW_TYPE2_SPEC		0x20
>  
>  int pcibios_enabled;
> +EXPORT_SYMBOL_GPL(pcibios_enabled);
>  
>  /* According to the BIOS specification at:
>   * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could
Acked-by: Wang YanQing <udknight@gmail.com>

^ permalink raw reply

* Re: [PATCH] Added backlight driver for Acer Aspire 4736
From: joeyli @ 2012-03-21  3:00 UTC (permalink / raw)
  To: Pradeep Subrahmanion
  Cc: Matthew Garrett, rpurdie, FlorianSchandinat, akpm, linux-fbdev,
	linux-kernel
In-Reply-To: <1332269710.3921.31.camel@debian.Gayathri>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 5181 bytes --]

於 三,2012-03-21 於 00:25 +0530,Pradeep Subrahmanion 提到:
> > >From 5da43d2ee6c87dcf17fda34f0b50fe11b04a16bf Mon Sep 17 00:00:00 2001
> > From: "Lee, Chun-Yi" <jlee@suse.com>
> > Date: Tue, 20 Mar 2012 19:00:58 +0800
> > Subject: [PATCH] acer-wmi: add quirk table for video backlight vendor mode
> > 
> > There have some acer laptop have broken _BCM implemenation, the AML
> > code wrote value to EC register but firmware didn't change brighenss.
> > 
> > Fortunately, the brightness control works on those machines with
> > vendor mode. So, add quirk table for video backlight vendor mode
> > and unregister acpi video interface on those machines.
> > 
> > Tested on Acer TravelMate 4750
> > 
> > Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
> > ---
> >  drivers/platform/x86/Kconfig    |    4 ++++
> >  drivers/platform/x86/acer-wmi.c |   38 +++++++++++++++++++++++++++++++++++---
> >  2 files changed, 39 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 15dbd8c..fe3a494 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -26,6 +26,10 @@ config ACER_WMI
> >  	depends on RFKILL || RFKILL = n
> >  	depends on ACPI_WMI
> >  	select INPUT_SPARSEKMAP
> > +	# Acer WMI depends on ACPI_VIDEO when ACPI is enabled
> > +	# but for select to work, need to select ACPI_VIDEO's dependencies, ick
> > +        select VIDEO_OUTPUT_CONTROL if ACPI
> > +        select ACPI_VIDEO if ACPI
> >  	---help---
> >  	  This is a driver for newer Acer (and Wistron) laptops. It adds
> >  	  wireless radio and bluetooth control, and on some laptops,
> > diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> > index 1e5290b..984a7b5 100644
> > --- a/drivers/platform/x86/acer-wmi.c
> > +++ b/drivers/platform/x86/acer-wmi.c
> > @@ -43,6 +43,7 @@
> >  #include <linux/input/sparse-keymap.h>
> >  
> >  #include <acpi/acpi_drivers.h>
> > +#include <acpi/video.h>
> >  
> >  MODULE_AUTHOR("Carlos Corbacho");
> >  MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
> > @@ -478,6 +479,33 @@ static struct dmi_system_id acer_quirks[] = {
> >  	{}
> >  };
> >  
> > +static int video_set_backlight_video_vendor(const struct dmi_system_id *d)
> > +{
> > +	interface->capability &= ~ACER_CAP_BRIGHTNESS;
> > +	pr_info("Brightness must be controlled by generic video driver\n");
> > +	return 0;
> > +}
> > +
> > +static const struct dmi_system_id video_vendor_dmi_table[] = {
> > +	{
> > +		.callback = video_set_backlight_video_vendor,
> > +		.ident = "Acer Aspire 4736",
> > +		.matches = {
> > +			DMI_MATCH(DMI_BOARD_VENDOR, "Acer"),
> > +			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 4736"),
> > +		},
> > +	},
> > +	{
> > +		.callback = video_set_backlight_video_vendor,
> > +		.ident = "Acer TravelMate 4750",
> > +		.matches = {
> > +			DMI_MATCH(DMI_BOARD_VENDOR, "Acer"),
> > +			DMI_MATCH(DMI_PRODUCT_NAME, "TravelMate 4750"),
> > +		},
> > +	},
> > +	{}
> > +};
> > +
> >  /* Find which quirks are needed for a particular vendor/ model pair */
> >  static void find_quirks(void)
> >  {
> > @@ -1981,9 +2009,13 @@ static int __init acer_wmi_init(void)
> >  	set_quirks();
> >  
> >  	if (acpi_video_backlight_support()) {
> > -		interface->capability &= ~ACER_CAP_BRIGHTNESS;
> > -		pr_info("Brightness must be controlled by "
> > -		       "generic video driver\n");
> > +		if (dmi_check_system(video_vendor_dmi_table)) {
> > +			acpi_video_unregister();
> > +		} else {
> > +			interface->capability &= ~ACER_CAP_BRIGHTNESS;
> > +			pr_info("Brightness must be controlled by "
> > +				"acpi video driver\n");
> > +		}
> >  	}
> >  
> >  	if (wmi_has_guid(WMID_GUID3)) {
> 
> I tried out applied your patch . Boot message shows , 
> 
> [11.220410] acer_wmi: Brightness must be controlled by generic video
> driver
> 
> Now 'acpi_video0' and 'intel_backlight' are present
> inside /sys/class/backlight .Hot key works like earlier ( ie problem
> after maximum level still exists).
> I tried following commands , 
> 

It's not the expected behavior.

This new patch should remove acpi_video0 interface on your machine.
Please kindly provide your dmidecode:
 dmidecode > dmidecode.log

And, 
please make should the patch really applied, you can do a bit change on
pr_info message by yourself.

> echo 5 > /sys/class/backlight/acpi_video0/brightness 
> echo 5 > /sys/class/backlight/intel_backlight/brightness
> 
> But it doesn't make any change . 
> 
> In my case  , 'acpi_backlight = vendor' does not make any difference since the hot key control 
> 
> already starts working with 'acpi_osi=Linux' option.
> 
> cat /proc/cmdline gives , 
> 
> BOOT_IMAGE=/boot/vmlinuz-3.3.0+
> root=UUIDð197a59-c067-4fd8-ad90-c4d721816077 ro acpi_osi=Linux 
> 
> 
> Thanks , 
> 
> Pradeep Subrahmanion
> 

OK, that's more clearly, please remove acpi_osi=Linux then re-test, I
think the hotkey of backlight control only works with acpi_osi=Linux ?


Thanks a lot!
Joey Lee


--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v2] fbdev: sh_mipi_dsi: add extra phyctrl for sh_mipi_dsi_info
From: Kuninori Morimoto @ 2012-03-21  1:34 UTC (permalink / raw)
  To: linux-fbdev

sh_mipi uses some clocks, but the method of setup depends on CPU.

Current SuperH (like sh73a0) can control all of these clocks
by CPG (Clock Pulse Generator).
It means we can control it by clock framework only.
But on sh7372, it needs CPG settings AND sh_mipi PHYCTRL::PLLDS,
and only sh7372 has PHYCTRL::PLLDS.

But on current sh_mipi driver, PHYCTRL::PLLDS of sh7372 was
overwrote since the callback timing of clock setting was changed
by c2658b70f06108361aa5024798f9c1bf47c73374
(fbdev: sh_mipi_dsi: fixup setup timing of sh_mipi_setup()).
To solve this issue, this patch adds extra .phyctrl.

This patch adds detail explanation for unclear mipi settings
and fixup wrong PHYCTRL::PLLDS value for ap4evb (0xb -> 0x6).

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2

- fixup wrong MHz (321 -> 312)
- Adds explain that this patch fixup wrong PLLDS value for ap4evb

 arch/arm/mach-shmobile/board-ap4evb.c |   12 +++++++++---
 drivers/video/sh_mipi_dsi.c           |    2 +-
 include/video/sh_mipi_dsi.h           |    1 +
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index eeb4d96..50fcf1b 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -556,20 +556,25 @@ static struct platform_device keysc_device = {
 };
 
 /* MIPI-DSI */
-#define PHYCTRL		0x0070
 static int sh_mipi_set_dot_clock(struct platform_device *pdev,
 				 void __iomem *base,
 				 int enable)
 {
 	struct clk *pck = clk_get(&pdev->dev, "dsip_clk");
-	void __iomem *phy =  base + PHYCTRL;
 
 	if (IS_ERR(pck))
 		return PTR_ERR(pck);
 
 	if (enable) {
+		/*
+		 * DSIPCLK	= 24MHz
+		 * D-PHY	= DSIPCLK * ((0x6*2)+1) = 312MHz (see .phyctrl)
+		 * HsByteCLK	= D-PHY/8 = 39MHz
+		 *
+		 *  X * Y * FPS +		 * (544+72+600+16) * (961+8+8+2) * 30 = 36.1MHz
+		 */
 		clk_set_rate(pck, clk_round_rate(pck, 24000000));
-		iowrite32(ioread32(phy) | (0xb << 8), phy);
 		clk_enable(pck);
 	} else {
 		clk_disable(pck);
@@ -598,6 +603,7 @@ static struct sh_mipi_dsi_info mipidsi0_info = {
 	.lcd_chan	= &lcdc_info.ch[0],
 	.lane		= 2,
 	.vsynw_offset	= 17,
+	.phyctrl	= 0x6 << 8,
 	.flags		= SH_MIPI_DSI_SYNC_PULSES_MODE |
 			  SH_MIPI_DSI_HSbyteCLK,
 	.set_dot_clock	= sh_mipi_set_dot_clock,
diff --git a/drivers/video/sh_mipi_dsi.c b/drivers/video/sh_mipi_dsi.c
index 05151b8..214482c 100644
--- a/drivers/video/sh_mipi_dsi.c
+++ b/drivers/video/sh_mipi_dsi.c
@@ -271,7 +271,7 @@ static int __init sh_mipi_setup(struct sh_mipi *mipi,
 	iowrite32(0x00000001, base + PHYCTRL);
 	udelay(200);
 	/* Deassert resets, power on */
-	iowrite32(0x03070001, base + PHYCTRL);
+	iowrite32(0x03070001 | pdata->phyctrl, base + PHYCTRL);
 
 	/*
 	 * Default = ULPS enable |
diff --git a/include/video/sh_mipi_dsi.h b/include/video/sh_mipi_dsi.h
index 434d56b..06c67fb 100644
--- a/include/video/sh_mipi_dsi.h
+++ b/include/video/sh_mipi_dsi.h
@@ -51,6 +51,7 @@ struct sh_mipi_dsi_info {
 	int				lane;
 	unsigned long			flags;
 	u32				clksrc;
+	u32				phyctrl; /* for extra setting */
 	unsigned int			vsynw_offset;
 	int	(*set_dot_clock)(struct platform_device *pdev,
 				 void __iomem *base,
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH v4] fbdev: remove dependency of FB_SH_MOBILE_MERAM from FB_SH_MOBILE_LCDC
From: Kuninori Morimoto @ 2012-03-21  1:27 UTC (permalink / raw)
  To: linux-fbdev

MERAM can be used for other IP blocks as well in the future.
It doesn't necessarily mean that the MERAM driver depends on the LCDC.
This patch corrects dependency.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v3 -> v4

- move config to before endpoint

 drivers/video/Kconfig |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 8951cbd..0fb9540 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2013,18 +2013,6 @@ config FB_SH_MOBILE_HDMI
 	---help---
 	  Driver for the on-chip SH-Mobile HDMI controller.
 
-config FB_SH_MOBILE_MERAM
-	tristate "SuperH Mobile MERAM read ahead support for LCDC"
-	depends on FB_SH_MOBILE_LCDC
-	default y
-	---help---
-	  Enable MERAM support for the SH-Mobile LCD controller.
-
-	  This will allow for caching of the framebuffer to provide more
-	  reliable access under heavy main memory bus traffic situations.
-	  Up to 4 memory channels can be configured, allowing 4 RGB or
-	  2 YCbCr framebuffers to be configured.
-
 config FB_TMIO
 	tristate "Toshiba Mobile IO FrameBuffer support"
 	depends on FB && MFD_CORE
@@ -2434,4 +2422,15 @@ if FB || SGI_NEWPORT_CONSOLE
 	source "drivers/video/logo/Kconfig"
 endif
 
+config FB_SH_MOBILE_MERAM
+	tristate "SuperH Mobile MERAM read ahead support"
+	depends on (SUPERH || ARCH_SHMOBILE)
+	---help---
+	  Enable MERAM support for the SuperH controller.
+
+	  This will allow for caching of the framebuffer to provide more
+	  reliable access under heavy main memory bus traffic situations.
+	  Up to 4 memory channels can be configured, allowing 4 RGB or
+	  2 YCbCr framebuffers to be configured.
+
 endmenu
-- 
1.7.5.4


^ permalink raw reply related

* Re: [PATCH v3] fbdev: remove dependency of FB_SH_MOBILE_MERAM from FB_SH_MOBILE_LCDC
From: Kuninori Morimoto @ 2012-03-21  1:10 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <87d38qwmi9.wl%kuninori.morimoto.gx@renesas.com>


Hi Florian

> > MERAM can be used for other IP blocks as well in the future.
> > It doesn't necessarily mean that the MERAM driver depends on the LCDC.
> > This patch corrects dependency.
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > ---
> > v2 -> v3
> > 
> > - move config FB_SH_MOBILE_MERAM out of "choice"
> 
> I am afraid but this is still not correct. The problem is that after
> your patch this config option has no longer any dependency on the
> framebuffer subsystem. As "FB" is a menuconfig option I guess the first
> config option without any direct or indirect dependency on FB terminates
> the menu. As I don't understand how this driver is used I don't know how
> I should resolve this: either leave it where it is and just add an
> dependency on "FB" or move it even further down, probably just before
> endmenu (AFAIK the first thing that breaks it is OMAP2).

Thank you for checking my patch.
And sorry for breaking menuconfig

As Laurent/Hayama-san said that the MERAM will be used
from VIDEO_SH_MOBILE_CEU (${LINUX}/drivers/media/video/Kconfig)
in the future.

I move it to before endmenu (without "FB" dependency)

Best regards
---
Kuninori Morimoto

^ permalink raw reply

* Re: [PATCH] fbdev: sh_mipi_dsi: add extra phyctrl for sh_mipi_dsi_info
From: Kuninori Morimoto @ 2012-03-21  0:57 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <87y5rdnmmu.wl%kuninori.morimoto.gx@renesas.com>


Hi Florian

Thank you for checking patch

> > sh_mipi uses some clocks, but the method of setup depends on CPU.
> > 
> > Current SuperH (like sh73a0) can control all of these clocks
> > by CPG (Clock Pulse Generator).
> > It means we can control it by clock framework only.
> > But on sh7372, it needs CPG settings AND sh_mipi PHYCTRL::PLLDS,
> > and only sh7372 has PHYCTRL::PLLDS.
> > 
> > But on current sh_mipi driver, PHYCTRL::PLLDS of sh7372 was
> > overwrote since the callback timing of clock setting was changed
> > by c2658b70f06108361aa5024798f9c1bf47c73374
> > (fbdev: sh_mipi_dsi: fixup setup timing of sh_mipi_setup()).
> > 
> > The difference of PHYCTRL between current SuperH (=sh73a0) and
> > old SuperH (=sh7372) is not only PLLDS.
> > So this patch adds extra .phyctrl.
> > 
> > It also adds detail explanation for unclear mipi settings for ap4evb.
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > ---
(snip)
> >  	if (enable) {
> > +		/*
> > +		 * DSIPCLK	= 24MHz
> > +		 * D-PHY	= DSIPCLK * ((0x6*2)+1) = 321MHz (see .phyctrl)
> 
> Shouldn't this be 312?

Oops, indeed.
I fix it.

> > +		 * HsByteCLK	= D-PHY/8 = 39MHz
> > +		 *
> > +		 *  X * Y * FPS > > +		 * (544+72+600+16) * (961+8+8+2) * 30 = 36.1MHz
> > +		 */
> >  		clk_set_rate(pck, clk_round_rate(pck, 24000000));
> > -		iowrite32(ioread32(phy) | (0xb << 8), phy);
> 
> Here you remove writing a 0xb<<8 (0xb = 11 = 8+2+1)...
> 
> >  		clk_enable(pck);
> >  	} else {
> >  		clk_disable(pck);
> > @@ -598,6 +603,7 @@ static struct sh_mipi_dsi_info mipidsi0_info = {
> >  	.lcd_chan	= &lcdc_info.ch[0],
> >  	.lane		= 2,
> >  	.vsynw_offset	= 17,
> > +	.phyctrl	= 0x6 << 8,
> 
> ...and here you set a 0x6<<8 (0x6 = 6 = 4+2). Looks suspicious, was this
> change intended?

Sorry. the git log didn't explain that this patch fixup it.
0x6 is correct here. This used for D-PHY.

 D-PHY	= DSIPCLK * ((0x6*2)+1) = 321MHz (see .phyctrl)
                      ~~~

I add it on v2 patch.

Best regards
---
Kuninori Morimoto

^ permalink raw reply

* Re: Howto consolidate xgifb (staging) with sisfb
From: Alex Deucher @ 2012-03-20 21:40 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <201203200002.40429.PeterHuewe@gmx.de>

On Tue, Mar 20, 2012 at 5:06 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Hi,
>
> On Tue, Mar 20, 2012 at 12:02:39AM +0100, Peter Hüwe wrote:
>> I'm currently trying to improve the xgifb driver from staging a bit and my
>> final goal is to probably merge this driver with the sis driver (as both card
>> families seem to have a lot in common).
>>
>> However I'm a little bit unsure how to proceed here.
>
> I would first continue deleting unused dead code and definitions from the
> staging driver. We have already removed maybe 10000 lines of code without
> a loss of functionality, and I believe there is still more work to do.
>
> Then with the remaining code, we could start thinking how to consolidate
> the stuff with sisfb. Or even with the other fb drivers, I don't think
> stuff like bpp_to_var is specific to these two drivers.
>
> And once xgifb is cleaned up, the partial XGI support would be dropped
> from sisfb.

You could probably just add the pci ids to sisfb;  The xgi chips were
just rebadged sis chips.  IIRC, xgifb was just a fork of sisfb with
the strings changed.  XGI was a combination of the old trident and sis
graphics teams.  Most of the xgi cards were just rebadged sis cards
with the exception of the xp10 which was a trident variant and could
probably be merged with the tridentfb driver if there is one.

For reference see the X drivers:
Sis-based:
http://cgit.freedesktop.org/xorg/driver/xf86-video-sis
http://cgit.freedesktop.org/xorg/driver/xf86-video-xgi

Trident-based:
http://cgit.freedesktop.org/xorg/driver/xf86-video-trident
http://cgit.freedesktop.org/xorg/driver/xf86-video-xgixp

Alex

^ permalink raw reply

* Re: Howto consolidate xgifb (staging) with sisfb
From: Aaro Koskinen @ 2012-03-20 21:06 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <201203200002.40429.PeterHuewe@gmx.de>

Hi,

On Tue, Mar 20, 2012 at 12:02:39AM +0100, Peter Hüwe wrote:
> I'm currently trying to improve the xgifb driver from staging a bit and my 
> final goal is to probably merge this driver with the sis driver (as both card 
> families seem to have a lot in common).
> 
> However I'm a little bit unsure how to proceed here.

I would first continue deleting unused dead code and definitions from the
staging driver. We have already removed maybe 10000 lines of code without
a loss of functionality, and I believe there is still more work to do.

Then with the remaining code, we could start thinking how to consolidate
the stuff with sisfb. Or even with the other fb drivers, I don't think
stuff like bpp_to_var is specific to these two drivers.

And once xgifb is cleaned up, the partial XGI support would be dropped
from sisfb.

A.

^ permalink raw reply

* Re: [PATCH] Added backlight driver for Acer Aspire 4736
From: Pradeep Subrahmanion @ 2012-03-20 18:55 UTC (permalink / raw)
  To: joeyli
  Cc: Matthew Garrett, rpurdie, FlorianSchandinat, akpm, linux-fbdev,
	linux-kernel
In-Reply-To: <1332241764.10557.329.camel@linux-s257.site>


> >From 5da43d2ee6c87dcf17fda34f0b50fe11b04a16bf Mon Sep 17 00:00:00 2001
> From: "Lee, Chun-Yi" <jlee@suse.com>
> Date: Tue, 20 Mar 2012 19:00:58 +0800
> Subject: [PATCH] acer-wmi: add quirk table for video backlight vendor mode
> 
> There have some acer laptop have broken _BCM implemenation, the AML
> code wrote value to EC register but firmware didn't change brighenss.
> 
> Fortunately, the brightness control works on those machines with
> vendor mode. So, add quirk table for video backlight vendor mode
> and unregister acpi video interface on those machines.
> 
> Tested on Acer TravelMate 4750
> 
> Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
> ---
>  drivers/platform/x86/Kconfig    |    4 ++++
>  drivers/platform/x86/acer-wmi.c |   38 +++++++++++++++++++++++++++++++++++---
>  2 files changed, 39 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 15dbd8c..fe3a494 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -26,6 +26,10 @@ config ACER_WMI
>  	depends on RFKILL || RFKILL = n
>  	depends on ACPI_WMI
>  	select INPUT_SPARSEKMAP
> +	# Acer WMI depends on ACPI_VIDEO when ACPI is enabled
> +	# but for select to work, need to select ACPI_VIDEO's dependencies, ick
> +        select VIDEO_OUTPUT_CONTROL if ACPI
> +        select ACPI_VIDEO if ACPI
>  	---help---
>  	  This is a driver for newer Acer (and Wistron) laptops. It adds
>  	  wireless radio and bluetooth control, and on some laptops,
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index 1e5290b..984a7b5 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -43,6 +43,7 @@
>  #include <linux/input/sparse-keymap.h>
>  
>  #include <acpi/acpi_drivers.h>
> +#include <acpi/video.h>
>  
>  MODULE_AUTHOR("Carlos Corbacho");
>  MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
> @@ -478,6 +479,33 @@ static struct dmi_system_id acer_quirks[] = {
>  	{}
>  };
>  
> +static int video_set_backlight_video_vendor(const struct dmi_system_id *d)
> +{
> +	interface->capability &= ~ACER_CAP_BRIGHTNESS;
> +	pr_info("Brightness must be controlled by generic video driver\n");
> +	return 0;
> +}
> +
> +static const struct dmi_system_id video_vendor_dmi_table[] = {
> +	{
> +		.callback = video_set_backlight_video_vendor,
> +		.ident = "Acer Aspire 4736",
> +		.matches = {
> +			DMI_MATCH(DMI_BOARD_VENDOR, "Acer"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 4736"),
> +		},
> +	},
> +	{
> +		.callback = video_set_backlight_video_vendor,
> +		.ident = "Acer TravelMate 4750",
> +		.matches = {
> +			DMI_MATCH(DMI_BOARD_VENDOR, "Acer"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "TravelMate 4750"),
> +		},
> +	},
> +	{}
> +};
> +
>  /* Find which quirks are needed for a particular vendor/ model pair */
>  static void find_quirks(void)
>  {
> @@ -1981,9 +2009,13 @@ static int __init acer_wmi_init(void)
>  	set_quirks();
>  
>  	if (acpi_video_backlight_support()) {
> -		interface->capability &= ~ACER_CAP_BRIGHTNESS;
> -		pr_info("Brightness must be controlled by "
> -		       "generic video driver\n");
> +		if (dmi_check_system(video_vendor_dmi_table)) {
> +			acpi_video_unregister();
> +		} else {
> +			interface->capability &= ~ACER_CAP_BRIGHTNESS;
> +			pr_info("Brightness must be controlled by "
> +				"acpi video driver\n");
> +		}
>  	}
>  
>  	if (wmi_has_guid(WMID_GUID3)) {

I tried out applied your patch . Boot message shows , 

[11.220410] acer_wmi: Brightness must be controlled by generic video
driver

Now 'acpi_video0' and 'intel_backlight' are present
inside /sys/class/backlight .Hot key works like earlier ( ie problem
after maximum level still exists).
I tried following commands , 

echo 5 > /sys/class/backlight/acpi_video0/brightness 
echo 5 > /sys/class/backlight/intel_backlight/brightness

But it doesn't make any change . 

In my case  , 'acpi_backlight = vendor' does not make any difference since the hot key control 

already starts working with 'acpi_osi=Linux' option.

cat /proc/cmdline gives , 

BOOT_IMAGE=/boot/vmlinuz-3.3.0+
root=UUIDð197a59-c067-4fd8-ad90-c4d721816077 ro acpi_osi=Linux 


Thanks , 

Pradeep Subrahmanion


^ permalink raw reply

* Re: [PATCH] Added backlight driver for Acer Aspire 4736
From: joeyli @ 2012-03-20 11:09 UTC (permalink / raw)
  To: Pradeep Subrahmanion
  Cc: Matthew Garrett, rpurdie, FlorianSchandinat, akpm, linux-fbdev,
	linux-kernel
In-Reply-To: <1332122514.10557.270.camel@linux-s257.site>

於 一,2012-03-19 於 10:01 +0800,joeyli 提到:
> 於 日,2012-03-18 於 10:40 +0530,Pradeep Subrahmanion 提到:
> > On Thu, 2012-03-15 at 16:05 +0800, joeyli wrote:
> > > Hi Pradeep, 
> > > 
> > > 於 三,2012-03-14 於 11:47 +0530,Pradeep Subrahmanion 提到:
> > > > On Wed, Mar 14, 2012 at 11:21 AM, joeyli <jlee@suse.com> wrote:
> > > > > 於 三,2012-03-14 於 08:13 +0530,Pradeep Subrahmanion 提到:
> > > > >> Hi Joey ,
> > > > >>
> > > > >> > Per my understood, EC firmware should change brightness but didn't do
> > > > >> > that, another
> > > > >> > way is touch i915 register in _BCM.
> > > > >>
> > > > >>   how do we do this ? you mean change the _BCM implementation ?
> > > > >
> > > > > "BIOS guy" should do something like this:
> > > > >
> > > > >  Method (AINT, 2, NotSerialized)
> > > > > {
> > > > > ...
> > > > >        If (LEqual (Arg0, One))
> > > > >        {
> > > > >            Store (Divide (Multiply (Arg1, 0xFF), 0x64, ), BCLP)
> > > > >            Or (BCLP, 0x80000000, BCLP)         <== touch BCLP register
> > > > >            Store (0x02, ASLC)
> > > > >        }
> > > > >
> > > > >    Method (_BCM, 1, NotSerialized)
> > > > >    {
> > > > >        If (LAnd (LGreaterEqual (Arg0, Zero), LLessEqual (Arg0, 0x64)))
> > > > >        {
> > > > >            AINT (One, Arg0)     <== call AINT method
> > > > >            Store (Arg0, BRTL)
> > > > >        }
> > > > >    }
> > > > >
> > > > >
> > > > > Just for reference, they should do that when EC didn't wire to
> > > > > backlight.
> > > > >
> > > > >> >
> > > > >> > Acer machine provide a broken _BCM implementation and they didn't test
> > > > >> > it.
> > > > >> >
> > > > >> > > > > By ' ACPI interface' , I mean 'acpi_video0' inside the
> > > > >> > > > > /sys/class/backlight. I havn't tried the /sys/class/backlight interface
> > > > >> > > > > directly . I will try that also.
> > > > >> > > >
> > > > >> > > > So writing values into /sys/class/backlight/acpi_video0/brightness does
> > > > >> > > > nothing?
> > > > >> > >
> > > > >> > >
> > > > >> > > No change in value when writing
> > > > >> > > to /sys/class/backlight/acpi_video0/brightness.
> > > > >> > >
> > > > >> > > Another thing is that when i did boot with acpi_backlight = 'acer_wmi' ,
> > > > >> > > in new kernel (3.3.0-rc7) , it shows following messages ,
> > > > >> > >
> > > > >> > > [    8.350825] wmi: Mapper loaded
> > > > >> > > [   10.363975] acer_wmi: Acer Laptop ACPI-WMI Extras
> > > > >> > > [   10.396186] acer_wmi: Function bitmap for Communication Device: 0x91
> > > > >> > > [   10.396385] acer_wmi: Brightness must be controlled by generic video
> > > > >> > > driver
> > > > >> > >
> > > > >> > > Also there was no interface inside /sys/class/backlight for acer_wmi.
> > > > >> > >
> > > > >> >
> > > > >> > Yes, acer_wmi support backlight control with AMW0 interface, your
> > > > >> > machine didn't have AMW0 interface.
> > > > >> >
> > > > >> > Normally, backlight should control by standard acpi interface.
> > > > >> >
> > > > >> > > I also tried writing directly to Embedded controller register .But no
> > > > >> > > change.
> > > > >> >
> > > > >> > The machine has broken _BCM method, because EC should do something after
> > > > >> > _BCM changed EC register.
> > > > >>
> > > > >> Thanks ,
> > > > >>
> > > > >> Pradeep Subrahmanion
> > > > >
> > > > > Why they didn't find _BCM not work?
> > > > >
> > > > > My guess is:
> > > > >
> > > > > Because the backlight control is through WDDM driver on Windows platform
> > > > > but not through standard ACPI method _BCM. They only test Windows
> > > > > platform, so, they didn't find _BCM broken.
> > > > >
> > > > > And, they also didn't really follow Microsoft WDDM spec:
> > > > >
> > > > >  http://msdn.microsoft.com/en-us/windows/hardware/gg487382.aspx
> > > > >
> > > > > Per spec,
> > > > > ODM should keep _BCM works fine for any other OS didn't support WDDM
> > > > > driver, but they didn't.
> > > > >
> > > > > At last year, I told Acer PM one time for this issue, they said will
> > > > > check but finally didn't response me.
> > > > >
> > > > 
> > > > >
> > > > > Thanks
> > > > > Joey Lee
> > > > >
> > > > 
> > > > So touching the PCI LBB register is the only feasible solution now
> > > > (even though it may not be a clean  method) ?
> > > > 
> > > > Thanks,
> > > > Pradeep Subrahmanion
> > > 
> > > That will be better leave LBB register only touched by i915 driver.
> > > 
> > > If 'acpi_backlight=vendor' works to you, maybe we can add a quirk to
> > > video_detect.c. 
> > > You can try the following patch.
> > 
> > Thanks . I tried your patch .acpi_backlight=vendor allows me to control
> > brightness with hot key.But there are problems with it like increasing
> 
> OK, thanks for your testing, I will send out patch and add Cc. to you.
> 

Could you please kindly try this new patch? I follow Matthew's kindly
suggestion put the quirk table to acer-wmi driver.

Please help to apply the following patch to acer-wmi and remember remove
my last patch of video_detect.c, then rebuild your kernel.

Hope this patch also can fix your problem.


Thanks a lot!
Joey Lee

From 5da43d2ee6c87dcf17fda34f0b50fe11b04a16bf Mon Sep 17 00:00:00 2001
From: "Lee, Chun-Yi" <jlee@suse.com>
Date: Tue, 20 Mar 2012 19:00:58 +0800
Subject: [PATCH] acer-wmi: add quirk table for video backlight vendor mode

There have some acer laptop have broken _BCM implemenation, the AML
code wrote value to EC register but firmware didn't change brighenss.

Fortunately, the brightness control works on those machines with
vendor mode. So, add quirk table for video backlight vendor mode
and unregister acpi video interface on those machines.

Tested on Acer TravelMate 4750

Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
---
 drivers/platform/x86/Kconfig    |    4 ++++
 drivers/platform/x86/acer-wmi.c |   38 +++++++++++++++++++++++++++++++++++---
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 15dbd8c..fe3a494 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -26,6 +26,10 @@ config ACER_WMI
 	depends on RFKILL || RFKILL = n
 	depends on ACPI_WMI
 	select INPUT_SPARSEKMAP
+	# Acer WMI depends on ACPI_VIDEO when ACPI is enabled
+	# but for select to work, need to select ACPI_VIDEO's dependencies, ick
+        select VIDEO_OUTPUT_CONTROL if ACPI
+        select ACPI_VIDEO if ACPI
 	---help---
 	  This is a driver for newer Acer (and Wistron) laptops. It adds
 	  wireless radio and bluetooth control, and on some laptops,
diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 1e5290b..984a7b5 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -43,6 +43,7 @@
 #include <linux/input/sparse-keymap.h>
 
 #include <acpi/acpi_drivers.h>
+#include <acpi/video.h>
 
 MODULE_AUTHOR("Carlos Corbacho");
 MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
@@ -478,6 +479,33 @@ static struct dmi_system_id acer_quirks[] = {
 	{}
 };
 
+static int video_set_backlight_video_vendor(const struct dmi_system_id *d)
+{
+	interface->capability &= ~ACER_CAP_BRIGHTNESS;
+	pr_info("Brightness must be controlled by generic video driver\n");
+	return 0;
+}
+
+static const struct dmi_system_id video_vendor_dmi_table[] = {
+	{
+		.callback = video_set_backlight_video_vendor,
+		.ident = "Acer Aspire 4736",
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "Acer"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 4736"),
+		},
+	},
+	{
+		.callback = video_set_backlight_video_vendor,
+		.ident = "Acer TravelMate 4750",
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "Acer"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TravelMate 4750"),
+		},
+	},
+	{}
+};
+
 /* Find which quirks are needed for a particular vendor/ model pair */
 static void find_quirks(void)
 {
@@ -1981,9 +2009,13 @@ static int __init acer_wmi_init(void)
 	set_quirks();
 
 	if (acpi_video_backlight_support()) {
-		interface->capability &= ~ACER_CAP_BRIGHTNESS;
-		pr_info("Brightness must be controlled by "
-		       "generic video driver\n");
+		if (dmi_check_system(video_vendor_dmi_table)) {
+			acpi_video_unregister();
+		} else {
+			interface->capability &= ~ACER_CAP_BRIGHTNESS;
+			pr_info("Brightness must be controlled by "
+				"acpi video driver\n");
+		}
 	}
 
 	if (wmi_has_guid(WMID_GUID3)) {
-- 
1.7.7




^ permalink raw reply related

* Re: Howto consolidate xgifb (staging) with sisfb
From: Thomas Winischhofer @ 2012-03-20  6:40 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <201203200002.40429.PeterHuewe@gmx.de>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Peter Hüwe wrote:
> Hi,
> 
> I'm currently trying to improve the xgifb driver from staging a bit and my 
> final goal is to probably merge this driver with the sis driver (as both card 
> families seem to have a lot in common).
> 
> However I'm a little bit unsure how to proceed here.
> 
> e.g. the sis and the xgifb driver have some nearly duplicated functions in 
> common, e.g.
> static void XGIfb_bpp_to_var(struct xgifb_video_info *xgifb_info, 			     
> struct fb_var_screeninfo *var)
> http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/staging/xgifb/XGI_main_26.c;hb=HEAD#l769
> 
> and 
> static void sisfb_bpp_to_var(struct sis_video_info *ivideo, struct 
> fb_var_screeninfo *var)
> http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/video/sis/sis_main.c;hb=HEAD#l1129
> 
> 
> have the same semantics. (the only difference is the _video_info struct).
> 
> -> Is there a way the xgifb driver can call the function from sis?  (I know 
> it's static and not EXPORT_SYMBOL'ed)
> Or how would you prepare the consolidation here ? 
> (e.g. make the functions identical? and when the merge get's done sometime 
> simply remove the duplicate?)
> 
> 
> 
> And how would you proceed with functions that are nearly identical (but each 
> function is missing some bits from the other card)
> e.g. 
> static int sisfb_get_fix(struct fb_fix_screeninfo *fix, int con, struct 
> fb_info *info)
> http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/video/sis/sis_main.c;hb=HEAD#l1839
> and 
> static int XGIfb_get_fix(struct fb_fix_screeninfo *fix, int con,  struct 
> fb_info *info)
> http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/staging/xgifb/XGI_main_26.c;hb=HEAD#l1365
> could be easily merged, but in which direction? Add the new code to directly 
> to sis or 'backport' sis stuff to xgifb? or both (i.e. make them identical 
> like in the first case?)
> 
> 
> 
> Thanks,
> Peter
> 
> 

I seem to remember that sisfb supported most of the XGI cards
(V3XT/V5/V8, Z7) as well.... What can xgifb do that sisfb can't? Merging
the blobs to support the Z9 and Z11 IMHO doesn't justify a separate driver.

Regards
Thomas

- --
Thomas Winischhofer
thomas AT winischhofer DOT net

The Online Photo Editor  ***************** http://www.phixr.com

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)

iD8DBQFPaCZMzydIRAktyUcRArf4AJ9Sp16HV7j5BmoJnV1GtRIwWKnayACePBF3
1yNMXuxDnpL77ov0EwLs5m0=EMek
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: Problem with framebuffer mmap on platforms with large addressing
From: Benjamin Herrenschmidt @ 2012-03-20  5:40 UTC (permalink / raw)
  To: Dmitry Eremin-Solenikov
  Cc: linux-fbdev, Florian Tobias Schandinat, Tony Breeds, linuxppc-dev
In-Reply-To: <CALT56yPjkPQOaXW4Eg8c-S4+a4Omou_YYacqrsCoTAYeXO0XkA@mail.gmail.com>


> >> That is interesting! Are those patches published or otherwise available
> >> somewhere? We are also very interested in enabling Canyonlands
> >> with Radeon KMS!
> >
> > You will run into additional problems with 460 due to the fact that it's
> > not cache coherent for DMA. Tony patches don't address that part of the
> > problem (they were used on a 476 based platform).
> 
> Hmm. Could you please spill a little bit more of details? Also are those patches
> for 476 merged or present somewhere?

Well, DMA on 46x isn't cache coherent. The DRM plays interesting games
with mapping/unmapping pages for DMA by the chip and I don't think we
have the right hooks to do the appropriate cache flushing on these guys,
but Tony might be able to comment, I don't know whether he tried or not.

On the other hand 476 has fully cache coherent DMA so the only problem
there is the >32-bit physical address space.

As for the patches, you'll have to wait for Tony to respond (I'll poke
him locally).

Cheers,
Ben.

> >> > In fact, we could make the new structure such that it doesn't break
> >> > userspace compatibility with 64-bit architectures at all, ie, the "new"
> >> > and "compat" ioctl could remain entirely equivalent on 64-bit.
> >>
> >> I remember stuff about compat_ioctl, but I have never used/implemented
> >> that. Are there any details of requirements for the structures being passed?
> >
> > In that specific case, I meant something else. IE. The old ioctl could
> > remain unchanged, and the new ioctl make the same as the old one on
> > 64-bit platforms.
> 
> I don't think this kind of magic would be good. I'd just stick to the new
> ioctl.
> 
> >
> > Cheers,
> > Ben.
> >
> >
> >
> 
> 
> 



^ permalink raw reply

* Re: [PATCH] Added backlight driver for Acer Aspire 4736
From: joeyli @ 2012-03-20  3:55 UTC (permalink / raw)
  To: Pradeep Subrahmanion
  Cc: Matthew Garrett, rpurdie, FlorianSchandinat, akpm, linux-fbdev,
	linux-kernel
In-Reply-To: <1332156793.2964.3.camel@debian.Gayathri>

Hi Pradeep, 

於 一,2012-03-19 於 17:03 +0530,Pradeep Subrahmanion 提到:
> > For the maximum level causes blank screen...
> > 
> > Please kindly help to identify which driver handle the brightness
> > change, run the following 2 commands:
> > 
> > # echo 5 > /sys/class/backlight/acer-wmi/brightness 
> > # echo 5 > /sys/class/backlight/intel_backlight/brightness 
> > 
> > Which one works to change brightness on your machine?
> 
> 	Both  of them do not work for me.Only hot keys are working. 
> 
> Writing to /sys/class/backlight/acer-wmi/brightness 
> 
> and /sys/class/backlight/intel_backlight/brightness do not cause any 
> 
> change in brightness.
> 
> 
> Regards , 
> 
> Pradeep Subrahmanion

A bit strange...

Could you please paste your /proc/cmdline ? Did you add
acpi_osi="Linux" ?


Thanks a lot!
Joey Lee


^ permalink raw reply

* Re: [PATCH] fbdev: sh_mipi_dsi: add extra phyctrl for sh_mipi_dsi_info
From: Florian Tobias Schandinat @ 2012-03-20  0:37 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <87y5rdnmmu.wl%kuninori.morimoto.gx@renesas.com>

Hi Morimoto-san,

On 03/07/2012 02:57 AM, Kuninori Morimoto wrote:
> sh_mipi uses some clocks, but the method of setup depends on CPU.
> 
> Current SuperH (like sh73a0) can control all of these clocks
> by CPG (Clock Pulse Generator).
> It means we can control it by clock framework only.
> But on sh7372, it needs CPG settings AND sh_mipi PHYCTRL::PLLDS,
> and only sh7372 has PHYCTRL::PLLDS.
> 
> But on current sh_mipi driver, PHYCTRL::PLLDS of sh7372 was
> overwrote since the callback timing of clock setting was changed
> by c2658b70f06108361aa5024798f9c1bf47c73374
> (fbdev: sh_mipi_dsi: fixup setup timing of sh_mipi_setup()).
> 
> The difference of PHYCTRL between current SuperH (=sh73a0) and
> old SuperH (=sh7372) is not only PLLDS.
> So this patch adds extra .phyctrl.
> 
> It also adds detail explanation for unclear mipi settings for ap4evb.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  arch/arm/mach-shmobile/board-ap4evb.c |   12 +++++++++---
>  drivers/video/sh_mipi_dsi.c           |    2 +-
>  include/video/sh_mipi_dsi.h           |    1 +
>  3 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
> index b4718b0..ca075aa 100644
> --- a/arch/arm/mach-shmobile/board-ap4evb.c
> +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> @@ -556,20 +556,25 @@ static struct platform_device keysc_device = {
>  };
>  
>  /* MIPI-DSI */
> -#define PHYCTRL		0x0070
>  static int sh_mipi_set_dot_clock(struct platform_device *pdev,
>  				 void __iomem *base,
>  				 int enable)
>  {
>  	struct clk *pck = clk_get(&pdev->dev, "dsip_clk");
> -	void __iomem *phy =  base + PHYCTRL;
>  
>  	if (IS_ERR(pck))
>  		return PTR_ERR(pck);
>  
>  	if (enable) {
> +		/*
> +		 * DSIPCLK	= 24MHz
> +		 * D-PHY	= DSIPCLK * ((0x6*2)+1) = 321MHz (see .phyctrl)

Shouldn't this be 312?

> +		 * HsByteCLK	= D-PHY/8 = 39MHz
> +		 *
> +		 *  X * Y * FPS > +		 * (544+72+600+16) * (961+8+8+2) * 30 = 36.1MHz
> +		 */
>  		clk_set_rate(pck, clk_round_rate(pck, 24000000));
> -		iowrite32(ioread32(phy) | (0xb << 8), phy);

Here you remove writing a 0xb<<8 (0xb = 11 = 8+2+1)...

>  		clk_enable(pck);
>  	} else {
>  		clk_disable(pck);
> @@ -598,6 +603,7 @@ static struct sh_mipi_dsi_info mipidsi0_info = {
>  	.lcd_chan	= &lcdc_info.ch[0],
>  	.lane		= 2,
>  	.vsynw_offset	= 17,
> +	.phyctrl	= 0x6 << 8,

...and here you set a 0x6<<8 (0x6 = 6 = 4+2). Looks suspicious, was this
change intended?

>  	.flags		= SH_MIPI_DSI_SYNC_PULSES_MODE |
>  			  SH_MIPI_DSI_HSbyteCLK,
>  	.set_dot_clock	= sh_mipi_set_dot_clock,
> diff --git a/drivers/video/sh_mipi_dsi.c b/drivers/video/sh_mipi_dsi.c
> index 05151b8..214482c 100644
> --- a/drivers/video/sh_mipi_dsi.c
> +++ b/drivers/video/sh_mipi_dsi.c
> @@ -271,7 +271,7 @@ static int __init sh_mipi_setup(struct sh_mipi *mipi,
>  	iowrite32(0x00000001, base + PHYCTRL);
>  	udelay(200);
>  	/* Deassert resets, power on */
> -	iowrite32(0x03070001, base + PHYCTRL);
> +	iowrite32(0x03070001 | pdata->phyctrl, base + PHYCTRL);
>  
>  	/*
>  	 * Default = ULPS enable |
> diff --git a/include/video/sh_mipi_dsi.h b/include/video/sh_mipi_dsi.h
> index 434d56b..06c67fb 100644
> --- a/include/video/sh_mipi_dsi.h
> +++ b/include/video/sh_mipi_dsi.h
> @@ -51,6 +51,7 @@ struct sh_mipi_dsi_info {
>  	int				lane;
>  	unsigned long			flags;
>  	u32				clksrc;
> +	u32				phyctrl; /* for extra setting */
>  	unsigned int			vsynw_offset;
>  	int	(*set_dot_clock)(struct platform_device *pdev,
>  				 void __iomem *base,

Thanks,

Florian Tobias Schandinat

^ permalink raw reply

* Re: [PATCH v3] fbdev: remove dependency of FB_SH_MOBILE_MERAM from FB_SH_MOBILE_LCDC
From: Florian Tobias Schandinat @ 2012-03-20  0:21 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <87d38qwmi9.wl%kuninori.morimoto.gx@renesas.com>

Hi Morimoto-san,

sorry for taking so long to reply.

On 03/06/2012 01:23 AM, Kuninori Morimoto wrote:
> MERAM can be used for other IP blocks as well in the future.
> It doesn't necessarily mean that the MERAM driver depends on the LCDC.
> This patch corrects dependency.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v2 -> v3
> 
> - move config FB_SH_MOBILE_MERAM out of "choice"

I am afraid but this is still not correct. The problem is that after
your patch this config option has no longer any dependency on the
framebuffer subsystem. As "FB" is a menuconfig option I guess the first
config option without any direct or indirect dependency on FB terminates
the menu. As I don't understand how this driver is used I don't know how
I should resolve this: either leave it where it is and just add an
dependency on "FB" or move it even further down, probably just before
endmenu (AFAIK the first thing that breaks it is OMAP2).


Best regards,

Florian Tobias Schandinat

> 
>  drivers/video/Kconfig |   23 +++++++++++------------
>  1 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 8951cbd..36e8b06 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2013,18 +2013,6 @@ config FB_SH_MOBILE_HDMI
>  	---help---
>  	  Driver for the on-chip SH-Mobile HDMI controller.
>  
> -config FB_SH_MOBILE_MERAM
> -	tristate "SuperH Mobile MERAM read ahead support for LCDC"
> -	depends on FB_SH_MOBILE_LCDC
> -	default y
> -	---help---
> -	  Enable MERAM support for the SH-Mobile LCD controller.
> -
> -	  This will allow for caching of the framebuffer to provide more
> -	  reliable access under heavy main memory bus traffic situations.
> -	  Up to 4 memory channels can be configured, allowing 4 RGB or
> -	  2 YCbCr framebuffers to be configured.
> -
>  config FB_TMIO
>  	tristate "Toshiba Mobile IO FrameBuffer support"
>  	depends on FB && MFD_CORE
> @@ -2332,6 +2320,17 @@ config FB_MB862XX_LIME
>  
>  endchoice
>  
> +config FB_SH_MOBILE_MERAM
> +	tristate "SuperH Mobile MERAM read ahead support"
> +	depends on (SUPERH || ARCH_SHMOBILE)
> +	---help---
> +	  Enable MERAM support for the SuperH controller.
> +
> +	  This will allow for caching of the framebuffer to provide more
> +	  reliable access under heavy main memory bus traffic situations.
> +	  Up to 4 memory channels can be configured, allowing 4 RGB or
> +	  2 YCbCr framebuffers to be configured.
> +
>  config FB_MB862XX_I2C
>  	bool "Support I2C bus on MB862XX GDC"
>  	depends on FB_MB862XX && I2C


^ permalink raw reply

* Re: Howto consolidate xgifb (staging) with sisfb
From: Greg KH @ 2012-03-19 23:26 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <201203200002.40429.PeterHuewe@gmx.de>

On Tue, Mar 20, 2012 at 12:02:39AM +0100, Peter Hüwe wrote:
> Hi,
> 
> I'm currently trying to improve the xgifb driver from staging a bit and my 
> final goal is to probably merge this driver with the sis driver (as both card 
> families seem to have a lot in common).
> 
> However I'm a little bit unsure how to proceed here.
> 
> e.g. the sis and the xgifb driver have some nearly duplicated functions in 
> common, e.g.
> static void XGIfb_bpp_to_var(struct xgifb_video_info *xgifb_info, 			     
> struct fb_var_screeninfo *var)
> http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/staging/xgifb/XGI_main_26.c;hb=HEAD#l769
> 
> and 
> static void sisfb_bpp_to_var(struct sis_video_info *ivideo, struct 
> fb_var_screeninfo *var)
> http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/video/sis/sis_main.c;hb=HEAD#l1129
> 
> 
> have the same semantics. (the only difference is the _video_info struct).
> 
> -> Is there a way the xgifb driver can call the function from sis?  (I know 
> it's static and not EXPORT_SYMBOL'ed)

Declare it not static and EXPORT_SYMBOL it :)

> Or how would you prepare the consolidation here ? 
> (e.g. make the functions identical? and when the merge get's done sometime 
> simply remove the duplicate?)

How about moving things to a "library" module that both modules call
with these functions, pulling them from one of the drivers.

> And how would you proceed with functions that are nearly identical (but each 
> function is missing some bits from the other card)
> e.g. 
> static int sisfb_get_fix(struct fb_fix_screeninfo *fix, int con, struct 
> fb_info *info)
> http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/video/sis/sis_main.c;hb=HEAD#l1839
> and 
> static int XGIfb_get_fix(struct fb_fix_screeninfo *fix, int con,  struct 
> fb_info *info)
> http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/staging/xgifb/XGI_main_26.c;hb=HEAD#l1365
> could be easily merged, but in which direction? Add the new code to directly 
> to sis or 'backport' sis stuff to xgifb? or both (i.e. make them identical 
> like in the first case?)

Whatever you feel is best to do here.

greg k-h

^ 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