[PATCH wayland-protocols v6] Add zwp_linux_explicit_synchronization_v1

[Alexandros Frantzis]

On Thu, Nov 08, 2018 at 05:43:42PM +0200, Pekka Paalanen wrote:
> On Thu, 8 Nov 2018 16:34:52 +0200
> Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:
> 
> > On Thu, Nov 08, 2018 at 03:00:58PM +0200, Pekka Paalanen wrote:
> > > On Wed, 07 Nov 2018 20:22:38 +0000
> > > Simon Ser <contact at emersion.fr> wrote:
> > >   
> > > > Thanks! With these fixes, this is now
> > > > 
> > > > Reviewed-by: Simon Ser <contact at emersion.fr>
> > > > 
> > > > Below is a small comment, I don't know if it's relevant or not, I just wanted to
> > > > point it out.
> > > >   
> > 
> > ...
> > 
> > > > > +  <interface name="zwp_surface_synchronization_v1" version="1">    
> > > > 
> > > > I missed this before, but: is there a reason why the "linux" prefix has been
> > > > dropped here? Maybe it should be renamed to
> > > > zwp_linux_surface_synchronization_v1? What about zwp_buffer_release_v1?  
> > > 
> > > That's a good question, because these names are kind of global. Not
> > > really global, but it could cause some name conflicts if the same
> > > program or a compilation unit needed to use two different but
> > > same-named interfaces. They are less global than the same of the global
> > > interface though, which needs to be unique per platform for real.
> > > 
> > > The stable names would be wp_surface_synchronization and
> > > wp_buffer_release, with the root interface being
> > > wp_linux_explicit_synchronization.
> > > 
> > > The dmabuf extension relies of the Linux definition of a dmabuf. This
> > > extension relies on the Linux definition of a fence, AFAIU.
> > > 
> > > So yes, I think repeating "linux" in the interface names would be
> > > appropriate.  
> > 
> > Hi Pekka and Simon,
> > 
> > adding the linux_ prefix is reasonable (and means we could be winning at
> > the longest interface name competition :)).
> > 
> > This got me thinking (also see Daniel's email), though, if there would
> > be benefit in moving in the other direction: making this protocol
> > agnostic of the fence type details. So, instead of
> > "zwp_linux_explicit_synchronization_v1" we will have
> > "zwp_explicit_synchronization_v1" with the passed fd being an opaque fd
> > from a protocol perspective.
> > 
> > Of course, this reintroduces the problem of how the client can figure
> > out what kind of buffer/fence combinations the server can accept, which
> > we have resolved here by limiting this protocol to dmabuf/dma_fences.
> > One way would be to allow per-platform protocols that just advertise
> > supported combinations, e.g., having
> > "zwp_linux_explicit_synchronization_v1" just means that the
> > dmabuf/dmafence combo is supported for zwp_explicit_synchronization
> > operations.
> > 
> > Finally, if we think an opaque fd may be limiting, there are other
> > options to explore (e.g., a zwp_fence interface with factories in
> > per-platform extensions).
> 
> Hi Alf,
> 
> I'd keep it simple. Leave out other fence types until we have a use
> case. It's still an unstable protocol, and we can see if other types
> appear by the time people want to declare it stable.
> 
> An opaque fd is kind of useless. If the spec does not say what it is,
> how could a compositor or a client do anything with it?

...

The idea is to have something akin to how the EGL_KHR_image_base and
EGL_KHR_image_* extensions are structured.

So, the base protocol, wp_explicit_synchronization, would just define
the operations but not any acceptable fence types. Then we could have
platform-specific protocols like
wp_{linux,dmafence,syncobj...}_explicit_synchronization, which would
just state what the acceptable fence types are, and possibly which
buffer types these fences can be associated with. Their presence would
let the client know how to create the fence. In order to have a usable
implementation, the server will need to expose at least one of these
platform-specific protocols.

As for the server detecting the fence fd type internally, in case
multiple fence types are supported, we can express this in terms of
checking for fence validity, which we already need in order to support
the INVALID_FENCE error.

Even if this is overkill for now, I think it's a viable design to keep
in mind if in the future we need to support multiple fence types.

> For simplicity, I'd go with "linux" in the name and the explicit
> connection with linux_dmabuf buffers.

Fine with me.

Thanks,
Alexandros