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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Damian,

On Tue, Feb 15, 2011 at 10:51 AM, Damian Hobson-Garcia
<dhobsong@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---

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
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux