[PATCH 1/1] sh_mobile_lcdc: Add NV12 input framebuffer support

[PATCH 1/1] sh_mobile_lcdc: Add NV12 input framebuffer support

[Damian Hobson-Garcia]

Specify .bpp = 12 and .yuv = 1 when configuring the LCDC channel that you want
to you with NV12 input support.

Due to the encoding of YUV data, writing 0s to the framebuffer will clear to
green instead of black.

There is also no native framebuffer support for YUV formats,
so this mode will not work with most software rendering.

Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
---
 drivers/video/sh_mobile_lcdcfb.c |   45 ++++++++++++++++++++++++++++++++-----
 drivers/video/sh_mobile_lcdcfb.h |    2 +-
 include/video/sh_mobile_lcdc.h   |    3 ++
 3 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index bd4840a..3a3b7d6 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -67,6 +67,7 @@ static unsigned long lcdc_offs_mainlcd[NR_CH_REGS] = {
 	[LDSM1R] = 0x428,
 	[LDSM2R] = 0x42c,
 	[LDSA1R] = 0x430,
+	[LDSA2R] = 0x434,
 	[LDMLSR] = 0x438,
 	[LDHCNR] = 0x448,
 	[LDHSYNR] = 0x44c,
@@ -151,6 +152,7 @@ static bool banked(int reg_nr)
 	case LDDFR:
 	case LDSM1R:
 	case LDSA1R:
+	case LDSA2R:
 	case LDMLSR:
 	case LDHCNR:
 	case LDHSYNR:
@@ -545,6 +547,7 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 	case 16:
 		lcdc_write(priv, _LDDDSR, lcdc_read(priv, _LDDDSR) | 6);
 		break;
+	case 12:
 	case 24:
 		lcdc_write(priv, _LDDDSR, lcdc_read(priv, _LDDDSR) | 7);
 		break;
@@ -561,8 +564,11 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 
 		/* set bpp format in PKF[4:0] */
 		tmp = lcdc_read_chan(ch, LDDFR);
-		tmp &= ~0x0001001f;
+		tmp &= ~0x0003031f;
 		switch (ch->info->var.bits_per_pixel) {
+		case 12:
+			tmp |= (0x3 << 16);
+			break;
 		case 16:
 			tmp |= 0x03;
 			break;
@@ -576,6 +582,10 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 
 		/* point out our frame buffer */
 		lcdc_write_chan(ch, LDSA1R, ch->info->fix.smem_start);
+		if (ch->info->var.nonstd = REN_COLOR_NV12)
+			lcdc_write_chan(ch, LDSA2R,
+				ch->info->fix.smem_start +
+				ch->info->var.xres * ch->info->var.yres);
 
 		/* set line size */
 		lcdc_write_chan(ch, LDMLSR, ch->info->fix.line_length);
@@ -808,6 +818,9 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
 	new_pan_offset = (var->yoffset * info->fix.line_length) +
 		(var->xoffset * (info->var.bits_per_pixel / 8));
 
+	if (var->nonstd = REN_COLOR_NV12)
+		new_pan_offset = new_pan_offset * 3 / 2;
+
 	if (new_pan_offset = ch->pan_offset)
 		return 0;	/* No change, do nothing */
 
@@ -815,6 +828,9 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
 
 	/* Set the source address for the next refresh */
 	lcdc_write_chan_mirror(ch, LDSA1R, ch->dma_handle + new_pan_offset);
+	if (var->nonstd = REN_COLOR_NV12)
+		lcdc_write_chan_mirror(ch, LDSA2R, ch->dma_handle
+			+ new_pan_offset + var->xres * var->yres);
 	if (lcdc_chan_is_sublcd(ch))
 		lcdc_write(ch->lcdc, _LDRCNTR, ldrcntr ^ LDRCNTR_SRS);
 	else
@@ -885,7 +901,10 @@ static void sh_mobile_fb_reconfig(struct fb_info *info)
 		/* Couldn't reconfigure, hopefully, can continue as before */
 		return;
 
-	info->fix.line_length = mode1.xres * (ch->cfg.bpp / 8);
+	if (info->var.nonstd = REN_COLOR_NV12)
+		info->fix.line_length = mode1.xres;
+	else
+		info->fix.line_length = mode1.xres * (ch->cfg.bpp / 8);
 
 	/*
 	 * fb_set_var() calls the notifier change internally, only if
@@ -980,8 +999,18 @@ static struct fb_ops sh_mobile_lcdc_ops = {
 	.fb_check_var	= sh_mobile_check_var,
 };
 
-static int sh_mobile_lcdc_set_bpp(struct fb_var_screeninfo *var, int bpp)
+static int sh_mobile_lcdc_set_bpp(struct fb_var_screeninfo *var, int bpp,
+				   int yuv)
 {
+	if (yuv) {
+		if (bpp = 12) { /*NV12*/
+			var->bits_per_pixel = 12;
+			var->nonstd = REN_COLOR_NV12;
+			return 0;
+		}
+		return -EINVAL;
+	}
+
 	switch (bpp) {
 	case 16: /* PKF[4:0] = 00011 - RGB 565 */
 		var->red.offset = 11;
@@ -1274,7 +1303,7 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
 				max_cfg->xres, max_cfg->yres);
 
 		info->fix = sh_mobile_lcdc_fix;
-		info->fix.smem_len = max_size * (cfg->bpp / 8) * 2;
+		info->fix.smem_len = max_size * 2 * cfg->bpp / 8;
 
 		if (!mode) {
 			mode = &default_720p;
@@ -1292,7 +1321,7 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
 		var->yres_virtual = var->yres * 2;
 		var->activate = FB_ACTIVATE_NOW;
 
-		error = sh_mobile_lcdc_set_bpp(var, cfg->bpp);
+		error = sh_mobile_lcdc_set_bpp(var, cfg->bpp, cfg->yuv);
 		if (error)
 			break;
 
@@ -1316,7 +1345,11 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
 		}
 
 		info->fix.smem_start = ch->dma_handle;
-		info->fix.line_length = var->xres * (cfg->bpp / 8);
+		if (var->nonstd = REN_COLOR_NV12)
+			info->fix.line_length = var->xres;
+		else
+			info->fix.line_length = var->xres * (cfg->bpp / 8);
+
 		info->screen_base = buf;
 		info->device = &pdev->dev;
 		ch->display_var = *var;
diff --git a/drivers/video/sh_mobile_lcdcfb.h b/drivers/video/sh_mobile_lcdcfb.h
index 9ecee2f..c953cb0 100644
--- a/drivers/video/sh_mobile_lcdcfb.h
+++ b/drivers/video/sh_mobile_lcdcfb.h
@@ -8,7 +8,7 @@
 
 /* per-channel registers */
 enum { LDDCKPAT1R, LDDCKPAT2R, LDMT1R, LDMT2R, LDMT3R, LDDFR, LDSM1R,
-       LDSM2R, LDSA1R, LDMLSR, LDHCNR, LDHSYNR, LDVLNR, LDVSYNR, LDPMR,
+       LDSM2R, LDSA1R, LDSA2R, LDMLSR, LDHCNR, LDHSYNR, LDVLNR, LDVSYNR, LDPMR,
        LDHAJR,
        NR_CH_REGS };
 
diff --git a/include/video/sh_mobile_lcdc.h b/include/video/sh_mobile_lcdc.h
index daabae5..147c80d 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -25,6 +25,8 @@ enum {
 	SYS24,  /* 24bpp */
 };
 
+#define REN_COLOR_NV12  0x1 /* Non-standard framebuffer color format - NV12 */
+
 enum { LCDC_CHAN_DISABLED = 0,
        LCDC_CHAN_MAINLCD,
        LCDC_CHAN_SUBLCD };
@@ -77,6 +79,7 @@ struct sh_mobile_lcdc_chan_cfg {
 	struct sh_mobile_lcdc_lcd_size_cfg lcd_size_cfg;
 	struct sh_mobile_lcdc_board_cfg board_cfg;
 	struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn I/F */
+	int yuv;
 };
 
 struct sh_mobile_lcdc_info {
-- 
1.7.1

Re: [PATCH 1/1] sh_mobile_lcdc: Add NV12 input framebuffer support

[Magnus Damm]

Hi Damian,

On Tue, Feb 15, 2011 at 10:51 AM, Damian Hobson-Garcia
<dhobsong@igel.co.jp> wrote:
> Specify .bpp = 12 and .yuv = 1 when configuring the LCDC channel that you want
> to you with NV12 input support.
>
> Due to the encoding of YUV data, writing 0s to the framebuffer will clear to
> green instead of black.
>
> There is also no native framebuffer support for YUV formats,
> so this mode will not work with most software rendering.
>
> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> ---

Nice to see patches for YUV mode support!

> --- a/include/video/sh_mobile_lcdc.h
> +++ b/include/video/sh_mobile_lcdc.h
> @@ -25,6 +25,8 @@ enum {
>        SYS24,  /* 24bpp */
>  };
>
> +#define REN_COLOR_NV12  0x1 /* Non-standard framebuffer color format - NV12 */
> +
>  enum { LCDC_CHAN_DISABLED = 0,
>        LCDC_CHAN_MAINLCD,
>        LCDC_CHAN_SUBLCD };
> @@ -77,6 +79,7 @@ struct sh_mobile_lcdc_chan_cfg {
>        struct sh_mobile_lcdc_lcd_size_cfg lcd_size_cfg;
>        struct sh_mobile_lcdc_board_cfg board_cfg;
>        struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn I/F */
> +       int yuv;

Instead of having "yuv" here I think you should use "nonstd" and make
sure the "nonstd" field in struct fb_var_screeninfo is set the same
way.

Also, the REN_COLOR_NV12 constant can go away IMO.

I believe the best way to deal with this is to map the "nonstd" value
directly to bit 16-18 in LDDFR. If "nonstd" is non-zero then you
should program bits 8-9 depending on the bpp value. This way both 12,
16 and 20 (?) bit YUV can be supported with compression enabled or
not. I believe both sh7724 and sh7372 can be supported too - given
that the correct "nonstd" value is provided. But since it's provided
by the platform data it is part of the system configuration and we can
assume it is correct.

Please consider reworking the patch to make it more generic. Just
adding NV12 support is aiming too low. =)

Thanks,

/ magnus

Re: [PATCH 1/1] sh_mobile_lcdc: Add NV12 input framebuffer support

[Damian]

On 2011/02/17 9:19, Magnus Damm wrote:
Hi Magnus,
> Hi Damian,
>
> On Tue, Feb 15, 2011 at 10:51 AM, Damian Hobson-Garcia
> <dhobsong@igel.co.jp>  wrote:
>> Specify .bpp = 12 and .yuv = 1 when configuring the LCDC channel that you want
>> to you with NV12 input support.
>>
>> Due to the encoding of YUV data, writing 0s to the framebuffer will clear to
>> green instead of black.
>>
>> There is also no native framebuffer support for YUV formats,
>> so this mode will not work with most software rendering.
>>
>> Signed-off-by: Damian Hobson-Garcia<dhobsong@igel.co.jp>
>> ---
>
> Nice to see patches for YUV mode support!
And thank you for your comments.
>
>> --- a/include/video/sh_mobile_lcdc.h
>> +++ b/include/video/sh_mobile_lcdc.h
>> @@ -25,6 +25,8 @@ enum {
>>         SYS24,  /* 24bpp */
>>   };
>>
>> +#define REN_COLOR_NV12  0x1 /* Non-standard framebuffer color format - NV12 */
>> +
>>   enum { LCDC_CHAN_DISABLED = 0,
>>         LCDC_CHAN_MAINLCD,
>>         LCDC_CHAN_SUBLCD };
>> @@ -77,6 +79,7 @@ struct sh_mobile_lcdc_chan_cfg {
>>         struct sh_mobile_lcdc_lcd_size_cfg lcd_size_cfg;
>>         struct sh_mobile_lcdc_board_cfg board_cfg;
>>         struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn I/F */
>> +       int yuv;
>
> Instead of having "yuv" here I think you should use "nonstd" and make
> sure the "nonstd" field in struct fb_var_screeninfo is set the same
> way.
>
> Also, the REN_COLOR_NV12 constant can go away IMO.
Yes, I guess that using the nonstd together with the bits_per_pixel,
REN_COLOR_NV12 is unnecessary.
>
> I believe the best way to deal with this is to map the "nonstd" value
> directly to bit 16-18 in LDDFR. If "nonstd" is non-zero then you
> should program bits 8-9 depending on the bpp value. This way both 12,
> 16 and 20 (?) bit YUV can be supported with compression enabled or
> not. I believe both sh7724 and sh7372 can be supported too - given
> that the correct "nonstd" value is provided. But since it's provided
> by the platform data it is part of the system configuration and we can
> assume it is correct.
Sounds good.  By 20 bit are you referring to the YCbCr 4:4:4 mode?  I 
haven't really looked at that but it sounds like it should work.  I 
think that its a 24 bit mode though, if I understand correctly 8 bits 
each for Y, Cb, Cr.
>
> Please consider reworking the patch to make it more generic. Just
> adding NV12 support is aiming too low. =)
>
I'm also looking at re-arranging the arrangement of the Y and CbCr 
planes so that in a double buffer (or more) situation all of the Y 
planes come first, followed by the CbCr planes, as it makes the panning 
calculations much simpler.

I'll submit these all as a version 2 of this patch.
> Thanks,
>
> / magnus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp

Re: [PATCH 1/1] sh_mobile_lcdc: Add NV12 input framebuffer support

[Damian]

On 2011/02/17 14:45, Damian wrote:
Hi again Magnus,
> On 2011/02/17 9:19, Magnus Damm wrote:
> Hi Magnus,
>> Hi Damian,
>>
>> On Tue, Feb 15, 2011 at 10:51 AM, Damian Hobson-Garcia
>> <dhobsong@igel.co.jp> wrote:
>>> Specify .bpp = 12 and .yuv = 1 when configuring the LCDC channel that
>>> you want
>>> to you with NV12 input support.
>>>
>>> Due to the encoding of YUV data, writing 0s to the framebuffer will
>>> clear to
>>> green instead of black.
>>>
>>> There is also no native framebuffer support for YUV formats,
>>> so this mode will not work with most software rendering.
>>>
>>> Signed-off-by: Damian Hobson-Garcia<dhobsong@igel.co.jp>
>>> ---
>>
>> Nice to see patches for YUV mode support!
> And thank you for your comments.
>>
>>> --- a/include/video/sh_mobile_lcdc.h
>>> +++ b/include/video/sh_mobile_lcdc.h
>>> @@ -25,6 +25,8 @@ enum {
>>> SYS24, /* 24bpp */
>>> };
>>>
>>> +#define REN_COLOR_NV12 0x1 /* Non-standard framebuffer color format
>>> - NV12 */
>>> +
>>> enum { LCDC_CHAN_DISABLED = 0,
>>> LCDC_CHAN_MAINLCD,
>>> LCDC_CHAN_SUBLCD };
>>> @@ -77,6 +79,7 @@ struct sh_mobile_lcdc_chan_cfg {
>>> struct sh_mobile_lcdc_lcd_size_cfg lcd_size_cfg;
>>> struct sh_mobile_lcdc_board_cfg board_cfg;
>>> struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn I/F */
>>> + int yuv;
>>
>> Instead of having "yuv" here I think you should use "nonstd" and make
>> sure the "nonstd" field in struct fb_var_screeninfo is set the same
>> way.
>>
>> Also, the REN_COLOR_NV12 constant can go away IMO.
> Yes, I guess that using the nonstd together with the bits_per_pixel,
> REN_COLOR_NV12 is unnecessary.
>>
>> I believe the best way to deal with this is to map the "nonstd" value
>> directly to bit 16-18 in LDDFR. If "nonstd" is non-zero then you
>> should program bits 8-9 depending on the bpp value. This way both 12,
>> 16 and 20 (?) bit YUV can be supported with compression enabled or
>> not. I believe both sh7724 and sh7372 can be supported too - given
>> that the correct "nonstd" value is provided. But since it's provided
>> by the platform data it is part of the system configuration and we can
>> assume it is correct.

One question/comment about setting the value of nonstd.
Ideally, when an application reads the nonstd field from 
fb_var_screeninfo, it would need to be sure that the value corresponds 
to a YUV setting instead of some other valid non-zero nonstd value, 
right? So maybe defining a bitmask for the YUV bit somewhere accessible 
from user space would be beneficial (perhaps a new file, 
linux/sh_mobile_fb.h, which might also be a good place to define any 
ioctls that might be used in future, for blending for example).
It seems that for both sh7372 and sh7724 that bit would be bit 16 in 
LDDFR, which is nice. As for the remaining bits, which are chip 
dependent, I was thinking that it would be good to define similar 
mask/flags and "or" them all together to build up nonstd.  The question 
is where to define these.  There are separate files for mach/sh7724.h 
and mach/sh7372.h, which I though might fit nicely or would it be better 
to define flags for both chips in video/sh_mobile_lcdc.h?

> Sounds good. By 20 bit are you referring to the YCbCr 4:4:4 mode? I
> haven't really looked at that but it sounds like it should work. I think
> that its a 24 bit mode though, if I understand correctly 8 bits each for
> Y, Cb, Cr.
>>
>> Please consider reworking the patch to make it more generic. Just
>> adding NV12 support is aiming too low. =)
>>
> I'm also looking at re-arranging the arrangement of the Y and CbCr
> planes so that in a double buffer (or more) situation all of the Y
> planes come first, followed by the CbCr planes, as it makes the panning
> calculations much simpler.
>
> I'll submit these all as a version 2 of this patch.
>> Thanks,
>>
>> / magnus
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

Thanks,
-- 
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp