Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH RESEND 1/7] video: s3c-fb: Add S5P64X0 specific
From: Ajay Kumar @ 2011-07-25 14:11 UTC (permalink / raw)
  To: linux-fbdev

This patch:
Adds s3c_fb_driverdata for S5P64X0, which supports 3 windows.
Also, register "s5p64x0-fb" type driver_data.
Existing s3c_fb_driverdata definitions in s3c-fb.c
supports 5 or 2 windows.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
acked-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/s3c-fb.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 4aecf21..0fda252 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -1859,6 +1859,30 @@ static struct s3c_fb_driverdata s3c_fb_data_s3c2443 = {
 	},
 };
 
+static struct s3c_fb_driverdata s3c_fb_data_s5p64x0 = {
+	.variant = {
+		.nr_windows	= 3,
+		.vidtcon	= VIDTCON0,
+		.wincon		= WINCON(0),
+		.winmap		= WINxMAP(0),
+		.keycon		= WKEYCON,
+		.osd		= VIDOSD_BASE,
+		.osd_stride	= 16,
+		.buf_start	= VIDW_BUF_START(0),
+		.buf_size	= VIDW_BUF_SIZE(0),
+		.buf_end	= VIDW_BUF_END(0),
+
+		.palette = {
+			[0] = 0x2400,
+			[1] = 0x2800,
+			[2] = 0x2c00,
+		},
+	},
+	.win[0] = &s3c_fb_data_s5p_wins[0],
+	.win[1] = &s3c_fb_data_s5p_wins[1],
+	.win[2] = &s3c_fb_data_s5p_wins[2],
+};
+
 static struct platform_device_id s3c_fb_driver_ids[] = {
 	{
 		.name		= "s3c-fb",
@@ -1872,6 +1896,9 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
 	}, {
 		.name		= "s3c2443-fb",
 		.driver_data	= (unsigned long)&s3c_fb_data_s3c2443,
+	}, {
+		.name		= "s5p64x0-fb",
+		.driver_data	= (unsigned long)&s3c_fb_data_s5p64x0,
 	},
 	{},
 };
-- 
1.7.0.4


^ permalink raw reply related

* RE: [PATCH v3] video: da8xx-fb: Increased resolution configuration
From: Manjunathappa, Prakash @ 2011-07-26  4:32 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1310963333-29806-1-git-send-email-prakash.pm@ti.com>

Hi Paul,

Could you please accept this patch as there are no comments?

Thanks,
Prakash
On Mon, Jul 18, 2011 at 09:58:53, Manjunathappa, Prakash wrote:
> Revised LCD controller in upcoming TI SoC which is an updated version of
> LCDC IP that was found on TI's DA850 SoC supports 2048*2048 resolution.
> Below are the encoding details:
> Width:
> Pixels Per Line = {pplmsb, ppllsb, 4'b1111} + 1
> Where pplmsb:1bit=>Raster Timing0[3], ppllsb:6bits=>Raster Timing0[9:4].
> And encoded value can range from 16 to 2048 in multiples of 16.
> 
> Height:
> Lines Per Panel = {lpp_b10, lpp}
> Where lpp:10bits=>Raster Timing1[9:0], lpp_b10:1bit=>Raster Timing2[26].
> And encoded value can range from 1 to 2048, programmable range is 0 to
> 2047.
> 
> Patch is verified on emulation platform of upcoming SoC for updated
> feature and on DA850 platform to make sure nothing existing breaks.
> 
> Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> ---
> Since v2:
> Corrected comment describing horizontal resolution bits and removed unnecessary
> outer parenthesis.
> Since v1:
> 1)Fixed the bug in configuration of lpp_b10 in Raster Timing2[26] register.
> 2)Reframed commit message.
> 
>  drivers/video/da8xx-fb.c |   31 ++++++++++++++++++++++++++++---
>  1 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 620f1c3..94b611a 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -460,18 +460,43 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
>  
>  	/* Set the Panel Width */
>  	/* Pixels per line = (PPL + 1)*16 */
> -	/*0x3F in bits 4..9 gives max horisontal resolution = 1024 pixels*/
> -	width &= 0x3f0;
> +	if (lcd_revision = LCD_VERSION_1) {
> +		/*
> +		 * 0x3F in bits 4..9 gives max horizontal resolution = 1024
> +		 * pixels.
> +		 */
> +		width &= 0x3f0;
> +	} else {
> +		/*
> +		 * 0x7F in bits 4..10 gives max horizontal resolution = 2048
> +		 * pixels.
> +		 */
> +		width &= 0x7f0;
> +	}
> +
>  	reg = lcdc_read(LCD_RASTER_TIMING_0_REG);
>  	reg &= 0xfffffc00;
> -	reg |= ((width >> 4) - 1) << 4;
> +	if (lcd_revision = LCD_VERSION_1) {
> +		reg |= ((width >> 4) - 1) << 4;
> +	} else {
> +		width = (width >> 4) - 1;
> +		reg |= ((width & 0x3f) << 4) | ((width & 0x40) >> 3);
> +	}
>  	lcdc_write(reg, LCD_RASTER_TIMING_0_REG);
>  
>  	/* Set the Panel Height */
> +	/* Set bits 9:0 of Lines Per Pixel */
>  	reg = lcdc_read(LCD_RASTER_TIMING_1_REG);
>  	reg = ((height - 1) & 0x3ff) | (reg & 0xfffffc00);
>  	lcdc_write(reg, LCD_RASTER_TIMING_1_REG);
>  
> +	/* Set bit 10 of Lines Per Pixel */
> +	if (lcd_revision = LCD_VERSION_2) {
> +		reg = lcdc_read(LCD_RASTER_TIMING_2_REG);
> +		reg |= ((height - 1) & 0x400) << 16;
> +		lcdc_write(reg, LCD_RASTER_TIMING_2_REG);
> +	}
> +
>  	/* Set the Raster Order of the Frame Buffer */
>  	reg = lcdc_read(LCD_RASTER_CTRL_REG) & ~(1 << 8);
>  	if (raster_order)
> -- 
> 1.7.1
> 
> 


^ permalink raw reply

* [PATCH RESEND] video: s3c-fb: Add support EXYNOS4 FIMD
From: Jingoo Han @ 2011-07-26  9:48 UTC (permalink / raw)
  To: linux-fbdev

This patch adds struct s3c_fb_driverdata s3c_fb_data_exynos4 for EXYNOS4
and adds lcd clock gating support.

FIMD driver needs two clocks for FIMD IP and LCD pixel clock. Previously,
both clocks are provided by using bus clock such as HCLK. However, EXYNOS4
can not select HCLK for LCD pixel clock because the EXYNOS4 FIMD IP does not
have the CLKSEL bit of VIDCON0. So, FIMD driver should provide the lcd clock
using SCLK_FIMD as LCD pixel clock for EXYNOS4.

The driver selects enabling lcd clock according to has_clksel which means
the CLKSEL bit of VIDCON0. If there is has_clksel, the driver will not
enable the lcd clock using SCLK_FIMD because bus clock using HCLK is used
a LCD pixel clock.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/Kconfig  |    2 +-
 drivers/video/s3c-fb.c |   88 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 549b960..963b8b7 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2027,7 +2027,7 @@ config FB_TMIO_ACCELL
 
 config FB_S3C
 	tristate "Samsung S3C framebuffer support"
-	depends on FB && S3C_DEV_FB
+	depends on FB && (S3C_DEV_FB || S5P_DEV_FIMD0)
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 4aecf21..cb0d3ea 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -81,6 +81,7 @@ struct s3c_fb;
  * @palette: Address of palette memory, or 0 if none.
  * @has_prtcon: Set if has PRTCON register.
  * @has_shadowcon: Set if has SHADOWCON register.
+ * @has_clksel: Set if VIDCON0 register has CLKSEL bit.
  */
 struct s3c_fb_variant {
 	unsigned int	is_2443:1;
@@ -98,6 +99,7 @@ struct s3c_fb_variant {
 
 	unsigned int	has_prtcon:1;
 	unsigned int	has_shadowcon:1;
+	unsigned int	has_clksel:1;
 };
 
 /**
@@ -186,6 +188,7 @@ struct s3c_fb_vsync {
  * @dev: The device that we bound to, for printing, etc.
  * @regs_res: The resource we claimed for the IO registers.
  * @bus_clk: The clk (hclk) feeding our interface and possibly pixclk.
+ * @lcd_clk: The clk (sclk) feeding pixclk.
  * @regs: The mapped hardware registers.
  * @variant: Variant information for this hardware.
  * @enabled: A bitmask of enabled hardware windows.
@@ -200,6 +203,7 @@ struct s3c_fb {
 	struct device		*dev;
 	struct resource		*regs_res;
 	struct clk		*bus_clk;
+	struct clk		*lcd_clk;
 	void __iomem		*regs;
 	struct s3c_fb_variant	 variant;
 
@@ -336,10 +340,15 @@ static int s3c_fb_check_var(struct fb_var_screeninfo *var,
  */
 static int s3c_fb_calc_pixclk(struct s3c_fb *sfb, unsigned int pixclk)
 {
-	unsigned long clk = clk_get_rate(sfb->bus_clk);
+	unsigned long clk;
 	unsigned long long tmp;
 	unsigned int result;
 
+	if (sfb->variant.has_clksel)
+		clk = clk_get_rate(sfb->bus_clk);
+	else
+		clk = clk_get_rate(sfb->lcd_clk);
+
 	tmp = (unsigned long long)clk;
 	tmp *= pixclk;
 
@@ -1354,13 +1363,24 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
 
 	clk_enable(sfb->bus_clk);
 
+	if (!sfb->variant.has_clksel) {
+		sfb->lcd_clk = clk_get(dev, "sclk_fimd");
+		if (IS_ERR(sfb->lcd_clk)) {
+			dev_err(dev, "failed to get lcd clock\n");
+			ret = PTR_ERR(sfb->lcd_clk);
+			goto err_bus_clk;
+		}
+
+		clk_enable(sfb->lcd_clk);
+	}
+
 	pm_runtime_enable(sfb->dev);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(dev, "failed to find registers\n");
 		ret = -ENOENT;
-		goto err_clk;
+		goto err_lcd_clk;
 	}
 
 	sfb->regs_res = request_mem_region(res->start, resource_size(res),
@@ -1368,7 +1388,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
 	if (!sfb->regs_res) {
 		dev_err(dev, "failed to claim register region\n");
 		ret = -ENOENT;
-		goto err_clk;
+		goto err_lcd_clk;
 	}
 
 	sfb->regs = ioremap(res->start, resource_size(res));
@@ -1450,7 +1470,13 @@ err_ioremap:
 err_req_region:
 	release_mem_region(sfb->regs_res->start, resource_size(sfb->regs_res));
 
-err_clk:
+err_lcd_clk:
+	if (!sfb->variant.has_clksel) {
+		clk_disable(sfb->lcd_clk);
+		clk_put(sfb->lcd_clk);
+	}
+
+err_bus_clk:
 	clk_disable(sfb->bus_clk);
 	clk_put(sfb->bus_clk);
 
@@ -1481,6 +1507,11 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
 
 	iounmap(sfb->regs);
 
+	if (!sfb->variant.has_clksel) {
+		clk_disable(sfb->lcd_clk);
+		clk_put(sfb->lcd_clk);
+	}
+
 	clk_disable(sfb->bus_clk);
 	clk_put(sfb->bus_clk);
 
@@ -1510,6 +1541,9 @@ static int s3c_fb_suspend(struct device *dev)
 		s3c_fb_blank(FB_BLANK_POWERDOWN, win->fbinfo);
 	}
 
+	if (!sfb->variant.has_clksel)
+		clk_disable(sfb->lcd_clk);
+
 	clk_disable(sfb->bus_clk);
 	return 0;
 }
@@ -1524,6 +1558,9 @@ static int s3c_fb_resume(struct device *dev)
 
 	clk_enable(sfb->bus_clk);
 
+	if (!sfb->variant.has_clksel)
+		clk_enable(sfb->lcd_clk);
+
 	/* setup gpio and output polarity controls */
 	pd->setup_gpio();
 	writel(pd->vidcon1, sfb->regs + VIDCON1);
@@ -1569,6 +1606,9 @@ static int s3c_fb_runtime_suspend(struct device *dev)
 		s3c_fb_blank(FB_BLANK_POWERDOWN, win->fbinfo);
 	}
 
+	if (!sfb->variant.has_clksel)
+		clk_disable(sfb->lcd_clk);
+
 	clk_disable(sfb->bus_clk);
 	return 0;
 }
@@ -1583,6 +1623,9 @@ static int s3c_fb_runtime_resume(struct device *dev)
 
 	clk_enable(sfb->bus_clk);
 
+	if (!sfb->variant.has_clksel)
+		clk_enable(sfb->lcd_clk);
+
 	/* setup gpio and output polarity controls */
 	pd->setup_gpio();
 	writel(pd->vidcon1, sfb->regs + VIDCON1);
@@ -1755,6 +1798,7 @@ static struct s3c_fb_driverdata s3c_fb_data_64xx = {
 		},
 
 		.has_prtcon	= 1,
+		.has_clksel	= 1,
 	},
 	.win[0]	= &s3c_fb_data_64xx_wins[0],
 	.win[1]	= &s3c_fb_data_64xx_wins[1],
@@ -1785,6 +1829,7 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pc100 = {
 		},
 
 		.has_prtcon	= 1,
+		.has_clksel	= 1,
 	},
 	.win[0]	= &s3c_fb_data_s5p_wins[0],
 	.win[1]	= &s3c_fb_data_s5p_wins[1],
@@ -1815,6 +1860,37 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pv210 = {
 		},
 
 		.has_shadowcon	= 1,
+		.has_clksel	= 1,
+	},
+	.win[0]	= &s3c_fb_data_s5p_wins[0],
+	.win[1]	= &s3c_fb_data_s5p_wins[1],
+	.win[2]	= &s3c_fb_data_s5p_wins[2],
+	.win[3]	= &s3c_fb_data_s5p_wins[3],
+	.win[4]	= &s3c_fb_data_s5p_wins[4],
+};
+
+static struct s3c_fb_driverdata s3c_fb_data_exynos4 = {
+	.variant = {
+		.nr_windows	= 5,
+		.vidtcon	= VIDTCON0,
+		.wincon		= WINCON(0),
+		.winmap		= WINxMAP(0),
+		.keycon		= WKEYCON,
+		.osd		= VIDOSD_BASE,
+		.osd_stride	= 16,
+		.buf_start	= VIDW_BUF_START(0),
+		.buf_size	= VIDW_BUF_SIZE(0),
+		.buf_end	= VIDW_BUF_END(0),
+
+		.palette = {
+			[0] = 0x2400,
+			[1] = 0x2800,
+			[2] = 0x2c00,
+			[3] = 0x3000,
+			[4] = 0x3400,
+		},
+
+		.has_shadowcon	= 1,
 	},
 	.win[0]	= &s3c_fb_data_s5p_wins[0],
 	.win[1]	= &s3c_fb_data_s5p_wins[1],
@@ -1843,6 +1919,7 @@ static struct s3c_fb_driverdata s3c_fb_data_s3c2443 = {
 			[0] = 0x400,
 			[1] = 0x800,
 		},
+		.has_clksel	= 1,
 	},
 	.win[0] = &(struct s3c_fb_win_variant) {
 		.palette_sz	= 256,
@@ -1870,6 +1947,9 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
 		.name		= "s5pv210-fb",
 		.driver_data	= (unsigned long)&s3c_fb_data_s5pv210,
 	}, {
+		.name		= "exynos4-fb",
+		.driver_data	= (unsigned long)&s3c_fb_data_exynos4,
+	}, {
 		.name		= "s3c2443-fb",
 		.driver_data	= (unsigned long)&s3c_fb_data_s3c2443,
 	},
-- 
1.7.1


^ permalink raw reply related

* [PATCH v2] backlight/platform_lcd: change set power function parameter
From: Donggeun Kim @ 2011-07-27  9:12 UTC (permalink / raw)
  To: linux-fbdev
  Cc: rpurdie, lethal, linux-kernel, kyungmin.park, dh09.lee, dg77.kim

The set_power function in platdata for platform_lcd driver
has originally two parameters.
The first parameter type is struct plat_lcd_data * and
the second parameter type is unsigned int .

In some cases, it is needed to get pointer typed 'struct device'
in order to call regulator_* functions.
This patch allows users to define a set_power function
which control regulators related to platform_lcd device.

Currently, variables in struct plat_lcd_data are not used
in several set_power functions which are defined at arch/arm/ directory.

Changes since v1 :
* Remove compile warning message by including another heeder file

Signed-off-by: Donggeun Kim <dg77.kim@samsung.com>
Signed-off-by: Donghwa Lee <dh09.lee@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
 drivers/video/backlight/platform_lcd.c |    2 +-
 include/video/platform_lcd.h           |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/platform_lcd.c b/drivers/video/backlight/platform_lcd.c
index 302330a..cf23a3f 100644
--- a/drivers/video/backlight/platform_lcd.c
+++ b/drivers/video/backlight/platform_lcd.c
@@ -49,7 +49,7 @@ static int platform_lcd_set_power(struct lcd_device *lcd, int power)
 	if (power = FB_BLANK_POWERDOWN || plcd->suspended)
 		lcd_power = 0;
 
-	plcd->pdata->set_power(plcd->pdata, lcd_power);
+	plcd->pdata->set_power(lcd, lcd_power);
 	plcd->power = power;
 
 	return 0;
diff --git a/include/video/platform_lcd.h b/include/video/platform_lcd.h
index ad3bdfe..5d7e943 100644
--- a/include/video/platform_lcd.h
+++ b/include/video/platform_lcd.h
@@ -10,12 +10,13 @@
  * published by the Free Software Foundation.
  *
 */
+#include <linux/lcd.h>
 
 struct plat_lcd_data;
 struct fb_info;
 
 struct plat_lcd_data {
-	void	(*set_power)(struct plat_lcd_data *, unsigned int power);
+	void	(*set_power)(struct lcd_device *, unsigned int power);
 	int	(*match_fb)(struct plat_lcd_data *, struct fb_info *);
 };
 
-- 
1.7.4.1


^ permalink raw reply related

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
From: Guennadi Liakhovetski @ 2011-07-28  8:31 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Florian Tobias Schandinat, Geert Uytterhoeven, Paul Mundt,
	linux-fbdev, linux-media, dri-devel
In-Reply-To: <201107111732.52156.laurent.pinchart@ideasonboard.com>

On Mon, 11 Jul 2011, Laurent Pinchart wrote:

> On Friday 24 June 2011 21:45:57 Florian Tobias Schandinat wrote:
> > On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
> > > On Fri, Jun 24, 2011 at 08:19, Paul Mundt wrote:
> > >> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> > >>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat wrote:
> > >>>> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> > >>>>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> > >>>>>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes
> > >>>>>> must be non-zero), I don't think there are any conflicts with
> > >>>>>> existing values of
> > >>>>>> nonstd. To make it even safer and easier to parse, you could set bit
> > >>>>>> 31 of
> > >>>>>> nonstd as a FOURCC indicator.
> > >>>>> 
> > >>>>> I would then create a union between nonstd and fourcc, and document
> > >>>>> nonstd as
> > >>>>> being used for the legacy API only. Most existing drivers use a
> > >>>>> couple of nonstd bits only. The driver that (ab)uses nonstd the most
> > >>>>> is pxafb and uses
> > >>>>> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
> > >>>>> 0xff000000 != 0 could be used as a FOURCC mode test.
> > >>>>> 
> > >>>>> This assumes that FOURCCs will never have their last character set to
> > >>>>> '\0'. Is
> > >>>>> that a safe assumption for the future ?
> > >>>> 
> > >>>> Yes, I think. The information I found indicates that space should be
> > >>>> used for padding, so a \0 shouldn't exist.
> > >>>> I think using only the nonstd field and requiring applications to
> > >>>> check the capabilities would be possible, although not fool proof ;)
> > >>> 
> > >>> So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC
> > >>> if any of them is set.
> > >>> 
> > >>> Nicely backwards compatible, as sane drivers should reject nonstd
> > >>> values they don't support (apps _will_ start filling in FOURCC values
> > >>> ignoring capabilities, won't they?).
> > >> 
> > >> That seems like a reasonable case, but if we're going to do that then
> > >> certainly the nonstd bit encoding needs to be documented and treated as
> > >> a hard ABI.
> > >> 
> > >> I'm not so sure about the if any bit in the upper byte is set assume
> > >> FOURCC case though, there will presumably be other users in the future
> > >> that will want bits for themselves, too. What exactly was the issue with
> > >> having a FOURCC capability bit in the upper byte?
> > > 
> > > That indeed gives less issues (as long as you don't stuff 8-bit ASCII
> > > in the MSB) and more bits for tradiditional nonstd users.
> > 
> > The only disadvantage I can see is that it requires adding this bit in the
> > application and stripping it when interpreting it. But I think the 24 lower
> > bits should be enough for driver specific behavior (as the current values
> > really can only be interpreted per driver).
> 
> I'm also not keen on adding/stripping the MSB. It would be easier for 
> applications to use FOURCCs directly.
> 
> > > BTW, after giving it some more thought: what does the FB_CAP_FOURCC buys
> > > us? It's not like all drivers will support whatever random FOURCC mode
> > > you give them, so you have to check whether it's supported by doing a
> > > set_var first.
> > 
> > Because any solution purely based on the nonstd field is insane. The abuse
> > of that field is just too widespread. Drivers that use nonstd usually only
> > check whether it is zero or nonzero and others will just interpret parts
> > of nonstd and ignore the rest. They will happily accept FOURCC values in
> > the nonstd and just doing the wrong thing. Even if we would fix those the
> > problems applications will run into with older kernels will remain.
> 
> I agree with Florian here. Many drivers currently check whether nonstd != 0. 
> Who knows what kind of values applications feed them ?

FWIW, I prefer the original Laurent's proposal, with a slight uncertainty 
about whether we need the .capability field, or whether the try-and-check 
approach is sufficient. As for struct fb_var_screeninfo fields to support 
switching to a FOURCC mode, I also prefer an explicit dedicated flag to 
specify switching to it. Even though using FOURCC doesn't fit under the 
notion of a videomode, using one of .vmode bits is too tempting, so, I 
would actually take the plunge and use FB_VMODE_FOURCC.

As for the actual location of the fourcc code, I would leave .nonstd 
alone: who knows, maybe drivers will need both, whereas using grayscale 
and fourcc certainly doesn't make any sense. And I personally don't see a 
problem with using a union: buggy applications are, well, buggy... 
Actually, since in FOURCC mode we don't need any of

	__u32 bits_per_pixel;		/* guess what			*/
	__u32 grayscale;		/* != 0 Graylevels instead of colors */

	struct fb_bitfield red;		/* bitfield in fb mem if true color, */
	struct fb_bitfield green;	/* else only length is significant */
	struct fb_bitfield blue;
	struct fb_bitfield transp;	/* transparency			*/	

so, we could put them all in the union for the case, if we need anything 
else for the fourcc configuration in the future.

> I'd like to reach an agreement on the API, and implement it. I'm fine with 
> either grayscale or nonstd to store the FOURCC (with a slight preference for 
> grayscale), and with either a vmode flag or using the most significant byte of 
> the grayscale/nonstd field to detect FOURCC mode. I believe FB_CAP_FOURCC (or 
> something similar) is needed.

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

^ permalink raw reply

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
From: Laurent Pinchart @ 2011-07-28 10:51 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Florian Tobias Schandinat, Geert Uytterhoeven, Paul Mundt,
	linux-fbdev, linux-media, dri-devel
In-Reply-To: <Pine.LNX.4.64.1107280943470.20737@axis700.grange>

Hi Guennadi,

Thanks for the review.

On Thursday 28 July 2011 10:31:24 Guennadi Liakhovetski wrote:
> On Mon, 11 Jul 2011, Laurent Pinchart wrote:
> > On Friday 24 June 2011 21:45:57 Florian Tobias Schandinat wrote:
> > > On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
> > > > On Fri, Jun 24, 2011 at 08:19, Paul Mundt wrote:
> > > >> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> > > >>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat wrote:
> > > >>>> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> > > >>>>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> > > >>>>>> As FOURCC values are always 4 ASCII characters (hence all 4
> > > >>>>>> bytes must be non-zero), I don't think there are any conflicts
> > > >>>>>> with existing values of
> > > >>>>>> nonstd. To make it even safer and easier to parse, you could set
> > > >>>>>> bit 31 of
> > > >>>>>> nonstd as a FOURCC indicator.
> > > >>>>> 
> > > >>>>> I would then create a union between nonstd and fourcc, and
> > > >>>>> document nonstd as
> > > >>>>> being used for the legacy API only. Most existing drivers use a
> > > >>>>> couple of nonstd bits only. The driver that (ab)uses nonstd the
> > > >>>>> most is pxafb and uses
> > > >>>>> bits 22:0. Bits 31:24 are never used as far as I can tell, so
> > > >>>>> nonstd& 0xff000000 != 0 could be used as a FOURCC mode test.
> > > >>>>> 
> > > >>>>> This assumes that FOURCCs will never have their last character
> > > >>>>> set to '\0'. Is
> > > >>>>> that a safe assumption for the future ?
> > > >>>> 
> > > >>>> Yes, I think. The information I found indicates that space should
> > > >>>> be used for padding, so a \0 shouldn't exist.
> > > >>>> I think using only the nonstd field and requiring applications to
> > > >>>> check the capabilities would be possible, although not fool proof
> > > >>>> ;)
> > > >>> 
> > > >>> So we can declare the 8 msb bits of nonstd reserved, and assume
> > > >>> FOURCC if any of them is set.
> > > >>> 
> > > >>> Nicely backwards compatible, as sane drivers should reject nonstd
> > > >>> values they don't support (apps _will_ start filling in FOURCC
> > > >>> values ignoring capabilities, won't they?).
> > > >> 
> > > >> That seems like a reasonable case, but if we're going to do that
> > > >> then certainly the nonstd bit encoding needs to be documented and
> > > >> treated as a hard ABI.
> > > >> 
> > > >> I'm not so sure about the if any bit in the upper byte is set assume
> > > >> FOURCC case though, there will presumably be other users in the
> > > >> future that will want bits for themselves, too. What exactly was
> > > >> the issue with having a FOURCC capability bit in the upper byte?
> > > > 
> > > > That indeed gives less issues (as long as you don't stuff 8-bit ASCII
> > > > in the MSB) and more bits for tradiditional nonstd users.
> > > 
> > > The only disadvantage I can see is that it requires adding this bit in
> > > the application and stripping it when interpreting it. But I think the
> > > 24 lower bits should be enough for driver specific behavior (as the
> > > current values really can only be interpreted per driver).
> > 
> > I'm also not keen on adding/stripping the MSB. It would be easier for
> > applications to use FOURCCs directly.
> > 
> > > > BTW, after giving it some more thought: what does the FB_CAP_FOURCC
> > > > buys us? It's not like all drivers will support whatever random
> > > > FOURCC mode you give them, so you have to check whether it's
> > > > supported by doing a set_var first.
> > > 
> > > Because any solution purely based on the nonstd field is insane. The
> > > abuse of that field is just too widespread. Drivers that use nonstd
> > > usually only check whether it is zero or nonzero and others will just
> > > interpret parts of nonstd and ignore the rest. They will happily
> > > accept FOURCC values in the nonstd and just doing the wrong thing.
> > > Even if we would fix those the problems applications will run into
> > > with older kernels will remain.
> > 
> > I agree with Florian here. Many drivers currently check whether nonstd !> > 0. Who knows what kind of values applications feed them ?
> 
> FWIW, I prefer the original Laurent's proposal, with a slight uncertainty
> about whether we need the .capability field, or whether the try-and-check
> approach is sufficient.

Try-and-check would be better, but unfortunately I don't think it would work. 
Existing drivers will either ignore the new FB_VMODE_FOURCC flag or fail. I 
expect many drivers that ignore it to still accept it blindly, and not clear 
it, so applications won't be able to find out whether the flag is supported.

> As for struct fb_var_screeninfo fields to support switching to a FOURCC
> mode, I also prefer an explicit dedicated flag to specify switching to it.
> Even though using FOURCC doesn't fit under the notion of a videomode, using
> one of .vmode bits is too tempting, so, I would actually take the plunge and
> use FB_VMODE_FOURCC.

Another option would be to consider any grayscale > 1 value as a FOURCC. I've 
briefly checked the in-tree drivers: they only assign grayscale with 0 or 1, 
and check whether grayscale is 0 or different than 0. If a userspace 
application only sets grayscale > 1 when talking to a driver that supports the 
FOURCC-based API, we could get rid of the flag.

What can't be easily found out is whether existing applications set grayscale 
to a > 1 value. They would break when used with FOURCC-aware drivers if we 
consider any grayscale > 1 value as a FOURCC. Is that a risk we can take ?

> As for the actual location of the fourcc code, I would leave .nonstd
> alone: who knows, maybe drivers will need both, whereas using grayscale
> and fourcc certainly doesn't make any sense. And I personally don't see a
> problem with using a union: buggy applications are, well, buggy...

I agree with this.

> Actually, since in FOURCC mode we don't need any of
> 
> 	__u32 bits_per_pixel;		/* guess what			*/
> 	__u32 grayscale;		/* != 0 Graylevels instead of colors */
> 
> 	struct fb_bitfield red;		/* bitfield in fb mem if true color, */
> 	struct fb_bitfield green;	/* else only length is significant */
> 	struct fb_bitfield blue;
> 	struct fb_bitfield transp;	/* transparency			*/
> 
> so, we could put them all in the union for the case, if we need anything
> else for the fourcc configuration in the future.

Good point. It might make sense to exclude bits_per_pixel from the union 
though, as that's interesting information for applications. The red, green, 
blue and transp fields are less useful.

> > I'd like to reach an agreement on the API, and implement it. I'm fine
> > with either grayscale or nonstd to store the FOURCC (with a slight
> > preference for grayscale), and with either a vmode flag or using the
> > most significant byte of the grayscale/nonstd field to detect FOURCC
> > mode. I believe FB_CAP_FOURCC (or something similar) is needed.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [PATCH] fb: avoid possible deadlock caused by fb_set_suspend
From: Francis Moreau @ 2011-07-28 22:10 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: lethal, linux-fbdev, torvalds, bonbons, linux-kernel,
	Herton Ronaldo Krzesinski, stable
In-Reply-To: <4E271B72.3070909@gmx.de>

On Wed, Jul 20, 2011 at 8:16 PM, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> what are your plans for the patch below? Do you queue it for 3.1?

Looks like someone else fixed it slightly differently:

http://permalink.gmane.org/gmane.linux.kernel.stable/15187

-- 
Francis

^ permalink raw reply

* Re: [Bugme-new] [Bug 39842] New: savagefb.h CARD SERIES definition
From: Andrew Morton @ 2011-07-28 23:27 UTC (permalink / raw)
  To: linux-fbdev


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Sat, 23 Jul 2011 10:20:41 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id9842
> 
>            Summary: savagefb.h CARD SERIES definition typo
>            Product: Drivers
>            Version: 2.5
>     Kernel Version: 3.0.0
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Video(Other)
>         AssignedTo: drivers_video-other@kernel-bugs.osdl.org
>         ReportedBy: jpsinthemix@verizon.net
>         Regression: Yes
> 
> 
> I have an old T22 Thinkpad with integrated Savage/IX-MV video. As of linux-3.0,
> the video is non-functional; the PC either hard hangs immediately or has
> totally garbled video, and then hard hangs, as soon as I attempt to edit a file
> or cat a file with more than a screen-full of text. The foregoing details are
> actually not important any more as in looking at the code changes I discovered
> a typo in linux-3.0.0/drivers/video/savage/savagefb.h and have applied the
> following patch to fix the issue:
> 
> --- linux-3.0.0.old/drivers/video/savage/savagefb.h     2011-07-21
> 22:17:23.000000000 -0400
> +++ linux-3.0.0.new/drivers/video/savage/savagefb.h     2011-07-23
> 05:13:33.801215293 -0400
> @@ -55,7 +55,7 @@
> 
>  #define S3_SAVAGE3D_SERIES(chip)  ((chip>=S3_SAVAGE3D) &&
> (chip<=S3_SAVAGE_MX))
> 
> -#define S3_SAVAGE4_SERIES(chip)   ((chip>=S3_SAVAGE4) ||
> (chip<=S3_PROSAVAGEDDR))
> +#define S3_SAVAGE4_SERIES(chip)   ((chip>=S3_SAVAGE4) &&
> (chip<=S3_PROSAVAGEDDR))
> 
>  #define S3_SAVAGE_MOBILE_SERIES(chip)  ((chip=S3_SAVAGE_MX) ||
> (chip=S3_SUPERSAVAGE))
> 
> 
> P.S. Many, many thanks for lifting the 800x600 resolution limit! I've been
> using a patch/hack to bump the limit to 1024x768 for an very long time...
> John

Please resend that fix as a formal signed-off-patch as a reply-to-all
to this email?  Documentation/SubmittingPatches has some tips.

Thanks.


^ permalink raw reply

* Re: [Bugme-new] [Bug 39842] New: savagefb.h CARD SERIES definition
From: John Stanley @ 2011-07-29 12:20 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20110728162752.3a4602e4.akpm@linux-foundation.org>

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

Replying to all.. I have also attached the one-line patch.
thanks,
John

On 07/28/2011 07:27 PM, Andrew Morton wrote:
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
>
> On Sat, 23 Jul 2011 10:20:41 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
>
>> https://bugzilla.kernel.org/show_bug.cgi?id=39842
>>
>>             Summary: savagefb.h CARD SERIES definition typo
>>             Product: Drivers
>>             Version: 2.5
>>      Kernel Version: 3.0.0
>>            Platform: All
>>          OS/Version: Linux
>>                Tree: Mainline
>>              Status: NEW
>>            Severity: normal
>>            Priority: P1
>>           Component: Video(Other)
>>          AssignedTo: drivers_video-other@kernel-bugs.osdl.org
>>          ReportedBy: jpsinthemix@verizon.net
>>          Regression: Yes
>>
>>
>> I have an old T22 Thinkpad with integrated Savage/IX-MV video. As of linux-3.0,
>> the video is non-functional; the PC either hard hangs immediately or has
>> totally garbled video, and then hard hangs, as soon as I attempt to edit a file
>> or cat a file with more than a screen-full of text. The foregoing details are
>> actually not important any more as in looking at the code changes I discovered
>> a typo in linux-3.0.0/drivers/video/savage/savagefb.h and have applied the
>> following patch to fix the issue:
>>
>> --- linux-3.0.0.old/drivers/video/savage/savagefb.h     2011-07-21
>> 22:17:23.000000000 -0400
>> +++ linux-3.0.0.new/drivers/video/savage/savagefb.h     2011-07-23
>> 05:13:33.801215293 -0400
>> @@ -55,7 +55,7 @@
>>
>>   #define S3_SAVAGE3D_SERIES(chip)  ((chip>=S3_SAVAGE3D)&&
>> (chip<=S3_SAVAGE_MX))
>>
>> -#define S3_SAVAGE4_SERIES(chip)   ((chip>=S3_SAVAGE4) ||
>> (chip<=S3_PROSAVAGEDDR))
>> +#define S3_SAVAGE4_SERIES(chip)   ((chip>=S3_SAVAGE4)&&
>> (chip<=S3_PROSAVAGEDDR))
>>
>>   #define S3_SAVAGE_MOBILE_SERIES(chip)  ((chip==S3_SAVAGE_MX) ||
>> (chip==S3_SUPERSAVAGE))
>>
>>
>> P.S. Many, many thanks for lifting the 800x600 resolution limit! I've been
>> using a patch/hack to bump the limit to 1024x768 for an very long time...
>> John
> Please resend that fix as a formal signed-off-patch as a reply-to-all
> to this email?  Documentation/SubmittingPatches has some tips.
>
> Thanks.
>
>


[-- Attachment #2: 01-linux-3.0.0-savagefb-card-series-fix.patch --]
[-- Type: text/plain, Size: 539 bytes --]

--- linux-3.0.0.old/drivers/video/savage/savagefb.h	2011-07-21 22:17:23.000000000 -0400
+++ linux-3.0.0.new/drivers/video/savage/savagefb.h	2011-07-23 05:13:33.801215293 -0400
@@ -55,7 +55,7 @@
 
 #define S3_SAVAGE3D_SERIES(chip)  ((chip>=S3_SAVAGE3D) && (chip<=S3_SAVAGE_MX))
 
-#define S3_SAVAGE4_SERIES(chip)   ((chip>=S3_SAVAGE4) || (chip<=S3_PROSAVAGEDDR))
+#define S3_SAVAGE4_SERIES(chip)   ((chip>=S3_SAVAGE4) && (chip<=S3_PROSAVAGEDDR))
 
 #define S3_SAVAGE_MOBILE_SERIES(chip)  ((chip==S3_SAVAGE_MX) || (chip==S3_SUPERSAVAGE))
 


^ permalink raw reply

* [PATCH 0/4] sh_mobile_meram cleanups and fixes
From: Laurent Pinchart @ 2011-07-30 20:26 UTC (permalink / raw)
  To: linux-fbdev

Hi everybody,

This patch series follows the sh_mobile_lcdc patches I've posted to the list.
As with the previous series, I've started with a readability patch that made
me find a bug. Patches 1/4, 2/4 and 4/4 are cleanups, and patch 3/4 fix the
bug. Please see individual patch descriptions for details.

Laurent Pinchart (4):
  fbdev: sh_mobile_meram: Replace hardcoded register values with macros
  fbdev: sh_mobile_meram: Validate ICB configuration outside mutex
  fbdev: sh_mobile_meram: Fix MExxCTL register save on runtime PM
    suspend
  fbdev: sh_mobile_meram: Remove unneeded sh_mobile_meram.h

 drivers/video/sh_mobile_lcdcfb.c |    2 +-
 drivers/video/sh_mobile_meram.c  |  122 ++++++++++++++++++++++++++------------
 drivers/video/sh_mobile_meram.h  |   33 ----------
 3 files changed, 84 insertions(+), 73 deletions(-)
 delete mode 100644 drivers/video/sh_mobile_meram.h

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* [PATCH 1/4] fbdev: sh_mobile_meram: Replace hardcoded register values with macros
From: Laurent Pinchart @ 2011-07-30 20:26 UTC (permalink / raw)
  To: linux-fbdev

Instead of hardcoding register values through the driver, define macros
for individual register bits using the register name and the bit name,
and use the macros.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_meram.c |   97 ++++++++++++++++++++++++++++----------
 1 files changed, 71 insertions(+), 26 deletions(-)

diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 39f28a1..85577cf 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -20,22 +20,67 @@
 #include "sh_mobile_meram.h"
 
 /* meram registers */
-#define MExxCTL 0x0
-#define MExxBSIZE 0x4
-#define MExxMNCF 0x8
-#define MExxSARA 0x10
-#define MExxSARB 0x14
-#define MExxSBSIZE 0x18
-
-#define MERAM_MExxCTL_VAL(ctl, next_icb, addr)	\
-	((ctl) | (((next_icb) & 0x1f) << 11) | (((addr) & 0x7ff) << 16))
-#define	MERAM_MExxBSIZE_VAL(a, b, c) \
-	(((a) << 28) | ((b) << 16) | (c))
-
-#define MEVCR1 0x4
-#define MEACTS 0x10
-#define MEQSEL1 0x40
-#define MEQSEL2 0x44
+#define MEVCR1			0x4
+#define MEVCR1_RST		(1 << 31)
+#define MEVCR1_WD		(1 << 30)
+#define MEVCR1_AMD1		(1 << 29)
+#define MEVCR1_AMD0		(1 << 28)
+#define MEQSEL1			0x40
+#define MEQSEL2			0x44
+
+#define MExxCTL			0x400
+#define MExxCTL_BV		(1 << 31)
+#define MExxCTL_BSZ_SHIFT	28
+#define MExxCTL_MSAR_MASK	(0x7ff << MExxCTL_MSAR_SHIFT)
+#define MExxCTL_MSAR_SHIFT	16
+#define MExxCTL_NXT_MASK	(0x1f << MExxCTL_NXT_SHIFT)
+#define MExxCTL_NXT_SHIFT	11
+#define MExxCTL_WD1		(1 << 10)
+#define MExxCTL_WD0		(1 << 9)
+#define MExxCTL_WS		(1 << 8)
+#define MExxCTL_CB		(1 << 7)
+#define MExxCTL_WBF		(1 << 6)
+#define MExxCTL_WF		(1 << 5)
+#define MExxCTL_RF		(1 << 4)
+#define MExxCTL_CM		(1 << 3)
+#define MExxCTL_MD_READ		(1 << 0)
+#define MExxCTL_MD_WRITE	(2 << 0)
+#define MExxCTL_MD_ICB_WB	(3 << 0)
+#define MExxCTL_MD_ICB		(4 << 0)
+#define MExxCTL_MD_FB		(7 << 0)
+#define MExxCTL_MD_MASK		(7 << 0)
+#define MExxBSIZE		0x404
+#define MExxBSIZE_RCNT_SHIFT	28
+#define MExxBSIZE_YSZM1_SHIFT	16
+#define MExxBSIZE_XSZM1_SHIFT	0
+#define MExxMNCF		0x408
+#define MExxMNCF_KWBNM_SHIFT	28
+#define MExxMNCF_KRBNM_SHIFT	24
+#define MExxMNCF_BNM_SHIFT	16
+#define MExxMNCF_XBV		(1 << 15)
+#define MExxMNCF_CPL_YCBCR444	(1 << 12)
+#define MExxMNCF_CPL_YCBCR420	(2 << 12)
+#define MExxMNCF_CPL_YCBCR422	(3 << 12)
+#define MExxMNCF_CPL_MSK	(3 << 12)
+#define MExxMNCF_BL		(1 << 2)
+#define MExxMNCF_LNM_SHIFT	0
+#define MExxSARA		0x410
+#define MExxSARB		0x414
+#define MExxSBSIZE		0x418
+#define MExxSBSIZE_HDV		(1 << 31)
+#define MExxSBSIZE_HSZ16	(0 << 28)
+#define MExxSBSIZE_HSZ32	(1 << 28)
+#define MExxSBSIZE_HSZ64	(2 << 28)
+#define MExxSBSIZE_HSZ128	(3 << 28)
+#define MExxSBSIZE_SBSIZZ_SHIFT	0
+
+#define MERAM_MExxCTL_VAL(next, addr)	\
+	((((next) << MExxCTL_NXT_SHIFT) & MExxCTL_NXT_MASK) | \
+	 (((addr) << MExxCTL_MSAR_SHIFT) & MExxCTL_MSAR_MASK))
+#define	MERAM_MExxBSIZE_VAL(rcnt, yszm1, xszm1) \
+	(((rcnt) << MExxBSIZE_RCNT_SHIFT) | \
+	 ((yszm1) << MExxBSIZE_YSZM1_SHIFT) | \
+	 ((xszm1) << MExxBSIZE_XSZM1_SHIFT))
 
 static unsigned long common_regs[] = {
 	MEVCR1,
@@ -72,8 +117,7 @@ struct sh_mobile_meram_priv {
  * MERAM/ICB access functions
  */
 
-#define MERAM_ICB_OFFSET(base, idx, off)	\
-	((base) + (0x400 + ((idx) * 0x20) + (off)))
+#define MERAM_ICB_OFFSET(base, idx, off)	((base) + (off) + (idx) * 0x20)
 
 static inline void meram_write_icb(void __iomem *base, int idx, int off,
 	unsigned long val)
@@ -308,17 +352,18 @@ static int meram_init(struct sh_mobile_meram_priv *priv,
 	/*
 	 * Set MERAM for framebuffer
 	 *
-	 * 0x70f:  WD = 0x3, WS=0x1, CM=0x1, MDû mode
 	 * we also chain the cache_icb and the marker_icb.
 	 * we also split the allocated MERAM buffer between two ICBs.
 	 */
 	meram_write_icb(priv->base, icb->cache_icb, MExxCTL,
-			MERAM_MExxCTL_VAL(0x70f, icb->marker_icb,
-					  icb->meram_offset));
+			MERAM_MExxCTL_VAL(icb->marker_icb, icb->meram_offset) |
+			MExxCTL_WD1 | MExxCTL_WD0 | MExxCTL_WS | MExxCTL_CM |
+			MExxCTL_MD_FB);
 	meram_write_icb(priv->base, icb->marker_icb, MExxCTL,
-			MERAM_MExxCTL_VAL(0x70f, icb->cache_icb,
-					  icb->meram_offset +
-					  icb->meram_size / 2));
+			MERAM_MExxCTL_VAL(icb->cache_icb, icb->meram_offset +
+					  icb->meram_size / 2) |
+			MExxCTL_WD1 | MExxCTL_WD0 | MExxCTL_WS | MExxCTL_CM |
+			MExxCTL_MD_FB);
 
 	return 0;
 }
@@ -507,7 +552,7 @@ static int sh_mobile_meram_runtime_suspend(struct device *dev)
 			/* Reset ICB on resume */
 			if (icb_regs[k] = MExxCTL)
 				priv->icb_saved_regs[j * ICB_REGS_SIZE + k] -					0x70;
+					MExxCTL_WBF | MExxCTL_WF | MExxCTL_RF;
 		}
 	}
 	return 0;
@@ -592,7 +637,7 @@ static int __devinit sh_mobile_meram_probe(struct platform_device *pdev)
 
 	/* initialize ICB addressing mode */
 	if (pdata->addr_mode = SH_MOBILE_MERAM_MODE1)
-		meram_write_reg(priv->base, MEVCR1, 1 << 29);
+		meram_write_reg(priv->base, MEVCR1, MEVCR1_AMD1);
 
 	pm_runtime_enable(&pdev->dev);
 
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH 2/4] fbdev: sh_mobile_meram: Validate ICB configuration outside mutex
From: Laurent Pinchart @ 2011-07-30 20:26 UTC (permalink / raw)
  To: linux-fbdev

Validate as much of the requested ICB configuration as possible outside
of the mutex-protected region when registering ICBs.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_meram.c |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 85577cf..25a7118 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -410,24 +410,22 @@ static int sh_mobile_meram_register(struct sh_mobile_meram_info *pdata,
 		xres, yres, (!pixelformat) ? "yuv" : "rgb",
 		base_addr_y, base_addr_c);
 
-	mutex_lock(&priv->lock);
-
 	/* we can't handle wider than 8192px */
 	if (xres > 8192) {
 		dev_err(&pdev->dev, "width exceeding the limit (> 8192).");
-		error = -EINVAL;
-		goto err;
-	}
-
-	if (priv->used_meram_cache_regions + 2 > SH_MOBILE_MERAM_ICB_NUM) {
-		dev_err(&pdev->dev, "no more ICB available.");
-		error = -EINVAL;
-		goto err;
+		return -EINVAL;
 	}
 
 	/* do we have at least one ICB config? */
 	if (cfg->icb[0].marker_icb < 0 || cfg->icb[0].cache_icb < 0) {
 		dev_err(&pdev->dev, "at least one ICB is required.");
+		return -EINVAL;
+	}
+
+	mutex_lock(&priv->lock);
+
+	if (priv->used_meram_cache_regions + 2 > SH_MOBILE_MERAM_ICB_NUM) {
+		dev_err(&pdev->dev, "no more ICB available.");
 		error = -EINVAL;
 		goto err;
 	}
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH 3/4] fbdev: sh_mobile_meram: Fix MExxCTL register save on runtime PM suspend
From: Laurent Pinchart @ 2011-07-30 20:26 UTC (permalink / raw)
  To: linux-fbdev

To reset the ICB on resume the MExxCTL register needs to be OR'ed with
MExxCTL_WBF | MExxCTL_WF | MExxCTL_RF, no set to that value. Fix this.

This fixes corruption at the bottom of the display when resuming from
runtime PM.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_meram.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 25a7118..7ba6dce 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -549,7 +549,7 @@ static int sh_mobile_meram_runtime_suspend(struct device *dev)
 				meram_read_icb(priv->base, j, icb_regs[k]);
 			/* Reset ICB on resume */
 			if (icb_regs[k] = MExxCTL)
-				priv->icb_saved_regs[j * ICB_REGS_SIZE + k] +				priv->icb_saved_regs[j * ICB_REGS_SIZE + k] | 					MExxCTL_WBF | MExxCTL_WF | MExxCTL_RF;
 		}
 	}
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH 4/4] fbdev: sh_mobile_meram: Remove unneeded sh_mobile_meram.h
From: Laurent Pinchart @ 2011-07-30 20:26 UTC (permalink / raw)
  To: linux-fbdev

The drivers/video/sh_mobile_meram.h header contains unused definitions
and declarations. Move the only used macro to sh_mobile_meram.c, and
remove the header.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |    2 +-
 drivers/video/sh_mobile_meram.c  |    5 +++--
 drivers/video/sh_mobile_meram.h  |   33 ---------------------------------
 3 files changed, 4 insertions(+), 36 deletions(-)
 delete mode 100644 drivers/video/sh_mobile_meram.h

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index d0f768e..93e10ef 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -24,10 +24,10 @@
 #include <linux/backlight.h>
 #include <linux/gpio.h>
 #include <video/sh_mobile_lcdc.h>
+#include <video/sh_mobile_meram.h>
 #include <asm/atomic.h>
 
 #include "sh_mobile_lcdcfb.h"
-#include "sh_mobile_meram.h"
 
 #define SIDE_B_OFFSET 0x1000
 #define MIRROR_OFFSET 0x2000
diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 7ba6dce..f632970 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -16,8 +16,7 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
-
-#include "sh_mobile_meram.h"
+#include <video/sh_mobile_meram.h>
 
 /* meram registers */
 #define MEVCR1			0x4
@@ -82,6 +81,8 @@
 	 ((yszm1) << MExxBSIZE_YSZM1_SHIFT) | \
 	 ((xszm1) << MExxBSIZE_XSZM1_SHIFT))
 
+#define SH_MOBILE_MERAM_ICB_NUM		32
+
 static unsigned long common_regs[] = {
 	MEVCR1,
 	MEQSEL1,
diff --git a/drivers/video/sh_mobile_meram.h b/drivers/video/sh_mobile_meram.h
deleted file mode 100644
index 1615204..0000000
--- a/drivers/video/sh_mobile_meram.h
+++ /dev/null
@@ -1,33 +0,0 @@
-#ifndef __sh_mobile_meram_h__
-#define __sh_mobile_meram_h__
-
-#include <linux/mutex.h>
-#include <video/sh_mobile_meram.h>
-
-/*
- * MERAM private
- */
-
-#define MERAM_ICB_Y 0x1
-#define MERAM_ICB_C 0x2
-
-/* MERAM cache size */
-#define SH_MOBILE_MERAM_ICB_NUM		32
-
-#define SH_MOBILE_MERAM_CACHE_OFFSET(p)	((p) >> 16)
-#define SH_MOBILE_MERAM_CACHE_SIZE(p)	((p) & 0xffff)
-
-int sh_mobile_meram_alloc_icb(const struct sh_mobile_meram_cfg *cfg,
-		   int xres,
-		   int yres,
-		   unsigned int base_addr,
-		   int yuv_mode,
-		   int *marker_icb,
-		   int *out_pitch);
-
-void sh_mobile_meram_free_icb(int marker_icb);
-
-#define SH_MOBILE_MERAM_START(ind, ab) \
-	(0xC0000000 | ((ab & 0x1) << 23) | ((ind & 0x1F) << 24))
-
-#endif /* !__sh_mobile_meram_h__ */
-- 
1.7.3.4


^ permalink raw reply related

* Patch fixing savagefb driver typo/regression in linux-3.0.0
From: John Stanley @ 2011-07-31 10:04 UTC (permalink / raw)
  To: linux-fbdev

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

   I have an old T22 Thinkpad with integrated Savage/IX-MV video. As of 
linux-3.0, the video is non-functional; the PC either hard hangs 
immediately or has totally garbled video, and then hard hangs, as soon 
as I attempt to edit a file or cat a file with more than a screen-full 
of text. The foregoing details are actually not important any more as in 
looking at the code changes I discovered a typo in
linux-3.0.0/drivers/video/savage/savagefb.h and have applied the 
attached patch to fix the issue.

PLEASE NOTE:
   I realize that the patch does not quite adhere to linux kernel patch 
coding-style conventions (in particular, spaces around logical 
operators, and lines not exceeding 80 chars), but I have simply changed an
'||' to an '&&' -- and NOT attempted to resolve pre-existing 
coding-style violations.

thanks much,
John

P.S. Many, many thanks for lifting the 800x600 resolution limit! I've 
been using a patch/hack to bump the limit to 1024x768 for an very long 
time...


[-- Attachment #2: linux-3.0.0-savagefb-savage4-series-def-fix.patch --]
[-- Type: text/plain, Size: 1290 bytes --]

This patch corrects a typo/regression in the savage driver code occurring in going
from linux-2.6.39.3 to linux-3.0.0.

In linux-2.6.39.3, there were two savage4 variants (S3_SAVAGE4 and S3_PROSAVAGE).
Hence, S3_SAVAGE4_SERIES was defined as

  S3_SAVAGE4 or S3_PROSAVAGE

In linux-3.0.0, two additional savage4 variants (S3_TWISTER and S3_PROSAVAGEDDR) have
been added. Hence S3_SAVAGE4_SERIES should be defined as

  S3_SAVAGE4, S3_PROSAVAGE, S3_TWISTER, or S3_PROSAVAGEDDR

In going from linux-2.6.39.3 to linux-3.0.0, the define statement for S3_SAVAGE4_SERIES
was incompletely modified: the '||' should have been changed to an '&&'; for an '||' will
set S3_SAVAGE4_SERIES true for any card.

Signed-off-by: John P. Stanley <jpsinthemix@verizon.net>

--- linux-3.0.0/drivers/video/savage/savagefb.h.orig	2011-07-21 22:17:23.000000000 -0400
+++ linux-3.0.0/drivers/video/savage/savagefb.h	2011-07-31 05:27:47.314798215 -0400
@@ -55,7 +55,7 @@
 
 #define S3_SAVAGE3D_SERIES(chip)  ((chip>=S3_SAVAGE3D) && (chip<=S3_SAVAGE_MX))
 
-#define S3_SAVAGE4_SERIES(chip)   ((chip>=S3_SAVAGE4) || (chip<=S3_PROSAVAGEDDR))
+#define S3_SAVAGE4_SERIES(chip)   ((chip>=S3_SAVAGE4) && (chip<=S3_PROSAVAGEDDR))
 
 #define S3_SAVAGE_MOBILE_SERIES(chip)  ((chip==S3_SAVAGE_MX) || (chip==S3_SUPERSAVAGE))
 

^ permalink raw reply

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
From: Geert Uytterhoeven @ 2011-07-31 20:32 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Guennadi Liakhovetski, Florian Tobias Schandinat, Paul Mundt,
	linux-fbdev, linux-media, dri-devel
In-Reply-To: <201107281251.35018.laurent.pinchart@ideasonboard.com>

On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
>> mode, I also prefer an explicit dedicated flag to specify switching to it.
>> Even though using FOURCC doesn't fit under the notion of a videomode, using
>> one of .vmode bits is too tempting, so, I would actually take the plunge and
>> use FB_VMODE_FOURCC.
>
> Another option would be to consider any grayscale > 1 value as a FOURCC. I've
> briefly checked the in-tree drivers: they only assign grayscale with 0 or 1,
> and check whether grayscale is 0 or different than 0. If a userspace
> application only sets grayscale > 1 when talking to a driver that supports the
> FOURCC-based API, we could get rid of the flag.
>
> What can't be easily found out is whether existing applications set grayscale
> to a > 1 value. They would break when used with FOURCC-aware drivers if we
> consider any grayscale > 1 value as a FOURCC. Is that a risk we can take ?

I think we can. I'd expect applications to use either 1 or -1 (i.e.
all ones), both are
invalid FOURCC values.

Still, I prefer the nonstd way.
And limiting traditional nonstd values to the lowest 24 bits (there
are no in-tree
drivers using the highest 8 bits, right?).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
From: Florian Tobias Schandinat @ 2011-07-31 22:54 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Paul Mundt, linux-fbdev,
	linux-media, dri-devel
In-Reply-To: <CAMuHMdX=c=p7oASCE+GgY9AgaCPWoXRQyjEGpn4BvA9xSY6GQg@mail.gmail.com>

On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>  wrote:
>>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
>>> mode, I also prefer an explicit dedicated flag to specify switching to it.
>>> Even though using FOURCC doesn't fit under the notion of a videomode, using
>>> one of .vmode bits is too tempting, so, I would actually take the plunge and
>>> use FB_VMODE_FOURCC.
>>
>> Another option would be to consider any grayscale>  1 value as a FOURCC. I've
>> briefly checked the in-tree drivers: they only assign grayscale with 0 or 1,
>> and check whether grayscale is 0 or different than 0. If a userspace
>> application only sets grayscale>  1 when talking to a driver that supports the
>> FOURCC-based API, we could get rid of the flag.
>>
>> What can't be easily found out is whether existing applications set grayscale
>> to a>  1 value. They would break when used with FOURCC-aware drivers if we
>> consider any grayscale>  1 value as a FOURCC. Is that a risk we can take ?
>
> I think we can. I'd expect applications to use either 1 or -1 (i.e.
> all ones), both are
> invalid FOURCC values.
>
> Still, I prefer the nonstd way.
> And limiting traditional nonstd values to the lowest 24 bits (there
> are no in-tree
> drivers using the highest 8 bits, right?).

Okay, it would be okay for me to
- write raw FOURCC values in nonstd, enable FOURCC mode if upper byte != 0
- not having an explicit flag to enable FOURCC
- in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
- making support of FOURCC visible to userspace by capabilites |= FB_CAP_FOURCC

The capabilities is not strictly necessary but I think it's very useful as
- it allows applications to make sure the extension is supported (for example to 
adjust the UI)
- it allows applications to distinguish whether a particular format is not 
supported or FOURCC at all
- it allows signaling further extensions of the API
- it does not hurt, one line per driver and still some bytes in fixinfo free


So using it would look like this:
- the driver must have capabilities |= FB_CAP_FOURCC
- the application may check capabilities to know whether FOURCC is supported
- the application may write a raw FOURCC value in nonstd to request changing to 
FOURCC mode with this format
- when the driver switches to a FOURCC mode it must have visual = 
FB_VISUAL_FOURCC and the current FOURCC format in nonstd
- the application should check visual and nonstd to make sure it gets what it wanted


So if there are no strong objections against this I think we should implement it.
I do not really care whether we use a union or not but I think if we decide to 
have one it should cover all fields that are undefined/unused in FOURCC mode.


Hope we can find anything that everyone considers acceptable,

Florian Tobias Schandinat

^ permalink raw reply

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
From: Laurent Pinchart @ 2011-07-31 23:28 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Geert Uytterhoeven, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel
In-Reply-To: <4E35DD38.7070609@gmx.de>

Hi Florian,

Thanks for the feedback.

On Monday 01 August 2011 00:54:48 Florian Tobias Schandinat wrote:
> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
> > On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
> >>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
> >>> mode, I also prefer an explicit dedicated flag to specify switching to
> >>> it. Even though using FOURCC doesn't fit under the notion of a
> >>> videomode, using one of .vmode bits is too tempting, so, I would
> >>> actually take the plunge and use FB_VMODE_FOURCC.
> >> 
> >> Another option would be to consider any grayscale>  1 value as a FOURCC.
> >> I've briefly checked the in-tree drivers: they only assign grayscale
> >> with 0 or 1, and check whether grayscale is 0 or different than 0. If a
> >> userspace application only sets grayscale>  1 when talking to a driver
> >> that supports the FOURCC-based API, we could get rid of the flag.
> >> 
> >> What can't be easily found out is whether existing applications set
> >> grayscale to a>  1 value. They would break when used with FOURCC-aware
> >> drivers if we consider any grayscale>  1 value as a FOURCC. Is that a
> >> risk we can take ?
> > 
> > I think we can. I'd expect applications to use either 1 or -1 (i.e.
> > all ones), both are
> > invalid FOURCC values.
> > 
> > Still, I prefer the nonstd way.
> > And limiting traditional nonstd values to the lowest 24 bits (there
> > are no in-tree
> > drivers using the highest 8 bits, right?).
> 
> Okay, it would be okay for me to
> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte != 0
> - not having an explicit flag to enable FOURCC
> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
> - making support of FOURCC visible to userspace by capabilites |> FB_CAP_FOURCC
> 
> The capabilities is not strictly necessary but I think it's very useful as
> - it allows applications to make sure the extension is supported (for
> example to adjust the UI)
> - it allows applications to distinguish whether a particular format is not
> supported or FOURCC at all
> - it allows signaling further extensions of the API
> - it does not hurt, one line per driver and still some bytes in fixinfo
> free

Without a FOURCC capability applications will need to try FOURCCs blindly. 
Drivers that are not FOURCC aware would then risk interpreting the FOURCC as 
something else. As you mention below applications will need that check that 
visual = FB_VISUAL_FOURCC, so it's less of an issue than I initially thought, 
but it doesn't become a non-issue. The display might still show glitches.

> So using it would look like this:
> - the driver must have capabilities |= FB_CAP_FOURCC
> - the application may check capabilities to know whether FOURCC is
> supported - the application may write a raw FOURCC value in nonstd to
> request changing to FOURCC mode with this format
> - when the driver switches to a FOURCC mode it must have visual > FB_VISUAL_FOURCC and the current FOURCC format in nonstd
> - the application should check visual and nonstd to make sure it gets what
> it wanted
> 
> 
> So if there are no strong objections against this I think we should
> implement it. I do not really care whether we use a union or not but I
> think if we decide to have one it should cover all fields that are
> undefined/unused in FOURCC mode.
> 
> 
> Hope we can find anything that everyone considers acceptable,

This sounds good to me, except that I would use the grayscale field instead of 
the nonstd field. nonstd has pretty weird usecases, while grayscale is better 
defined. nonstd might also make sense combined with FOURCC-based modes, while 
grayscale would be completely redundant.

What's your opinion on that ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
From: Laurent Pinchart @ 2011-07-31 23:30 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Guennadi Liakhovetski, Florian Tobias Schandinat, Paul Mundt,
	linux-fbdev, linux-media, dri-devel
In-Reply-To: <CAMuHMdX=c=p7oASCE+GgY9AgaCPWoXRQyjEGpn4BvA9xSY6GQg@mail.gmail.com>

Hi Geert,

Thanks for the feedback.

On Sunday 31 July 2011 22:32:42 Geert Uytterhoeven wrote:
> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
> >> As for struct fb_var_screeninfo fields to support switching to a FOURCC
> >> mode, I also prefer an explicit dedicated flag to specify switching to
> >> it. Even though using FOURCC doesn't fit under the notion of a
> >> videomode, using one of .vmode bits is too tempting, so, I would
> >> actually take the plunge and use FB_VMODE_FOURCC.
> > 
> > Another option would be to consider any grayscale > 1 value as a FOURCC.
> > I've briefly checked the in-tree drivers: they only assign grayscale
> > with 0 or 1, and check whether grayscale is 0 or different than 0. If a
> > userspace application only sets grayscale > 1 when talking to a driver
> > that supports the FOURCC-based API, we could get rid of the flag.
> > 
> > What can't be easily found out is whether existing applications set
> > grayscale to a > 1 value. They would break when used with FOURCC-aware
> > drivers if we consider any grayscale > 1 value as a FOURCC. Is that a
> > risk we can take ?
> 
> I think we can. I'd expect applications to use either 1 or -1 (i.e. all
> ones), both are invalid FOURCC values.

OK.

> Still, I prefer the nonstd way.
> And limiting traditional nonstd values to the lowest 24 bits (there
> are no in-tree drivers using the highest 8 bits, right?).

None that I've found. I still have a preference for the grayscale field 
though. As mentioned by Guennadi, the grayscale field would become redundant 
for FOURCC-based formats. It's then a good candidate, and would let drivers 
(and applications) do any crazy stuff they want with the nonstd field.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
From: Florian Tobias Schandinat @ 2011-07-31 23:58 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel
In-Reply-To: <201108010128.13832.laurent.pinchart@ideasonboard.com>

Hi Laurent,

On 07/31/2011 11:28 PM, Laurent Pinchart wrote:
> Hi Florian,
>
> Thanks for the feedback.
>
> On Monday 01 August 2011 00:54:48 Florian Tobias Schandinat wrote:
>> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
>>> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
>>>>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
>>>>> mode, I also prefer an explicit dedicated flag to specify switching to
>>>>> it. Even though using FOURCC doesn't fit under the notion of a
>>>>> videomode, using one of .vmode bits is too tempting, so, I would
>>>>> actually take the plunge and use FB_VMODE_FOURCC.
>>>>
>>>> Another option would be to consider any grayscale>   1 value as a FOURCC.
>>>> I've briefly checked the in-tree drivers: they only assign grayscale
>>>> with 0 or 1, and check whether grayscale is 0 or different than 0. If a
>>>> userspace application only sets grayscale>   1 when talking to a driver
>>>> that supports the FOURCC-based API, we could get rid of the flag.
>>>>
>>>> What can't be easily found out is whether existing applications set
>>>> grayscale to a>   1 value. They would break when used with FOURCC-aware
>>>> drivers if we consider any grayscale>   1 value as a FOURCC. Is that a
>>>> risk we can take ?
>>>
>>> I think we can. I'd expect applications to use either 1 or -1 (i.e.
>>> all ones), both are
>>> invalid FOURCC values.
>>>
>>> Still, I prefer the nonstd way.
>>> And limiting traditional nonstd values to the lowest 24 bits (there
>>> are no in-tree
>>> drivers using the highest 8 bits, right?).
>>
>> Okay, it would be okay for me to
>> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte != 0
>> - not having an explicit flag to enable FOURCC
>> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
>> - making support of FOURCC visible to userspace by capabilites |>> FB_CAP_FOURCC
>>
>> The capabilities is not strictly necessary but I think it's very useful as
>> - it allows applications to make sure the extension is supported (for
>> example to adjust the UI)
>> - it allows applications to distinguish whether a particular format is not
>> supported or FOURCC at all
>> - it allows signaling further extensions of the API
>> - it does not hurt, one line per driver and still some bytes in fixinfo
>> free
>
> Without a FOURCC capability applications will need to try FOURCCs blindly.
> Drivers that are not FOURCC aware would then risk interpreting the FOURCC as
> something else. As you mention below applications will need that check that
> visual = FB_VISUAL_FOURCC, so it's less of an issue than I initially thought,
> but it doesn't become a non-issue. The display might still show glitches.

True.

>> So using it would look like this:
>> - the driver must have capabilities |= FB_CAP_FOURCC
>> - the application may check capabilities to know whether FOURCC is
>> supported - the application may write a raw FOURCC value in nonstd to
>> request changing to FOURCC mode with this format
>> - when the driver switches to a FOURCC mode it must have visual >> FB_VISUAL_FOURCC and the current FOURCC format in nonstd
>> - the application should check visual and nonstd to make sure it gets what
>> it wanted
>>
>>
>> So if there are no strong objections against this I think we should
>> implement it. I do not really care whether we use a union or not but I
>> think if we decide to have one it should cover all fields that are
>> undefined/unused in FOURCC mode.
>>
>>
>> Hope we can find anything that everyone considers acceptable,
>
> This sounds good to me, except that I would use the grayscale field instead of
> the nonstd field. nonstd has pretty weird usecases, while grayscale is better
> defined. nonstd might also make sense combined with FOURCC-based modes, while
> grayscale would be completely redundant.
>
> What's your opinion on that ?

I do not really care, either one would be okay for me.
You're right that nonstd is used for a lot of things and perhaps some of those 
should still be possible in FOURCC mode. On the other hand I think applications 
are more likely to pass random values to grayscale as its meaning seems globally 
accepted (in contrast to nonstd where the application needs to know the driver 
to get any use of it).
Perhaps you should also say that in FOURCC mode all unused pixel format fields 
should be set to 0 by the application and other values of those may get a 
meaning in later extensions or individual drivers.


Regards,

Florian Tobias Schandinat

^ permalink raw reply

* Fwd: [PATCH 0/2] video: add Samsung SoC based MIPI-DSI support.
From: InKi Dae @ 2011-08-01  4:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4DA6E551.1060300@samsung.com>

Hello, Paul.

I had posted patch sets below on 15 Apr.
but there is no any comments.
please review this patch sets.

thank you.


---------- Forwarded message ----------
From: daeinki <inki.dae@samsung.com>
Date: 2011/4/14
Subject: [PATCH 0/2] video: add Samsung SoC based MIPI-DSI support.
To: linux-fbdev@vger.kernel.org, lethal@linux-sh.org
ÂüÁ¶: kyungmin.park@samsung.com, Kukjin Kim <kgene.kim@samsung.com>,
akpm@linux-foundation.org, linux-arm-kernel@lists.infradead.org


Hello, all.

Samsung S5PV210 and EXYNOS4 SoC platform have one or two MIPI-DSI
controller and MIPI-DSI based LCD Panel could be used with this one.
this patch adds Samsung SoC based MIPI-DSI support.

to use MIPI-DSI based LCD Panel driver, mipi_dsim_lcd_device shoule be
registered to MIPI-DSI driver through s5p_mipi_dsi_register_lcd_device()
at machine code or machine specific somewhere first, and
mipi_dsim_lcd_driver should be registered when
s5p_mipi_dsi_register_driver() is called at init function of lcd driver,
and then probe() of that driver would be called by MIPI-DSI controller
driver if lcd panel name and id of mipi_dsim_lcd_device are same as ones
of mipi_dsim_lcd_driver.

for this, you can refer to sample lcd panel driver.
please see "Documentation/s5p_mipi_dsim/dsim_sample_lcd.c"

this patch series includes the following.

[PATCH 1/2] video: add Samsung SoC MIPI-DSI controller driver.
[PATCH 2/2] video: add Samsung SoC MIPI-DSI based sample lcd panel driver.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
From: Geert Uytterhoeven @ 2011-08-01  9:49 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Paul Mundt, linux-fbdev,
	linux-media, dri-devel
In-Reply-To: <4E35DD38.7070609@gmx.de>

On Mon, Aug 1, 2011 at 00:54, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
>> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart
>> <laurent.pinchart@ideasonboard.com>  wrote:
>>>>
>>>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
>>>> mode, I also prefer an explicit dedicated flag to specify switching to
>>>> it.
>>>> Even though using FOURCC doesn't fit under the notion of a videomode,
>>>> using
>>>> one of .vmode bits is too tempting, so, I would actually take the plunge
>>>> and
>>>> use FB_VMODE_FOURCC.
>>>
>>> Another option would be to consider any grayscale>  1 value as a FOURCC.
>>> I've
>>> briefly checked the in-tree drivers: they only assign grayscale with 0 or
>>> 1,
>>> and check whether grayscale is 0 or different than 0. If a userspace
>>> application only sets grayscale>  1 when talking to a driver that
>>> supports the
>>> FOURCC-based API, we could get rid of the flag.
>>>
>>> What can't be easily found out is whether existing applications set
>>> grayscale
>>> to a>  1 value. They would break when used with FOURCC-aware drivers if
>>> we
>>> consider any grayscale>  1 value as a FOURCC. Is that a risk we can take
>>> ?
>>
>> I think we can. I'd expect applications to use either 1 or -1 (i.e.
>> all ones), both are
>> invalid FOURCC values.
>>
>> Still, I prefer the nonstd way.
>> And limiting traditional nonstd values to the lowest 24 bits (there
>> are no in-tree
>> drivers using the highest 8 bits, right?).
>
> Okay, it would be okay for me to
> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte != 0
> - not having an explicit flag to enable FOURCC
> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
> - making support of FOURCC visible to userspace by capabilites |> FB_CAP_FOURCC
>
> The capabilities is not strictly necessary but I think it's very useful as
> - it allows applications to make sure the extension is supported (for
> example to adjust the UI)
> - it allows applications to distinguish whether a particular format is not
> supported or FOURCC at all
> - it allows signaling further extensions of the API
> - it does not hurt, one line per driver and still some bytes in fixinfo free
>
>
> So using it would look like this:
> - the driver must have capabilities |= FB_CAP_FOURCC
> - the application may check capabilities to know whether FOURCC is supported
> - the application may write a raw FOURCC value in nonstd to request changing
> to FOURCC mode with this format
> - when the driver switches to a FOURCC mode it must have visual > FB_VISUAL_FOURCC and the current FOURCC format in nonstd
> - the application should check visual and nonstd to make sure it gets what
> it wanted

As several of the FOURCC formats duplicate formats you can already
specify in some
other way (e.g. the RGB and greyscale formats), and as FOURCC makes life easier
for the application writer, I'm wondering whether it makes sense to add FOURCC
support in the generic layer for drivers that don't support it? I.e.
the generic layer would
fill in fb_var_screeninfo depending on the requested FOURCC mode, if possible.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [GIT PULL] OMAP display subsystem patches for 3.1
From: Tomi Valkeinen @ 2011-08-01 13:49 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Paul Mundt, linux-omap mailing list, linux-fbdev, lk-ml

Hi Linus,

Sorry for sending this pull request so late.

This pull request contains mostly OMAP display subsystem driver patches
related to preparing and making the DSS driver use runtime PM. Using
runtime PM creates new dependencies to the lower level arch stuff
(clocks and HWMODs), and while the patches for arch side have been ready
for some time, some of them seem to have fallen into some cracks and
didn't make it into the pull requests sent by the HWMOD maintainer. And
then the maintainer went to vacation before I could react.

I had to make two hack patches to the DSS driver to circumvent the
problems. The hack patches are small and easily removed for -rc2
presuming the arch side gets fixed at that point.

So, this pull request got delayed until I could make the fixes and test
them, and thus I'm sending this directly to you, and not via fbdev tree.
I hope this is not too late to make it into 3.1-rc1.

Br,
 Tomi


The following changes since commit 56299378726d5f2ba8d3c8cbbd13cb280ba45e4f:

  Linux 3.0-rc4 (2011-06-20 20:25:46 -0700)

are available in the git repository at:
  git://gitorious.org/linux-omap-dss2/linux.git for-3.1-rc1

Nishant Kamat (1):
      OMAP: DSS: Minor cleanup in ovl and mgr cache structs

Tomi Valkeinen (30):
      OMAP: DSS2: remove extra includes from include/video/omapdss.h
      OMAP: DSS2: DSI: Change dummy macros to inline functions
      OMAP: DSS2: OMAPFB: Add struct to store per-display data
      OMAP: DSS2: OMAPFB: Implement auto-update mode
      OMAP: DSS2: remove update_mode from omapdss
      OMAP: DSS2: Taal: Make driver more fault tolerant
      OMAP: DSS2: Reset LANEx_ULPS_SIG2 bits after use
      OMAP: DSS2: Add FEAT_VENC_REQUIRES_TV_DAC_CLK
      OMAP: DSS2: Add new FEAT definitions for features missing from OMAP2
      OMAP: DSS2: Add Color Phase Rotation support
      OMAP: DSS2: DSI: sync when disabling a display
      OMAP: DSS2: Fix FIFO threshold and burst size for OMAP4
      OMAP: DSS2: Handle dpll4_m4_ck in dss_get/put_clocks
      OMAP: DSS2: Clean up probe for DSS & DSI
      OMAP: DSS2: Fix init and unit sequence
      OMAP: DSS2: Remove clk optimization at dss init
      OMAP: DSS2: rewrite use of context_loss_count
      OMAP: DSS2: Use omap_pm_get_dev_context_loss_count to get ctx loss count
      OMAP: DSS2: DPI: remove unneeded SYSCK enable/disable
      OMAP: DSS2: Remove core_dump_clocks
      OMAP: DSS2: Remove CONFIG_OMAP2_DSS_SLEEP_BEFORE_RESET
      OMAP: DSS2: Use PM runtime & HWMOD support
      OMAP: DSS2: Remove unused opt_clock_available
      OMAP: DSS2: DISPC: remove finegrained clk enables/disables
      OMAP: DSS2: Remove unused code from display.c
      OMAP: DSS2: Remove ctx loss count from dss.c
      OMAP: DSS2: DISPC: Fix context save/restore
      OMAP: DSS2: DSS: Fix context save/restore
      HACK: OMAP: DSS2: clk hack for OMAP2/3
      OMAP: DSS2: HDMI: fix hdmi clock name

 arch/arm/mach-omap2/display.c             |   26 +--
 drivers/video/omap2/displays/panel-taal.c |   55 ++--
 drivers/video/omap2/dss/Kconfig           |   12 -
 drivers/video/omap2/dss/core.c            |   21 +-
 drivers/video/omap2/dss/dispc.c           |  562 ++++++++++++++++------------
 drivers/video/omap2/dss/display.c         |   57 +---
 drivers/video/omap2/dss/dpi.c             |   73 +++--
 drivers/video/omap2/dss/dsi.c             |  296 +++++++++------
 drivers/video/omap2/dss/dss.c             |  583 +++++++----------------------
 drivers/video/omap2/dss/dss.h             |   54 ++--
 drivers/video/omap2/dss/dss_features.c    |   36 ++-
 drivers/video/omap2/dss/dss_features.h    |    7 +
 drivers/video/omap2/dss/hdmi.c            |  162 ++++++--
 drivers/video/omap2/dss/manager.c         |  351 ++++++++++--------
 drivers/video/omap2/dss/overlay.c         |   27 +-
 drivers/video/omap2/dss/rfbi.c            |  114 +++++-
 drivers/video/omap2/dss/sdi.c             |   40 ++-
 drivers/video/omap2/dss/venc.c            |  183 +++++++--
 drivers/video/omap2/omapfb/omapfb-ioctl.c |   72 ++--
 drivers/video/omap2/omapfb/omapfb-main.c  |  166 +++++++--
 drivers/video/omap2/omapfb/omapfb-sysfs.c |   34 ++
 drivers/video/omap2/omapfb/omapfb.h       |   37 ++-
 include/video/omapdss.h                   |   26 +-
 23 files changed, 1639 insertions(+), 1355 deletions(-)



^ permalink raw reply

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
From: Laurent Pinchart @ 2011-08-01 14:11 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Geert Uytterhoeven, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel
In-Reply-To: <4E35EC23.1010701@gmx.de>

Hi Florian,

On Monday 01 August 2011 01:58:27 Florian Tobias Schandinat wrote:
> On 07/31/2011 11:28 PM, Laurent Pinchart wrote:
> > On Monday 01 August 2011 00:54:48 Florian Tobias Schandinat wrote:
> >> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
> >>> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
> >>>>> As for struct fb_var_screeninfo fields to support switching to a
> >>>>> FOURCC mode, I also prefer an explicit dedicated flag to specify
> >>>>> switching to it. Even though using FOURCC doesn't fit under the
> >>>>> notion of a videomode, using one of .vmode bits is too tempting, so,
> >>>>> I would actually take the plunge and use FB_VMODE_FOURCC.
> >>>> 
> >>>> Another option would be to consider any grayscale>   1 value as a
> >>>> FOURCC. I've briefly checked the in-tree drivers: they only assign
> >>>> grayscale with 0 or 1, and check whether grayscale is 0 or different
> >>>> than 0. If a userspace application only sets grayscale>   1 when
> >>>> talking to a driver that supports the FOURCC-based API, we could get
> >>>> rid of the flag.
> >>>> 
> >>>> What can't be easily found out is whether existing applications set
> >>>> grayscale to a>   1 value. They would break when used with
> >>>> FOURCC-aware drivers if we consider any grayscale>   1 value as a
> >>>> FOURCC. Is that a risk we can take ?
> >>> 
> >>> I think we can. I'd expect applications to use either 1 or -1 (i.e.
> >>> all ones), both are
> >>> invalid FOURCC values.
> >>> 
> >>> Still, I prefer the nonstd way.
> >>> And limiting traditional nonstd values to the lowest 24 bits (there
> >>> are no in-tree
> >>> drivers using the highest 8 bits, right?).
> >> 
> >> Okay, it would be okay for me to
> >> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte !> >> 0 - not having an explicit flag to enable FOURCC
> >> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
> >> - making support of FOURCC visible to userspace by capabilites |> >> FB_CAP_FOURCC
> >> 
> >> The capabilities is not strictly necessary but I think it's very useful
> >> as - it allows applications to make sure the extension is supported
> >> (for example to adjust the UI)
> >> - it allows applications to distinguish whether a particular format is
> >> not supported or FOURCC at all
> >> - it allows signaling further extensions of the API
> >> - it does not hurt, one line per driver and still some bytes in fixinfo
> >> free
> > 
> > Without a FOURCC capability applications will need to try FOURCCs
> > blindly. Drivers that are not FOURCC aware would then risk interpreting
> > the FOURCC as something else. As you mention below applications will
> > need that check that visual = FB_VISUAL_FOURCC, so it's less of an
> > issue than I initially thought, but it doesn't become a non-issue. The
> > display might still show glitches.
> 
> True.
> 
> >> So using it would look like this:
> >> - the driver must have capabilities |= FB_CAP_FOURCC
> >> - the application may check capabilities to know whether FOURCC is
> >> supported - the application may write a raw FOURCC value in nonstd to
> >> request changing to FOURCC mode with this format
> >> - when the driver switches to a FOURCC mode it must have visual > >> FB_VISUAL_FOURCC and the current FOURCC format in nonstd
> >> - the application should check visual and nonstd to make sure it gets
> >> what it wanted
> >> 
> >> 
> >> So if there are no strong objections against this I think we should
> >> implement it. I do not really care whether we use a union or not but I
> >> think if we decide to have one it should cover all fields that are
> >> undefined/unused in FOURCC mode.
> >> 
> >> 
> >> Hope we can find anything that everyone considers acceptable,
> > 
> > This sounds good to me, except that I would use the grayscale field
> > instead of the nonstd field. nonstd has pretty weird usecases, while
> > grayscale is better defined. nonstd might also make sense combined with
> > FOURCC-based modes, while grayscale would be completely redundant.
> > 
> > What's your opinion on that ?
> 
> I do not really care, either one would be okay for me.
> You're right that nonstd is used for a lot of things and perhaps some of
> those should still be possible in FOURCC mode. On the other hand I think
> applications are more likely to pass random values to grayscale as its
> meaning seems globally accepted (in contrast to nonstd where the
> application needs to know the driver to get any use of it).
> Perhaps you should also say that in FOURCC mode all unused pixel format
> fields should be set to 0 by the application and other values of those may
> get a meaning in later extensions or individual drivers.

Good point. I'll add that to the documentation.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: Patch fixing savagefb driver typo/regression in linux-3.0.0
From: Tormod Volden @ 2011-08-01 22:56 UTC (permalink / raw)
  To: linux-fbdev

On Sun, Jul 31, 2011 at 12:04 PM, John Stanley wrote:
>  I have an old T22 Thinkpad with integrated Savage/IX-MV video. As of
> linux-3.0, the video is non-functional; the PC either hard hangs immediately
> or has totally garbled video, and then hard hangs, as soon as I attempt to
> edit a file or cat a file with more than a screen-full of text. The
> foregoing details are actually not important any more as in looking at the
> code changes I discovered a typo in
> linux-3.0.0/drivers/video/savage/savagefb.h and have applied the attached
> patch to fix the issue.

John, you are absolutely correct. My bad, this was from a commit of
mine. You may add my
 Reviewed-by: Tormod Volden <debian.tormod@gmail.com>
to your patch. I'd suggest you change the patch description (mail
subject) to "[PATCH] Fix typo in savage4 series definition" or
something similar.

The patch should also go into the stable series (by cc'ing
stable@vger.kernel.org if I remember correctly).

> PLEASE NOTE:
>  I realize that the patch does not quite adhere to linux kernel patch
> coding-style conventions (in particular, spaces around logical operators,
> and lines not exceeding 80 chars), but I have simply changed an
> '||' to an '&&' -- and NOT attempted to resolve pre-existing coding-style
> violations.

I also preserved the old coding style, which is the same in the xorg
driver which shares much of this code. We can always change it there
as well if somebody would like to fix it up.

>
> thanks much,
> John
>
> P.S. Many, many thanks for lifting the 800x600 resolution limit! I've been
> using a patch/hack to bump the limit to 1024x768 for an very long time...

Glad to hear that! So there are still other savagefb users around :)
My apologies to those who were affected by the typo.

Cheers,
Tormod

^ 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