Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Mar 30, 2012 at 01:12:58PM +0300, Ville Syrjälä wrote:
> On Fri, Mar 30, 2012 at 11:54:50AM +0900, Seung-Woo Kim wrote:
> > Multi buffer plane pixel formats are added as like kernel header.
> > 
> > Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
> > ---
> >  include/drm/drm_fourcc.h |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> > index 85facb0..7cfd95a 100644
> > --- a/include/drm/drm_fourcc.h
> > +++ b/include/drm/drm_fourcc.h
> > @@ -107,6 +107,10 @@
> >  #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 */
> >  
> > +/* 2 non contiguous plane YCbCr */
> > +#define DRM_FORMAT_NV12M	fourcc_code('N', 'M', '1', '2') /* 2x2 subsampled Cr:Cb plane */
> 
> NAK. DRM_FORMAT_NV12 handles this just fine.

And I just realized that I was already too late with my NAK since this a
libdrm patch. Apparently the kernel drm_fourcc.h changes were snuck in
via some backdoor without review. Sigh.

So they're now in Linus's tree. But looks like format_check() was never
updated to accept them, so there's no way anyone could actually be using
them. So Dave, can we still remove them from the kernel header?


Just to clarify once mode. The original planar formats I defined in
drm_fourcc.h handle non-contiguous planes. The drivers are supposed to
use handles[],offsets[],pitches[] to handle this. The 'index' comments
in the drm_fourcc.h tells you which index of those arrays matches which
plane. This means that DRM_FORMAT_NV12M is functionally _identical_ to
DRM_FORMAT_NV12, and the same holds for the three plane format that got
submarined in.

The driver is not even supposed to accept DRM_FORMAT_NV12 unless both
index 0 and 1 are filled in properly by userspace. If the planes are
contiguous then userspace _must_ pass the same handle for index 0 and
1, and use offsets[] to tell the driver where each plane is. If the
hardware can't handle non-contiguous planes (never seen such hardware
myself) then the driver must refuse the operation if userspace passes
such a buffer to it.

I already posted a patch with a drm_framebuffer_check() function that
does basic sanity checking on this stuff. I'll pull some checksa from
that function and add them directly to drm_mode_addfb2(). Some of the
checks require the driver to pass the BO sizes, so those I can't add.
Also the plane overlap checks I had shouldn't be in generic code
because some hardware can handle line-by-line interleaved planes, and
my code would refuse those. Ideally the code should be improved to
allow such interleaved planes, and then the check could be added to the
generic code.

-- 
Ville Syrjälä
Intel OTC