On Tue, Nov 28, 2017 at 05:25:46PM +0000, Hyun Kwon wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > Sent: Tuesday, November 28, 2017 6:44 AM
> > To: Hyun Kwon <hy...@xilinx.com>
> > Cc: Daniel Vetter <daniel.vet...@intel.com>; Jani Nikula
> > <jani.nik...@linux.intel.com>; Sean Paul <seanp...@chromium.org>; David
> > Airlie <airl...@linux.ie>; dri-devel@lists.freedesktop.org;
> > mon...@monstr.eu; Laurent Pinchart
> > <laurent.pinch...@ideasonboard.com>; Satish Kumar Nagireddy
> > <satis...@xilinx.com>; Jeff Mouroux <jmour...@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 <jmour...@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 <jmour...@xilinx.com>
> > > Signed-off-by: Hyun Kwon <hyun.k...@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to