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

[Laurent Pinchart]

Hi Ville,

On Wednesday 30 May 2012 17:05:10 Ville Syrjälä wrote:
> On Wed, May 30, 2012 at 03:20:09PM +0200, Laurent Pinchart wrote:
> > 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 had missed the division by the horizontal subsampling factor in 
framebuffer_check(). It makes sense now.

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

-- 
Regards,

Laurent Pinchart