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

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
[off-list ref]  wrote:

quoted

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<redacted>
---

Nice to see patches for YUV mode support!

And thank you for your comments.

quoted

--- 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