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

[Hyun Kwon]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Tuesday, November 28, 2017 10:12 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 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".

Make sense to have a separate section for this kind of formats. Repeating the cover letter message for more details:

---
This series is to add new drm formats needed by some Xilinx IPs.
Some formats have unique characteristics such as pixels not being
byte-aligned. For instance, some 10bit formats have 2bit padding
after every 3-10bit components:

	32b[0]:	10b comp0 - 10b comp1 - 10b comp2 - 2b padding
	32b[1]:	10b comp3 - 10b comp4 - 10b comp5 - 2b padding
	...

To model this, additional information is added to struct drm_format_info.
The patch has been tested with downstream drivers as well as the downstream
user space component (ex, modified modetest).
---

Thanks,
-hyun

> 
> --
> Ville Syrjälä
> Intel OTC