[RFC,dri3proto,01/16] Add modifier/multi-plane requests, bump to v1.1

Submitted by Daniel Stone on June 8, 2017, 6:43 p.m.

Details

Message ID 20170608184342.5168-2-daniels@collabora.com
State New
Headers show
Series "DRI4096: modifiers and multi-plane" ( rev: 1 ) in X.org

Not browsing as part of any series.

Commit Message

Daniel Stone June 8, 2017, 6:43 p.m.
DRI3 version 1.1 adds support for explicit format modifiers, including
multi-planar buffers.

Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 dri3proto.h | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 140 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/dri3proto.h b/dri3proto.h
index ceddee8..767f420 100644
--- a/dri3proto.h
+++ b/dri3proto.h
@@ -25,7 +25,7 @@ 
 
 #define DRI3_NAME			"DRI3"
 #define DRI3_MAJOR			1
-#define DRI3_MINOR			0
+#define DRI3_MINOR			1
 
 #define DRI3NumberErrors		0
 #define DRI3NumberEvents		0
@@ -37,7 +37,15 @@ 
 #define X_DRI3FenceFromFD               4
 #define X_DRI3FDFromFence               5
 
-#define DRI3NumberRequests		6
+/* v1.1 */
+#define xDRI3GetSupportedFormats	6
+#define xDRI3GetSupportedModifiers	7
+#define xDRI3PixmapFromBuffers		8
+#define xDRI3BuffersFromPixmap		9
+#define xDRI3FenceFromDMAFenceFD        10
+#define xDRI3DMAFenceFDFromFrence       11
+
+#define DRI3NumberRequests		12
 
 typedef struct {
     CARD8   reqType;
@@ -164,4 +172,134 @@  typedef struct {
 
 #define sz_xDRI3FDFromFenceReply   32
 
+/* v1.1 */
+
+typedef struct {
+    CARD8   reqType;
+    CARD8   dri3ReqType;
+    CARD16  length B16;
+    CARD32  window B32;
+} xDRI3GetSupportedFormatsReq;
+#define sz_xDRI3GetSupportedFormatsReq     8
+
+typedef struct {
+    BYTE    type;   /* X_Reply */
+    CARD8   pad1;
+    CARD16  sequenceNumber B16;
+    CARD32  length B32;
+    CARD32  numFormats B32;
+    CARD32  pad12 B32;
+    CARD32  pad16 B32;
+    CARD32  pad20 B32;
+    CARD32  pad24 B32;
+    CARD32  pad28 B32;
+} xDRI3GetSupportedFormatsReply;
+#define sz_xDRI3GetSupportedFormatsReply   32
+
+typedef struct {
+    CARD8   reqType;
+    CARD8   dri3ReqType;
+    CARD16  length B16;
+    CARD32  window B32;
+    CARD32  format B32;
+} xDRI3GetSupportedModifiersReq;
+#define sz_xDRI3GetSupportedModifiersReq     12
+
+typedef struct {
+    BYTE    type;   /* X_Reply */
+    CARD8   pad1;
+    CARD16  sequenceNumber B16;
+    CARD32  length B32;
+    CARD32  format B32;
+    CARD32  numModifiers B32;
+    CARD32  pad16 B32;
+    CARD32  pad20 B32;
+    CARD32  pad24 B32;
+    CARD32  pad28 B32;
+} xDRI3GetSupportedModifiersReply;
+#define sz_xDRI3GetSupportedModifiersReply   32
+
+typedef struct {
+    CARD8   reqType;
+    CARD8   dri3ReqType;
+    CARD16  length B16;
+    CARD32  pixmap B32;
+    CARD32  drawable B32;
+    CARD8   num_buffers; /* Number of file descriptors passed */
+    CARD8   pad13;
+    CARD16  pad14 B16;
+    CARD16  width B16;
+    CARD16  height B16;
+    CARD32  stride0 B32;
+    CARD32  offset0 B32;
+    CARD32  stride1 B32;
+    CARD32  offset1 B32;
+    CARD32  stride2 B32;
+    CARD32  offset2 B32;
+    CARD32  stride3 B32;
+    CARD32  offset3 B32;
+    CARD32  format B32;
+    CARD32  modifier_hi B32;
+    CARD32  modifier_lo B32;
+} xDRI3PixmapFromBuffersReq;
+#define sz_xDRI3PixmapFromBuffersReq 64
+
+typedef struct {
+    CARD8   reqType;
+    CARD8   dri3ReqType;
+    CARD16  length B16;
+    CARD32  pixmap B32;
+} xDRI3BuffersFromPixmapReq;
+#define sz_xDRI3BuffersFromPixmapReq     8
+
+typedef struct {
+    BYTE    type;   /* X_Reply */
+    CARD8   nfd;    /* Number of file descriptors returned */
+    CARD16  sequenceNumber B16;
+    CARD32  length B32;
+    CARD16  width B16;
+    CARD16  height B16;
+    CARD32  format B32;
+    CARD32  modifier_hi B32;
+    CARD32  modifier_lo B32;
+    CARD32  pad24 B32;
+    CARD32  pad28 B32;
+} xDRI3BuffersFromPixmapReply;
+#define sz_xDRI3BuffersFromPixmapReply   56
+
+typedef struct {
+    CARD8   reqType;
+    CARD8   dri3ReqType;
+    CARD16  length B16;
+    CARD32  drawable B32;
+    CARD32  fence B32;
+} xDRI3FenceFromDMAFenceFDReq;
+
+#define sz_xDRI3FenceFromDMAFenceFDReq  12
+
+typedef struct {
+    CARD8   reqType;
+    CARD8   dri3ReqType;
+    CARD16  length B16;
+    CARD32  drawable B32;
+    CARD32  fence B32;
+} xDRI3DMAFenceFDFromFenceReq;
+
+#define sz_xDRI3DMAFenceFDFromFenceReq  12
+
+typedef struct {
+    BYTE    type;   /* X_Reply */
+    CARD8   nfd;    /* Number of file descriptors returned (1) */
+    CARD16  sequenceNumber B16;
+    CARD32  length B32;
+    CARD32  pad08 B32;
+    CARD32  pad12 B32;
+    CARD32  pad16 B32;
+    CARD32  pad20 B32;
+    CARD32  pad24 B32;
+    CARD32  pad28 B32;
+} xDRI3DMAFenceFDFromFenceReply;
+
+#define sz_xDRI3DMAFenceFDFromFenceReply   32
+
 #endif

Comments

On Thu, 2017-06-08 at 19:43 +0100, Daniel Stone wrote:
> DRI3 version 1.1 adds support for explicit format modifiers, including
> multi-planar buffers.
> 
> Signed-off-by: Daniel Stone <daniels@collabora.com>
> ---
>  dri3proto.h | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 140 insertions(+), 2 deletions(-)

Should update dri3proto.txt too.

> +typedef struct {
> +    CARD8   reqType;
> +    CARD8   dri3ReqType;
> +    CARD16  length B16;
> +    CARD32  pixmap B32;
> +    CARD32  drawable B32;
> +    CARD8   num_buffers; /* Number of file descriptors passed */
> +    CARD8   pad13;
> +    CARD16  pad14 B16;
> +    CARD16  width B16;
> +    CARD16  height B16;
> +    CARD32  stride0 B32;
> +    CARD32  offset0 B32;
> +    CARD32  stride1 B32;
> +    CARD32  offset1 B32;
> +    CARD32  stride2 B32;
> +    CARD32  offset2 B32;
> +    CARD32  stride3 B32;
> +    CARD32  offset3 B32;
> +    CARD32  format B32;
> +    CARD32  modifier_hi B32;
> +    CARD32  modifier_lo B32;
> +} xDRI3PixmapFromBuffersReq;
> +#define sz_xDRI3PixmapFromBuffersReq 64

Why exactly four strides/offsets?

- ajax
Daniel Stone <daniels@collabora.com> writes:

> DRI3 version 1.1 adds support for explicit format modifiers, including
> multi-planar buffers.
>
> Signed-off-by: Daniel Stone <daniels@collabora.com>
> ---
>  dri3proto.h | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 140 insertions(+), 2 deletions(-)
>
> diff --git a/dri3proto.h b/dri3proto.h
> index ceddee8..767f420 100644
> --- a/dri3proto.h
> +++ b/dri3proto.h
> @@ -25,7 +25,7 @@
>  
>  #define DRI3_NAME			"DRI3"
>  #define DRI3_MAJOR			1
> -#define DRI3_MINOR			0
> +#define DRI3_MINOR			1
>  
>  #define DRI3NumberErrors		0
>  #define DRI3NumberEvents		0
> @@ -37,7 +37,15 @@
>  #define X_DRI3FenceFromFD               4
>  #define X_DRI3FDFromFence               5
>  
> -#define DRI3NumberRequests		6
> +/* v1.1 */
> +#define xDRI3GetSupportedFormats	6
> +#define xDRI3GetSupportedModifiers	7
> +#define xDRI3PixmapFromBuffers		8
> +#define xDRI3BuffersFromPixmap		9
> +#define xDRI3FenceFromDMAFenceFD        10
> +#define xDRI3DMAFenceFDFromFrence       11
> +
> +#define DRI3NumberRequests		12
>  
>  typedef struct {
>      CARD8   reqType;
> @@ -164,4 +172,134 @@ typedef struct {
>  
>  #define sz_xDRI3FDFromFenceReply   32
>  
> +/* v1.1 */
> +
> +typedef struct {
> +    CARD8   reqType;
> +    CARD8   dri3ReqType;
> +    CARD16  length B16;
> +    CARD32  window B32;
> +} xDRI3GetSupportedFormatsReq;
> +#define sz_xDRI3GetSupportedFormatsReq     8
> +
> +typedef struct {
> +    BYTE    type;   /* X_Reply */
> +    CARD8   pad1;
> +    CARD16  sequenceNumber B16;
> +    CARD32  length B32;
> +    CARD32  numFormats B32;
> +    CARD32  pad12 B32;
> +    CARD32  pad16 B32;
> +    CARD32  pad20 B32;
> +    CARD32  pad24 B32;
> +    CARD32  pad28 B32;
> +} xDRI3GetSupportedFormatsReply;
> +#define sz_xDRI3GetSupportedFormatsReply   32
> +
> +typedef struct {
> +    CARD8   reqType;
> +    CARD8   dri3ReqType;
> +    CARD16  length B16;
> +    CARD32  window B32;
> +    CARD32  format B32;
> +} xDRI3GetSupportedModifiersReq;
> +#define sz_xDRI3GetSupportedModifiersReq     12
> +
> +typedef struct {
> +    BYTE    type;   /* X_Reply */
> +    CARD8   pad1;
> +    CARD16  sequenceNumber B16;
> +    CARD32  length B32;
> +    CARD32  format B32;
> +    CARD32  numModifiers B32;
> +    CARD32  pad16 B32;
> +    CARD32  pad20 B32;
> +    CARD32  pad24 B32;
> +    CARD32  pad28 B32;
> +} xDRI3GetSupportedModifiersReply;
> +#define sz_xDRI3GetSupportedModifiersReply   32
> +
> +typedef struct {
> +    CARD8   reqType;
> +    CARD8   dri3ReqType;
> +    CARD16  length B16;
> +    CARD32  pixmap B32;
> +    CARD32  drawable B32;
> +    CARD8   num_buffers; /* Number of file descriptors passed */
> +    CARD8   pad13;
> +    CARD16  pad14 B16;
> +    CARD16  width B16;
> +    CARD16  height B16;
> +    CARD32  stride0 B32;
> +    CARD32  offset0 B32;
> +    CARD32  stride1 B32;
> +    CARD32  offset1 B32;
> +    CARD32  stride2 B32;
> +    CARD32  offset2 B32;
> +    CARD32  stride3 B32;
> +    CARD32  offset3 B32;
> +    CARD32  format B32;
> +    CARD32  modifier_hi B32;
> +    CARD32  modifier_lo B32;

With the Present extension, we started putting CARD64s on the wire.
Let's use them here, too.

I don't really have other comments about the protocol yet, without the
documentation.
On 17 June 2017 at 02:21, Eric Anholt <eric@anholt.net> wrote:
> With the Present extension, we started putting CARD64s on the wire.
> Let's use them here, too.

Mm, we did have that originally, but anyone including XSync headers
very helpfully gets #define CARD64 XSyncValue, which blows up into int
+ unsigned int. This includes dri3_request.c, which pulls in syncsrv.h
-> misyncstr.h:
../../dri3/dri3_request.c: In function ‘proc_dri3_get_supported_modifiers’:
../../dri3/dri3_request.c:407:23: error: invalid initializer
     CARD64 modifier = 0;
                       ^
../../dri3/dri3_request.c:431:6: error: conversion to non-scalar type requested
      modifier = (CARD64) (modifiers_hi[i] << 32);
      ^~~~~~~~

I'd originally thought that came from the server-only part of
xexxtproto/syncstr.h, but turns out it's actually just in
miext/sync/misyncstr.h. So I guess we could run s/CARD64/XSyncValue/
across sync, but it would (at least theoretically) be an API break.

Cheers,
Daniel
On 13 June 2017 at 21:43, Adam Jackson <ajax@nwnk.net> wrote:
> On Thu, 2017-06-08 at 19:43 +0100, Daniel Stone wrote:
>> DRI3 version 1.1 adds support for explicit format modifiers, including
>> multi-planar buffers.
>>
>> Signed-off-by: Daniel Stone <daniels@collabora.com>
>> ---
>>  dri3proto.h | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 140 insertions(+), 2 deletions(-)
>
> Should update dri3proto.txt too.

Good point, will be fixed for v2.

>> +typedef struct {
>> +    [...]
>> +    CARD32  stride0 B32;
>> +    CARD32  offset0 B32;
>> +    CARD32  stride1 B32;
>> +    CARD32  offset1 B32;
>> +    CARD32  stride2 B32;
>> +    CARD32  offset2 B32;
>> +    CARD32  stride3 B32;
>> +    CARD32  offset3 B32;
>> +    [...]
>> +} xDRI3PixmapFromBuffersReq;
>> +#define sz_xDRI3PixmapFromBuffersReq 64
>
> Why exactly four strides/offsets?

Because that's what KMS takes for AddFB2, and (not coincidentally)
what EGL_EXT_image_dma_buf_import_modifiers also defines tokens for. I
don't even know of anyone using four, which would be tri-planar Y/U/V
with an auxiliary plane. But seemed prudent to line up with the
external APIs.

Cheers,
Daniel
On Mon, 2017-06-19 at 13:35 +0100, Daniel Stone wrote:
> On 13 June 2017 at 21:43, Adam Jackson <ajax@nwnk.net> wrote:
> > On Thu, 2017-06-08 at 19:43 +0100, Daniel Stone wrote:
> > > 
> > > +typedef struct {
> > > +    [...]
> > > +    CARD32  stride0 B32;
> > > +    CARD32  offset0 B32;
> > > +    CARD32  stride1 B32;
> > > +    CARD32  offset1 B32;
> > > +    CARD32  stride2 B32;
> > > +    CARD32  offset2 B32;
> > > +    CARD32  stride3 B32;
> > > +    CARD32  offset3 B32;
> > > +    [...]
> > > +} xDRI3PixmapFromBuffersReq;
> > > +#define sz_xDRI3PixmapFromBuffersReq 64
> > 
> > Why exactly four strides/offsets?
> 
> Because that's what KMS takes for AddFB2, and (not coincidentally)
> what EGL_EXT_image_dma_buf_import_modifiers also defines tokens for. I
> don't even know of anyone using four, which would be tri-planar Y/U/V
> with an auxiliary plane. But seemed prudent to line up with the
> external APIs.

Aux plane for alpha or transparency key maybe? But thanks, that
clarifies the intent for me. I wasn't sure whether the multiple buffers
were for front/back and left/right for stereo or something. If they're
for planar color then 4 is plenty unless someone wants like CMYKA or
something silly like that.

- ajax
Hi,
Sorry for top-posting but formatting seems to get mangled with the phone
client lately, so.

'Aux plane for alpha or transparency key maybe?' - I should've clarified
aux plane usage earlier. Generally it's used as a compression status buffer
(per tile - or super/super-super tile? - if it's solid or the full tile
needs to be streamed), as well as things like a clear colour. That's how
Intel's CCS handles it, and I understand NV do the same. There's another
compression scheme from another vendor I'm not at liberty to detail, but
it's similar enough wrt this protocol.

AYUV I've only seen in the form of the FourCC of the same name, packed into
32bpp, single plane with no subsampling. I've not heard of an I420a or even
NV12a. So I'll grant you that for RGB, two is enough: single plane colour,
auxiliary buffer compression. But might as well just bite the bullet and
make YUV a bit less insurmountable at the same time.

I'll make sure to cover that in the dri3proto.txt update - thanks!

Cheers,
Daniel

On Mon, 19 Jun 2017 at 7:50 pm, Adam Jackson <ajax@nwnk.net> wrote:

> On Mon, 2017-06-19 at 13:35 +0100, Daniel Stone wrote:
> > On 13 June 2017 at 21:43, Adam Jackson <ajax@nwnk.net> wrote:
> > > On Thu, 2017-06-08 at 19:43 +0100, Daniel Stone wrote:
> > > >
> > > > +typedef struct {
> > > > +    [...]
> > > > +    CARD32  stride0 B32;
> > > > +    CARD32  offset0 B32;
> > > > +    CARD32  stride1 B32;
> > > > +    CARD32  offset1 B32;
> > > > +    CARD32  stride2 B32;
> > > > +    CARD32  offset2 B32;
> > > > +    CARD32  stride3 B32;
> > > > +    CARD32  offset3 B32;
> > > > +    [...]
> > > > +} xDRI3PixmapFromBuffersReq;
> > > > +#define sz_xDRI3PixmapFromBuffersReq 64
> > >
> > > Why exactly four strides/offsets?
> >
> > Because that's what KMS takes for AddFB2, and (not coincidentally)
> > what EGL_EXT_image_dma_buf_import_modifiers also defines tokens for. I
> > don't even know of anyone using four, which would be tri-planar Y/U/V
> > with an auxiliary plane. But seemed prudent to line up with the
> > external APIs.
>
> Aux plane for alpha or transparency key maybe? But thanks, that
> clarifies the intent for me. I wasn't sure whether the multiple buffers
> were for front/back and left/right for stereo or something. If they're
> for planar color then 4 is plenty unless someone wants like CMYKA or
> something silly like that.
>
> - ajax
>
Daniel Stone <daniel@fooishbar.org> writes:

> On 17 June 2017 at 02:21, Eric Anholt <eric@anholt.net> wrote:
>> With the Present extension, we started putting CARD64s on the wire.
>> Let's use them here, too.
>
> Mm, we did have that originally, but anyone including XSync headers
> very helpfully gets #define CARD64 XSyncValue, which blows up into int
> + unsigned int. This includes dri3_request.c, which pulls in syncsrv.h
> -> misyncstr.h:
> ../../dri3/dri3_request.c: In function ‘proc_dri3_get_supported_modifiers’:
> ../../dri3/dri3_request.c:407:23: error: invalid initializer
>      CARD64 modifier = 0;
>                        ^
> ../../dri3/dri3_request.c:431:6: error: conversion to non-scalar type requested
>       modifier = (CARD64) (modifiers_hi[i] << 32);
>       ^~~~~~~~
>
> I'd originally thought that came from the server-only part of
> xexxtproto/syncstr.h, but turns out it's actually just in
> miext/sync/misyncstr.h. So I guess we could run s/CARD64/XSyncValue/
> across sync, but it would (at least theoretically) be an API break.

I took a look at that, and it seems like a good plan.  Would you be up
for taking care of it, or should I put it on my list?

(Though, really, I wish CARD* would die in a fire.  We have stdint now.)
On 25 July 2017 at 21:43, Eric Anholt <eric@anholt.net> wrote:
> Daniel Stone <daniel@fooishbar.org> writes:
>> Mm, we did have that originally, but anyone including XSync headers
>> very helpfully gets #define CARD64 XSyncValue, which blows up into int
>> + unsigned int. [...]
>>
>> I'd originally thought that came from the server-only part of
>> xexxtproto/syncstr.h, but turns out it's actually just in
>> miext/sync/misyncstr.h. So I guess we could run s/CARD64/XSyncValue/
>> across sync, but it would (at least theoretically) be an API break.
>
> I took a look at that, and it seems like a good plan.  Would you be up
> for taking care of it, or should I put it on my list?

I'm juggling too many plates/projects/travels at the moment, so if you
could please take it that'd be great!

> (Though, really, I wish CARD* would die in a fire.  We have stdint now.)

No argument here ...

Cheers,
Daniel