[PATCH 2/3] uapi: drm: New fourcc codes needed by Xilinx Video IP

[Ville Syrjälä]

On Tue, Nov 28, 2017 at 05:25:46PM +0000, Hyun Kwon wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > Sent: Tuesday, November 28, 2017 6:44 AM
> > To: Hyun Kwon <hyunk at xilinx.com>
> > Cc: Daniel Vetter <daniel.vetter at intel.com>; Jani Nikula
> > <jani.nikula at linux.intel.com>; Sean Paul <seanpaul at chromium.org>; David
> > Airlie <airlied at linux.ie>; dri-devel at lists.freedesktop.org;
> > monstr at monstr.eu; Laurent Pinchart
> > <laurent.pinchart at ideasonboard.com>; Satish Kumar Nagireddy
> > <SATISHNA at xilinx.com>; Jeff Mouroux <jmouroux at xilinx.com>
> > Subject: Re: [PATCH 2/3] uapi: drm: New fourcc codes needed by Xilinx
> > Video IP
> > 
> > On Mon, Nov 27, 2017 at 06:27:32PM -0800, Hyun Kwon wrote:
> > > From: Jeffrey Mouroux <jmouroux at xilinx.com>
> > >
> > > The Xilinx Video Mixer and Xilinx Video Framebuffer DMA IP
> > > support video memory formats that are not represented in the
> > > current DRM fourcc library.  This patch adds those missing
> > > fourcc codes.
> > >
> > > Signed-off-by: Jeffrey Mouroux <jmouroux at xilinx.com>
> > > Signed-off-by: Hyun Kwon <hyun.kwon at xilinx.com>
> > > ---
> > >  include/uapi/drm/drm_fourcc.h | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/include/uapi/drm/drm_fourcc.h
> > b/include/uapi/drm/drm_fourcc.h
> > > index 3ad838d..83806d5 100644
> > > --- a/include/uapi/drm/drm_fourcc.h
> > > +++ b/include/uapi/drm/drm_fourcc.h
> > > @@ -112,6 +112,13 @@ extern "C" {
> > >  #define DRM_FORMAT_VYUY		fourcc_code('V', 'Y', 'U', 'Y')
> > /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */
> > >
> > >  #define DRM_FORMAT_AYUV		fourcc_code('A', 'Y', 'U', 'V')
> > /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
> > > +#define DRM_FORMAT_VUY888	fourcc_code('V', 'U', '2', '4') /* [23:0]
> > Cr:Cb:Y little endian */
> > > +#define DRM_FORMAT_XVUY8888	fourcc_code('X', 'V', '2', '4') /* [31:0]
> > x:Cr:Cb:Y 8:8:8:8 little endian */
> > > +#define DRM_FORMAT_XVUY2101010	fourcc_code('X', 'V', '3', '0')
> > /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */
> > > +
> > > +/* Grey scale */
> > > +#define DRM_FORMAT_Y8		fourcc_code('G', 'R', 'E', 'Y') /* 8-bit
> > packed greyscale Y:Y:Y:Y 8:8:8:8 */
> > > +#define DRM_FORMAT_Y10		fourcc_code('Y', '1', '0', ' ') /*
> > 10-bit packed greyscale X:Y:Y:Y 2:10:10:10 */
> > 
> > These don't make sense to me. What does it even mean to have Y
> > replicated three or four times for each pixel?
> 
> Each pixel has one Y component. The comment is to describe how components are stored in 32bit, but I agree it's confusing. Will describe better.

For the 8 bit Y format you should then define it as 8 bits. Unless of
course it really is defined as a 32bit word containing 4 pixels. The
10 bit case would be even funky since there would have to be two
bits of padding after every 3 pixels.

So are these really defined as 32 bit wits 3 or 4 pixels and potentially
a few bits of extra padding packed into each word? That seems rather
nuts to me because you can't even byte address each pixel.

I think such crazyness has no business living right next to the
sane formats, hence we should put all these into their own little
section of the header, and the names should somehow reflect that
they are in fact "special".

-- 
Ville Syrjälä
Intel OTC