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

[Ville Syrjälä]

On Wed, May 30, 2012 at 03:20:09PM +0200, Laurent Pinchart wrote:
> 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 at 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 ?

The way I used it originally was to calculate the minimum stride for a
plane. So the formula was something like this:
min_stride = width / plane_horiz_subsampling * plane_cpp

I guess you could also call it pixel stride (as opposed to line stride).
That is when you're walking the Cb (or Cr) samples you need to step two
bytes to the get to the next sample.

Or if you just think about the chroma plane as just another packed pixel
format then it also makes sense to have cpp=2.

YUYV and co. are more problematic though since you have the subsampled
and non-subsampled stuff in the same plane. There you could argue that
both 2 and 4 are sensible values. I used 2 there since it meant that I
didn't have to add special cases to the minimum stride calculations.

There might be a need to add drm_format_macropixel_size() or some such
function in the future which would return 4 for YUYV. Especially if
someone wants to add funky formats such as Y41P.

-- 
Ville Syrjälä
Intel OTC