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

[Ville Syrjälä]

On Mon, Dec 11, 2017 at 04:08:07PM +0200, Laurent Pinchart wrote:
> Hi Ville,
> 
> On Tuesday, 28 November 2017 20:11:42 EET Ville Syrjälä wrote:
> > On Tue, Nov 28, 2017 at 05:25:46PM +0000, Hyun Kwon wrote:
> > > On Tuesday, November 28, 2017 6:44 AM Ville Syrjälä wrote:
> > >> 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".
> 
> Thee Y8 format isn't special as it's just a plain 8-bit format.

Is it? The patch made it out to be something else perhaps. Impossible
to judge without reading the relevant xilinx docs I suppose.

> The Y10 format 
> is indeed not byte-addressable, but don't be too fast to dismiss it as crazy, 
> this kind of format is pretty common outside of the desktop graphics world. 
> There are even 10-bit formats with 3 components where the 8 LSBs of each 
> component are stored in three bytes, and the 2 MSBs are grouped in a fourth 
> byte with 2 bits of padding (or possibly with LSB and MSB switched, I can't 
> remember right now).

So you're saying the formats in this patch aren't quite as crazy as some
other formats out there? ;)

-- 
Ville Syrjälä
Intel OTC