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

[Keith Packard]

Daniel Stone <daniel at fooishbar.org> writes:

> Oof. Originally we tried to match the rest of the stack by using
> formats rather than depth/bpp, but that rather breaks the current
> model where DRI3 is a transport for a bag of bits, and Present is the
> mechanism by which you make those bag of bits visible in a Window.

Yeah, thanks for the good discussion, I was just poking at the
boundaries here, and I now agree that depth+bpp is probably our best
option. Including bpp lets us go beyond the core protocol and use
formats which are different from image formats, and that seems like a
good idea.

> Hm, why would they? For DirectColour, presumably either your only
> difference from a hardware-accelerated TrueColor source is that you
> stick a huge LUT at the end of the frag shader, or you have a software
> pipeline.

Nah, DirectColor has a LUT in the scanout engine so you can do colormap
animation effects. As I said, it's not exactly popular these days for
very good reasons.

>>> of the right format before using DRI3PixmapFromBuffers.
>>
>> Like was mentionned by Daniel on #xorg-devel, we really need to have
>> the 'drawable' here and can't easily retrieve the drawable-specific
>> modifiers just from the 'depth'. (We check if the window is a possible
>> candidate for scanout to avoid useless client buffer reallocation).

Yup. I see you're mixing 'drawable' and 'window' freely here; I'd like
to be consistent. I think using 'window' would be best as then we
can explicitly document the expectation of using the resulting pixmap in
a PresentPixmap request for the specified window.


>>
>> We don't explicitely state that in the spec though to be as general as
>> possible. I guess it wouldn't hurt to add an example though if you
>> think it would make things clearer.
>
> Maybe something like:
>   Return supported DRM FourCC modifiers for the specified 'window'.
>
>   The first list of 'drawable_modifiers' contains a set of modifiers which
>   the server considers optimal for the window's current configuration.
>   Using these modifiers to allocate, even if locally suboptimal to the
>   client driver, may result in a more optimal display pipeline, e.g. by
>   avoiding composition.

I think this might mention the expected use of the resulting pixmap in a later
PresentPixmap request; that makes the connection between the 'window'
parameter and the created pixmap explicit.

>> Do you mean adding a new synchronization mechanism (between X rendering
>> and client rendering) that would only apply to this request (not
>> PixapFromBuffer)? Was it considered problematic with DRI3 v1.0?

No, I'd love to see a section describing the synchronization available
to all objects created through this extension.

> DRI3 v1.2 - sent to the list in August but with very little comment -
> adds support for dma-fence FDs. Michel questioned whether it would be
> best to insert a GPU command-stream stall, or use an asynchronous
> fence-completion mechanism to only later trigger presentation when the
> fence has passed, similar to if the client had specified that
> presentation should wait for a UST equivalent to when the fence was
> signalled. My intuition is that he's right, but it would be great to
> get more feedback on this.
>
> Louis-Francis hooked this up to the Vulkan WSI in Mesa, so you could
> use real semaphore objects in QueuePresent and AcquireNextImage.
>
> https://lists.freedesktop.org/archives/xorg-devel/2017-August/054439.html
> https://lists.freedesktop.org/archives/mesa-dev/2017-August/168201.html

Those look like reasonable patches on a first read-through; I was a bit
confused by the lack of any way for the server to wakeup when the DMA
fence became readable? Does it not need to be able to do that in case
the driver doesn't deal with it?

>>> > +   DRM_FORMAT_MOD_INVALID may be passed for 'modifier', in
>>> > which
>>> > +   case the driver may make its own inference as to the exact
>>> > +   layout of the buffer(s).
>>>
>>> Presumably using information acquired externally?
>>
>> Or internally; some drivers add metadata to the buffer to know about
>> tiling, etc.

I'm not terribly excited about this name; is there some way we could
make usage clearer in code?
DRM_FORMAT_MOD_FIGURE_IT_OUT_AUTOMATICALLY_SOMEHOW seems a bit long
though?

> Since that's just advisory
> text, we could explicitly make it 'undefined', or just leave it
> completely unstated, or ...

Well, I'd love to have this usage documented in the protocol spec so
that when people read the code there's someplace that says what it
means. And that would be more clear if we used a constant with a better
name than INVALID.

>   Synchronization of access to buffers shared between processes is not
>   defined by this protocol [ed: well, until DRI3 v1.2 adds explicit fencing].
>   Without the use of additional extensions not defined by the DRI3
>   protocol as of version 1.1, synchronization between multiple processes
>   and contexts is considered to follow the implicit model.
>
>   In this model, the underlying driver is responsible for enforcing a strict
>   ordering such that any reads requested by one process or context are
>   fulfilled before any writes requested by another process or context, as
>   long as that read was definitively submitted before the write (e.g.
>   a through a blocking read command such as glReadPixels or explicitly
>   flushing the command stream through glFlush). A similar dependency
>   exists for reads submitted after writes: the driver must ensure that the
>   write is fully visible and coherent to the read request.
>
>   This restriction also applies for cross-device usage.

Yes, a section like this would be awesome. Add it as a top-level section
describing synchronization in the protocol.

As an X extension, I think we should be more focused on X APIs instead
of higher level abstractions (like GL or Vulkan). In particular, I think
we need things like this:

        When Damage events for a pixmap are received by a client,
        changes made through the X protocol will be visible in any
        buffers associated through this extension.

        Clients must ensure that all drawing done to buffers outside of
        X are visible in those buffers before sending X requests
        including a pixmap associated with them.

As far as the bare protocol and implementation of the extension, it all
looks good at this point. The synchronization stuff is just an old bug
that needs fixing.

-- 
-keith
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20180227/c6c365ab/attachment.sig>