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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Ville,

On Monday, 11 December 2017 16:24:32 EET Ville Syrjälä wrote:
> On Mon, Dec 11, 2017 at 04:08:07PM +0200, Laurent Pinchart wrote:
> > 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@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@xilinx.com>
> >>>>> Signed-off-by: Hyun Kwon <hyun.kwon@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.

What I meant is that despite the presentation of this 8-bit greyscale format 
as storing 4 pixels in 32 bits, it's really an 8-bit greyscale format without 
anything special.

> > 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? ;)

Don't get me started about some of the GPU tiled formats ;-) The graphics and 
camera industries have taken different paths because they have different 
needs, but both have invented their share of convoluted formats that are not 
always easy to support.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel