Re: [PATCH 3/4] drm: Add NV24 and NV42 pixel formats

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Ville,

Thank you for the review.

On Wednesday 30 May 2012 16:09:25 Ville Syrjälä wrote:
> On Wed, May 30, 2012 at 02:32:58PM +0200, Laurent Pinchart wrote:
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  include/drm/drm_fourcc.h |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> > index bdf0152..fac7235 100644
> > --- a/include/drm/drm_fourcc.h
> > +++ b/include/drm/drm_fourcc.h
> > @@ -106,6 +106,8 @@
> > 
> >  #define DRM_FORMAT_NV21		fourcc_code('N', 'V', '2', '1') /* 2x2
> >  subsampled Cb:Cr plane */ #define DRM_FORMAT_NV16		fourcc_code('N', 
'V',
> >  '1', '6') /* 2x1 subsampled Cr:Cb plane */ #define
> >  DRM_FORMAT_NV61		fourcc_code('N', 'V', '6', '1') /* 2x1 subsampled 
Cb:Cr
> >  plane */> 
> > +#define DRM_FORMAT_NV24		fourcc_code('N', 'V', '2', '4') /*
> > non-subsampled Cr:Cb plane */ +#define DRM_FORMAT_NV42		
fourcc_code('N',
> > 'V', '4', '2') /* non-subsampled Cb:Cr plane */
> If you want these to reach the driver you need to add them to
> format_check().

Oops, my bad, indeed.

> Also you should update drm_format_num_planes() and drm_format_plane_cpp()
> appropriately.

Will do.

I'm a bit puzzled by drm_format_plane_cpp(). I would have expected the return 
value to be 1 for NV12/21 and NV16/61 formats (2 U/V components, but 1/2 
horizontal subsampling). Is that a bug, or am I missing something ?

-- 
Regards,

Laurent Pinchart