Re: [PATCH] drm: add FOURCC formats for compute dma_buf interop.

[Daniel Vetter <daniel@ffwll.ch>]

On Sat, Mar 15, 2014 at 05:41:05AM +0100, Gwenole Beauchesne wrote:
> Hi,
> 
> 2014-03-14 22:52 GMT+01:00 Daniel Vetter <daniel@ffwll.ch>:
> > On Fri, Mar 14, 2014 at 06:59:21PM +0100, Gwenole Beauchesne wrote:
> >> This is a follow-up to:
> >> http://lists.freedesktop.org/archives/mesa-dev/2014-March/055742.html
> >>
> >> Add formats meant to convey a "compute" dimension when a DRM fourcc
> >> format is needed for dma_buf interop (EGL, OpenCL).
> >>
> >> Intended FOURCC format: ('T',<num_elements>,<type>,<size_element>).
> >> - <num_elements>: number of elements in the tuple. Range: [0..3].
> >> - <type>: type of each element. Values: {'_','I','U','F'},
> >>   respectively for normalized to [0..1] range, signed integers,
> >>   unsigned integers, floating-point.
> >> - <size_element>: size of the element. Values: {1, 2, 4, 8}.
> >>
> >> All entities are represented in native-endian byte-order in memory.
> >> For example, 'T2F4' format would represent the (float, float) tuple
> >> where elements are stored in little-endian byte-order on x86.
> >>
> >> Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
> >
> > So this fourcc stuff started out as pixel formats for the ADDFB2 ioctl,
> > i.e. stuff that can be displayed. Hence my first question: How can we
> > display these bastards here?
> 
> It's not meant to be displayed. The idea was maybe we could detect the
> fourcc (e.g. (fourcc & 0xff) == 'T') and reject the buffer accordingly
> when submitted to one of the display ioctl?

Well we already have explicit lists for all that, so no need to add a pile
of formats just to be able to reject them ;-)

> > So given all this: Why do we need this in the kernel's header? It sounds
> > like purely a userspace business with no need at all the enshrine this
> > into kernel ABI headers. Note that e.g. the mesa import/export interface
> > have their own fourcc #defines for exactly this reason.
> 
> I could have stuffed everything in gbm.h for instance, or another new
> header, but the EXT_dma_buf_import extension actually mentions
> drm_fourcc.h. So, that's why I finally moved the definitions to there.
> :)

That's a bit unfortunate ... But the spec also clearly states "as used by
the drm_mode_fb_cmd2 ioctl". And imo we can't make a case that this is
true.

> What would be the better place? Can we make the userspace libdrm
> version of the file live totally unsynchronized from the kernel
> headers then?

I think the right approach would be to ref the specification and also
allow other fourcc codes for non-display related buffer layaouts, maybe in
a different namespace. This entire fourcc stuff was only done since it was
a somewhat ill-defined "standard" for all kinds of YUV buffers. We've
already had to massivel pimp it to make it work properly with RGB.

Pimping this even further to support all kinds of random compute/gl
buffers sounds ill-advised. I think we need to steal a new namespace from
some existing standard for all this, maybe ogl or ocl has something? Ofc
that means creating a new dma_buf_import atttribute for eglCreateImageKHR
which could be used instead of DRM_FOURCC_EXT.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch