[wayland-protocols,v6] Add zwp_linux_explicit_synchronization_v1

Submitted by Alexandros Frantzis on Nov. 7, 2018, 2:59 p.m.

Details

Message ID 20181107145925.29627-1-alexandros.frantzis@collabora.com
State New
Series "Add zwp_linux_explicit_synchronization_v1"
Headers show

Commit Message

Alexandros Frantzis Nov. 7, 2018, 2:59 p.m.
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
---

Changes in patch v6:
  - Fixed wl_buffer.attach -> wl_surface.attach typos.
  - Added NO_BUFFER error and updated request descriptions.
  - Consistently used "error is raised" phrasing.
  - Added comment about buffer_release object destruction in event
    descriptions.

Changes in patch v5:
  - Further clarified the per-commit nature of buffer_release.
  - Used the wp_linux_dmabuf name to refer to all versions of that protocol.
  - Fixed wl_buffer.attach typo.
  - Clarified when an INVALID_FENCE error is raised.
  - Improved wording explaining the double-buffer state of buffer_release.
  - Allowed get_release requests on all non-null buffers.
  - Clarified that the compositor is free to select buffer_release events
    on a release by release basis.
  - Changed buffer_release to be self-destroyed after it emits an event.

Changes in patch v4:
  - Guaranteed protocol compatibility only with zwp_linux_dmabuf buffers.
  - Added the UNSUPPORTED_BUFFER error.

Changes in patch v3:
  - Reworded implicit/explicit synchronization intro in
    zwp_surface_synchronization_v1 description.
  - Removed confusing mention of wl_buffer.release in
    zwp_surface_synchronization_v1 description.
  - Clarified which fences are affected on sync object destruction.
  - Removed unclear mention about wl_buffer destruction
    in fenced_release description.
  - Clarified that the release events and their guarantees apply to
    the relevant commit only.
  - Reformatted text.

Changes in patch v2:
  - Added NO_SURFACE error for zwp_surface_synchronization_v1 requests.
  - Removed restriction to destroy a zwp_surface_synchronization_v1 object
    after the associated wl_surface is destroyed.
  - Clarified which buffer the acquire fence is associated with.
  - Clarified that exactly one event, either a fenced_release or a
    immediate_release, will be emitted for each commit.

 Makefile.am                                   |   1 +
 .../linux-explicit-synchronization/README     |   6 +
 ...x-explicit-synchronization-unstable-v1.xml | 234 ++++++++++++++++++
 3 files changed, 241 insertions(+)
 create mode 100644 unstable/linux-explicit-synchronization/README
 create mode 100644 unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml

Patch hide | download patch | download mbox

diff --git a/Makefile.am b/Makefile.am
index 6394e26..7dfbb9e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -21,6 +21,7 @@  unstable_protocols =								\
 	unstable/xdg-output/xdg-output-unstable-v1.xml				\
 	unstable/input-timestamps/input-timestamps-unstable-v1.xml	\
 	unstable/xdg-decoration/xdg-decoration-unstable-v1.xml	\
+	unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml \
 	$(NULL)
 
 stable_protocols =								\
diff --git a/unstable/linux-explicit-synchronization/README b/unstable/linux-explicit-synchronization/README
new file mode 100644
index 0000000..f13b404
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/README
@@ -0,0 +1,6 @@ 
+Linux explicit synchronization (dma-fence) protocol
+
+Maintainers:
+David Reveman <reveman@chromium.org>
+Daniel Stone <daniels@collabora.com>
+Alexandros Frantzis <alexandros.frantzis@collabora.com>
diff --git a/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
new file mode 100644
index 0000000..b87663f
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
@@ -0,0 +1,234 @@ 
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="zwp_linux_explicit_synchronization_unstable_v1">
+
+  <copyright>
+    Copyright 2016 The Chromium Authors.
+    Copyright 2017 Intel Corporation
+    Copyright 2018 Collabora, Ltd
+
+    Permission is hereby granted, free of charge, to any person obtaining a
+    copy of this software and associated documentation files (the "Software"),
+    to deal in the Software without restriction, including without limitation
+    the rights to use, copy, modify, merge, publish, distribute, sublicense,
+    and/or sell copies of the Software, and to permit persons to whom the
+    Software is furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice (including the next
+    paragraph) shall be included in all copies or substantial portions of the
+    Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+    DEALINGS IN THE SOFTWARE.
+  </copyright>
+
+  <interface name="zwp_linux_explicit_synchronization_v1" version="1">
+    <description summary="protocol for providing explicit synchronization">
+      This global is a factory interface, allowing clients to request
+      explicit synchronization for buffers on a per-surface basis.
+
+      See zwp_surface_synchronization_v1 for more information.
+
+      This interface is derived from Chromium's
+      zcr_linux_explicit_synchronization_v1.
+
+      Warning! The protocol described in this file is experimental and
+      backward incompatible changes may be made. Backward compatible changes
+      may be added together with the corresponding interface version bump.
+      Backward incompatible changes are done by bumping the version number in
+      the protocol and interface names and resetting the interface version.
+      Once the protocol is to be declared stable, the 'z' prefix and the
+      version number in the protocol and interface names are removed and the
+      interface version number is reset.
+    </description>
+
+    <request name="destroy" type="destructor">
+      <description summary="destroy explicit synchronization factory object">
+        Destroy this explicit synchronization factory object. Other objects,
+        including zwp_surface_synchronization_v1 objects created by this
+        factory, shall not be affected by this request.
+      </description>
+    </request>
+
+    <enum name="error">
+      <entry name="synchronization_exists" value="0"
+             summary="the surface already has a synchronization object associated"/>
+    </enum>
+
+    <request name="get_synchronization">
+      <description summary="extend surface interface for explicit synchronization">
+        Instantiate an interface extension for the given wl_surface to provide
+        explicit synchronization.
+
+        If the given wl_surface already has an explicit synchronization object
+        associated, the synchronization_exists protocol error is raised.
+      </description>
+
+      <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
+           summary="the new synchronization interface id"/>
+      <arg name="surface" type="object" interface="wl_surface"
+           summary="the surface"/>
+    </request>
+  </interface>
+
+  <interface name="zwp_surface_synchronization_v1" version="1">
+    <description summary="per-surface explicit synchronization support">
+      This object implements per-surface explicit synchronization.
+
+      Synchronization refers to co-ordination of pipelined operations performed
+      on buffers. Most GPU clients will schedule an asynchronous operation to
+      render to the buffer, then immediately send the buffer to the compositor
+      to be attached to a surface.
+
+      In implicit synchronization, ensuring that the rendering operation is
+      complete before the compositor displays the buffer is an implementation
+      detail handled by either the kernel or userspace graphics driver.
+
+      By contrast, in explicit synchronization, dma_fence objects mark when the
+      asynchronous operations are complete. When submitting a buffer, the
+      client provides an acquire fence which will be waited on before the
+      compositor accesses the buffer. The Wayland server, through a
+      zwp_buffer_release_v1 object, will inform the client with an event which
+      may be accompanied by a release fence, when the compositor will no longer
+      access the buffer contents due to the specific commit that requested the
+      release event.
+
+      Each surface can be associated with only one object of this interface at
+      any time.
+
+      Explicit synchronization is guaranteed to be supported only for buffers
+      created with any version of the wp_linux_dmabuf buffer factory.
+    </description>
+
+    <request name="destroy" type="destructor">
+      <description summary="destroy synchronization object">
+        Destroy this explicit synchronization object.
+
+        Any fence set by this object with set_acquire_fence since the last
+        commit will be discarded by the server. Any fences set by this object
+        before the last commit are not affected.
+
+        zwp_buffer_release_v1 objects created by this object are not affected
+        by this request.
+      </description>
+    </request>
+
+    <enum name="error">
+      <entry name="invalid_fence" value="0"
+             summary="the fence specified by the client could not be imported"/>
+      <entry name="duplicate_fence" value="1"
+             summary="multiple fences added for a single surface commit"/>
+      <entry name="duplicate_release" value="2"
+             summary="multiple releases added for a single surface commit"/>
+      <entry name="no_surface" value="3"
+             summary="the associated wl_surface was destroyed"/>
+      <entry name="unsupported_buffer" value="4"
+             summary="the buffer does not support explicit synchronization"/>
+      <entry name="no_buffer" value="5"
+             summary="no buffer was attached"/>
+    </enum>
+
+    <request name="set_acquire_fence">
+      <description summary="set the acquire fence">
+        Set the acquire fence that must be signaled before the compositor
+        may sample from the buffer attached with wl_surface.attach. The fence
+        is a dma_fence kernel object.
+
+        The acquire fence is double-buffered state, and will be applied on the
+        next wl_surface.commit request for the associated surface. Thus, it
+        applies only to the buffer that is attached to the surface at commit
+        time.
+
+        If the provided fd is not a valid dma_fence fd, then an INVALID_FENCE
+        error is raised.
+
+        If a fence has already been attached during the same commit cycle, a
+        DUPLICATE_FENCE error is raised.
+
+        If the associated wl_surface was destroyed, a NO_SURFACE error is
+        raised.
+
+        If at surface commit time the attached buffer does not support explicit
+        synchronization, an UNSUPPORTED_BUFFER error is raised.
+
+        If at surface commit time there is no buffer attached, a NO_BUFFER
+        error is raised.
+      </description>
+      <arg name="fd" type="fd" summary="acquire fence fd"/>
+    </request>
+
+    <request name="get_release">
+      <description summary="release fence for last-attached buffer">
+        Create a listener for the release of the buffer attached by the
+        client with wl_surface.attach. See zwp_buffer_release_v1
+        documentation for more information.
+
+        The release object is double-buffered state, and will be associated
+        with the buffer that is attached to the surface at wl_surface.commit
+        time.
+
+        If a zwp_buffer_release_v1 object has already been requested for
+        the surface in the same commit cycle, a DUPLICATE_RELEASE error
+        is raised.
+
+        If the associated wl_surface was destroyed, a NO_SURFACE error
+        is raised.
+
+        If at surface commit time there is no buffer attached, a NO_BUFFER
+        error is raised.
+      </description>
+      <arg name="release" type="new_id" interface="zwp_buffer_release_v1"
+           summary="new zwp_buffer_release_v1 object"/>
+    </request>
+  </interface>
+
+  <interface name="zwp_buffer_release_v1" version="1">
+    <description summary="buffer release explicit synchronization">
+      This object is instantiated in response to a
+      zwp_surface_synchronization_v1.get_release request.
+
+      It provides an alternative to wl_buffer.release events, providing a
+      unique release from a single wl_surface.commit request. The release event
+      also supports explicit synchronization, providing a fence FD for the
+      client to synchronize against.
+
+      Exactly one event, either a fenced_release or an immediate_release, will
+      be emitted for the wl_surface.commit request. The compositor can choose
+      release by release which event it uses.
+
+      This event does not replace wl_buffer.release events; servers are still
+      required to send those events.
+
+      Once a buffer release object has delivered a 'fenced_release' or an
+      'immediate_release' event it is automatically destroyed.
+    </description>
+
+    <event name="fenced_release">
+      <description summary="release buffer with fence">
+        Sent when the compositor has finalised its usage of the associated
+        buffer for the relevant commit, providing a dma_fence which will be
+        signaled when all operations by the compositor on that buffer for that
+        commit have finished.
+
+        This event destroys the zwp_buffer_release_v1 object.
+      </description>
+      <arg name="fence" type="fd" summary="fence for last operation on buffer"/>
+    </event>
+
+    <event name="immediate_release">
+      <description summary="release buffer immediately">
+        Sent when the compositor has finalised its usage of the associated
+        buffer for the relevant commit, and either performed no operations
+        using it, or has a guarantee that all its operations on that buffer for
+        that commit have finished.
+
+        This event destroys the zwp_buffer_release_v1 object.
+      </description>
+    </event>
+  </interface>
+
+</protocol>

Comments

Simon Ser Nov. 7, 2018, 8:22 p.m.
Thanks! With these fixes, this is now

Reviewed-by: Simon Ser <contact@emersion.fr>

Below is a small comment, I don't know if it's relevant or not, I just wanted to
point it out.

> Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
> ---
>
> Changes in patch v6:
>   - Fixed wl_buffer.attach -> wl_surface.attach typos.
>   - Added NO_BUFFER error and updated request descriptions.
>   - Consistently used "error is raised" phrasing.
>   - Added comment about buffer_release object destruction in event
>     descriptions.
>
> Changes in patch v5:
>   - Further clarified the per-commit nature of buffer_release.
>   - Used the wp_linux_dmabuf name to refer to all versions of that protocol.
>   - Fixed wl_buffer.attach typo.
>   - Clarified when an INVALID_FENCE error is raised.
>   - Improved wording explaining the double-buffer state of buffer_release.
>   - Allowed get_release requests on all non-null buffers.
>   - Clarified that the compositor is free to select buffer_release events
>     on a release by release basis.
>   - Changed buffer_release to be self-destroyed after it emits an event.
>
> Changes in patch v4:
>   - Guaranteed protocol compatibility only with zwp_linux_dmabuf buffers.
>   - Added the UNSUPPORTED_BUFFER error.
>
> Changes in patch v3:
>   - Reworded implicit/explicit synchronization intro in
>     zwp_surface_synchronization_v1 description.
>   - Removed confusing mention of wl_buffer.release in
>     zwp_surface_synchronization_v1 description.
>   - Clarified which fences are affected on sync object destruction.
>   - Removed unclear mention about wl_buffer destruction
>     in fenced_release description.
>   - Clarified that the release events and their guarantees apply to
>     the relevant commit only.
>   - Reformatted text.
>
> Changes in patch v2:
>   - Added NO_SURFACE error for zwp_surface_synchronization_v1 requests.
>   - Removed restriction to destroy a zwp_surface_synchronization_v1 object
>     after the associated wl_surface is destroyed.
>   - Clarified which buffer the acquire fence is associated with.
>   - Clarified that exactly one event, either a fenced_release or a
>     immediate_release, will be emitted for each commit.
>
>  Makefile.am                                   |   1 +
>  .../linux-explicit-synchronization/README     |   6 +
>  ...x-explicit-synchronization-unstable-v1.xml | 234 ++++++++++++++++++
>  3 files changed, 241 insertions(+)
>  create mode 100644 unstable/linux-explicit-synchronization/README
>  create mode 100644 unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
>
> diff --git a/Makefile.am b/Makefile.am
> index 6394e26..7dfbb9e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -21,6 +21,7 @@ unstable_protocols =								\
>  	unstable/xdg-output/xdg-output-unstable-v1.xml				\
>  	unstable/input-timestamps/input-timestamps-unstable-v1.xml	\
>  	unstable/xdg-decoration/xdg-decoration-unstable-v1.xml	\
> +	unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml \
>  	$(NULL)
>
>  stable_protocols =								\
> diff --git a/unstable/linux-explicit-synchronization/README b/unstable/linux-explicit-synchronization/README
> new file mode 100644
> index 0000000..f13b404
> --- /dev/null
> +++ b/unstable/linux-explicit-synchronization/README
> @@ -0,0 +1,6 @@
> +Linux explicit synchronization (dma-fence) protocol
> +
> +Maintainers:
> +David Reveman <reveman@chromium.org>
> +Daniel Stone <daniels@collabora.com>
> +Alexandros Frantzis <alexandros.frantzis@collabora.com>
diff --git a/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml > b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> new file mode 100644
> index 0000000..b87663f
> --- /dev/null
> +++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> @@ -0,0 +1,234 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="zwp_linux_explicit_synchronization_unstable_v1">
> +
> +  <copyright>
> +    Copyright 2016 The Chromium Authors.
> +    Copyright 2017 Intel Corporation
> +    Copyright 2018 Collabora, Ltd
> +
> +    Permission is hereby granted, free of charge, to any person obtaining a
> +    copy of this software and associated documentation files (the "Software"),
> +    to deal in the Software without restriction, including without limitation
> +    the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +    and/or sell copies of the Software, and to permit persons to whom the
> +    Software is furnished to do so, subject to the following conditions:
> +
> +    The above copyright notice and this permission notice (including the next
> +    paragraph) shall be included in all copies or substantial portions of the
> +    Software.
> +
> +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> +    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +    DEALINGS IN THE SOFTWARE.
> +  </copyright>
> +
> +  <interface name="zwp_linux_explicit_synchronization_v1" version="1">
> +    <description summary="protocol for providing explicit synchronization">
> +      This global is a factory interface, allowing clients to request
> +      explicit synchronization for buffers on a per-surface basis.
> +
> +      See zwp_surface_synchronization_v1 for more information.
> +
> +      This interface is derived from Chromium's
> +      zcr_linux_explicit_synchronization_v1.
> +
> +      Warning! The protocol described in this file is experimental and
> +      backward incompatible changes may be made. Backward compatible changes
> +      may be added together with the corresponding interface version bump.
> +      Backward incompatible changes are done by bumping the version number in
> +      the protocol and interface names and resetting the interface version.
> +      Once the protocol is to be declared stable, the 'z' prefix and the
> +      version number in the protocol and interface names are removed and the
> +      interface version number is reset.
> +    </description>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy explicit synchronization factory object">
> +        Destroy this explicit synchronization factory object. Other objects,
> +        including zwp_surface_synchronization_v1 objects created by this
> +        factory, shall not be affected by this request.
> +      </description>
> +    </request>
> +
> +    <enum name="error">
> +      <entry name="synchronization_exists" value="0"
> +             summary="the surface already has a synchronization object associated"/>
> +    </enum>
> +
> +    <request name="get_synchronization">
> +      <description summary="extend surface interface for explicit synchronization">
> +        Instantiate an interface extension for the given wl_surface to provide
> +        explicit synchronization.
> +
> +        If the given wl_surface already has an explicit synchronization object
> +        associated, the synchronization_exists protocol error is raised.
> +      </description>
> +
> +      <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
> +           summary="the new synchronization interface id"/>
> +      <arg name="surface" type="object" interface="wl_surface"
> +           summary="the surface"/>
> +    </request>
> +  </interface>
> +
> +  <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?

> +    <description summary="per-surface explicit synchronization support">
> +      This object implements per-surface explicit synchronization.
> +
> +      Synchronization refers to co-ordination of pipelined operations performed
> +      on buffers. Most GPU clients will schedule an asynchronous operation to
> +      render to the buffer, then immediately send the buffer to the compositor
> +      to be attached to a surface.
> +
> +      In implicit synchronization, ensuring that the rendering operation is
> +      complete before the compositor displays the buffer is an implementation
> +      detail handled by either the kernel or userspace graphics driver.
> +
> +      By contrast, in explicit synchronization, dma_fence objects mark when the
> +      asynchronous operations are complete. When submitting a buffer, the
> +      client provides an acquire fence which will be waited on before the
> +      compositor accesses the buffer. The Wayland server, through a
> +      zwp_buffer_release_v1 object, will inform the client with an event which
> +      may be accompanied by a release fence, when the compositor will no longer
> +      access the buffer contents due to the specific commit that requested the
> +      release event.
> +
> +      Each surface can be associated with only one object of this interface at
> +      any time.
> +
> +      Explicit synchronization is guaranteed to be supported only for buffers
> +      created with any version of the wp_linux_dmabuf buffer factory.
> +    </description>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy synchronization object">
> +        Destroy this explicit synchronization object.
> +
> +        Any fence set by this object with set_acquire_fence since the last
> +        commit will be discarded by the server. Any fences set by this object
> +        before the last commit are not affected.
> +
> +        zwp_buffer_release_v1 objects created by this object are not affected
> +        by this request.
> +      </description>
> +    </request>
> +
> +    <enum name="error">
> +      <entry name="invalid_fence" value="0"
> +             summary="the fence specified by the client could not be imported"/>
> +      <entry name="duplicate_fence" value="1"
> +             summary="multiple fences added for a single surface commit"/>
> +      <entry name="duplicate_release" value="2"
> +             summary="multiple releases added for a single surface commit"/>
> +      <entry name="no_surface" value="3"
> +             summary="the associated wl_surface was destroyed"/>
> +      <entry name="unsupported_buffer" value="4"
> +             summary="the buffer does not support explicit synchronization"/>
> +      <entry name="no_buffer" value="5"
> +             summary="no buffer was attached"/>
> +    </enum>
> +
> +    <request name="set_acquire_fence">
> +      <description summary="set the acquire fence">
> +        Set the acquire fence that must be signaled before the compositor
> +        may sample from the buffer attached with wl_surface.attach. The fence
> +        is a dma_fence kernel object.
> +
> +        The acquire fence is double-buffered state, and will be applied on the
> +        next wl_surface.commit request for the associated surface. Thus, it
> +        applies only to the buffer that is attached to the surface at commit
> +        time.
> +
> +        If the provided fd is not a valid dma_fence fd, then an INVALID_FENCE
> +        error is raised.
> +
> +        If a fence has already been attached during the same commit cycle, a
> +        DUPLICATE_FENCE error is raised.
> +
> +        If the associated wl_surface was destroyed, a NO_SURFACE error is
> +        raised.
> +
> +        If at surface commit time the attached buffer does not support explicit
> +        synchronization, an UNSUPPORTED_BUFFER error is raised.
> +
> +        If at surface commit time there is no buffer attached, a NO_BUFFER
> +        error is raised.
> +      </description>
> +      <arg name="fd" type="fd" summary="acquire fence fd"/>
> +    </request>
> +
> +    <request name="get_release">
> +      <description summary="release fence for last-attached buffer">
> +        Create a listener for the release of the buffer attached by the
> +        client with wl_surface.attach. See zwp_buffer_release_v1
> +        documentation for more information.
> +
> +        The release object is double-buffered state, and will be associated
> +        with the buffer that is attached to the surface at wl_surface.commit
> +        time.
> +
> +        If a zwp_buffer_release_v1 object has already been requested for
> +        the surface in the same commit cycle, a DUPLICATE_RELEASE error
> +        is raised.
> +
> +        If the associated wl_surface was destroyed, a NO_SURFACE error
> +        is raised.
> +
> +        If at surface commit time there is no buffer attached, a NO_BUFFER
> +        error is raised.
> +      </description>
> +      <arg name="release" type="new_id" interface="zwp_buffer_release_v1"
> +           summary="new zwp_buffer_release_v1 object"/>
> +    </request>
> +  </interface>
> +
> +  <interface name="zwp_buffer_release_v1" version="1">
> +    <description summary="buffer release explicit synchronization">
> +      This object is instantiated in response to a
> +      zwp_surface_synchronization_v1.get_release request.
> +
> +      It provides an alternative to wl_buffer.release events, providing a
> +      unique release from a single wl_surface.commit request. The release event
> +      also supports explicit synchronization, providing a fence FD for the
> +      client to synchronize against.
> +
> +      Exactly one event, either a fenced_release or an immediate_release, will
> +      be emitted for the wl_surface.commit request. The compositor can choose
> +      release by release which event it uses.
> +
> +      This event does not replace wl_buffer.release events; servers are still
> +      required to send those events.
> +
> +      Once a buffer release object has delivered a 'fenced_release' or an
> +      'immediate_release' event it is automatically destroyed.
> +    </description>
> +
> +    <event name="fenced_release">
> +      <description summary="release buffer with fence">
> +        Sent when the compositor has finalised its usage of the associated
> +        buffer for the relevant commit, providing a dma_fence which will be
> +        signaled when all operations by the compositor on that buffer for that
> +        commit have finished.
> +
> +        This event destroys the zwp_buffer_release_v1 object.
> +      </description>
> +      <arg name="fence" type="fd" summary="fence for last operation on buffer"/>
> +    </event>
> +
> +    <event name="immediate_release">
> +      <description summary="release buffer immediately">
> +        Sent when the compositor has finalised its usage of the associated
> +        buffer for the relevant commit, and either performed no operations
> +        using it, or has a guarantee that all its operations on that buffer for
> +        that commit have finished.
> +
> +        This event destroys the zwp_buffer_release_v1 object.
> +      </description>
> +    </event>
> +  </interface>
> +
> +</protocol>
> --
> 2.19.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen Nov. 8, 2018, 1 p.m.
On Wed, 07 Nov 2018 20:22:38 +0000
Simon Ser <contact@emersion.fr> wrote:

> Thanks! With these fixes, this is now
> 
> Reviewed-by: Simon Ser <contact@emersion.fr>
> 
> Below is a small comment, I don't know if it's relevant or not, I just wanted to
> point it out.
> 
> > Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
> > ---
> >
> > Changes in patch v6:
> >   - Fixed wl_buffer.attach -> wl_surface.attach typos.
> >   - Added NO_BUFFER error and updated request descriptions.
> >   - Consistently used "error is raised" phrasing.
> >   - Added comment about buffer_release object destruction in event
> >     descriptions.
> >
> > Changes in patch v5:
> >   - Further clarified the per-commit nature of buffer_release.
> >   - Used the wp_linux_dmabuf name to refer to all versions of that protocol.
> >   - Fixed wl_buffer.attach typo.
> >   - Clarified when an INVALID_FENCE error is raised.
> >   - Improved wording explaining the double-buffer state of buffer_release.
> >   - Allowed get_release requests on all non-null buffers.
> >   - Clarified that the compositor is free to select buffer_release events
> >     on a release by release basis.
> >   - Changed buffer_release to be self-destroyed after it emits an event.
> >
> > Changes in patch v4:
> >   - Guaranteed protocol compatibility only with zwp_linux_dmabuf buffers.
> >   - Added the UNSUPPORTED_BUFFER error.
> >
> > Changes in patch v3:
> >   - Reworded implicit/explicit synchronization intro in
> >     zwp_surface_synchronization_v1 description.
> >   - Removed confusing mention of wl_buffer.release in
> >     zwp_surface_synchronization_v1 description.
> >   - Clarified which fences are affected on sync object destruction.
> >   - Removed unclear mention about wl_buffer destruction
> >     in fenced_release description.
> >   - Clarified that the release events and their guarantees apply to
> >     the relevant commit only.
> >   - Reformatted text.
> >
> > Changes in patch v2:
> >   - Added NO_SURFACE error for zwp_surface_synchronization_v1 requests.
> >   - Removed restriction to destroy a zwp_surface_synchronization_v1 object
> >     after the associated wl_surface is destroyed.
> >   - Clarified which buffer the acquire fence is associated with.
> >   - Clarified that exactly one event, either a fenced_release or a
> >     immediate_release, will be emitted for each commit.
> >
> >  Makefile.am                                   |   1 +
> >  .../linux-explicit-synchronization/README     |   6 +
> >  ...x-explicit-synchronization-unstable-v1.xml | 234 ++++++++++++++++++
> >  3 files changed, 241 insertions(+)
> >  create mode 100644 unstable/linux-explicit-synchronization/README
> >  create mode 100644 unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 6394e26..7dfbb9e 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -21,6 +21,7 @@ unstable_protocols =								\
> >  	unstable/xdg-output/xdg-output-unstable-v1.xml				\
> >  	unstable/input-timestamps/input-timestamps-unstable-v1.xml	\
> >  	unstable/xdg-decoration/xdg-decoration-unstable-v1.xml	\
> > +	unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml \
> >  	$(NULL)
> >
> >  stable_protocols =								\
> > diff --git a/unstable/linux-explicit-synchronization/README b/unstable/linux-explicit-synchronization/README
> > new file mode 100644
> > index 0000000..f13b404
> > --- /dev/null
> > +++ b/unstable/linux-explicit-synchronization/README
> > @@ -0,0 +1,6 @@
> > +Linux explicit synchronization (dma-fence) protocol
> > +
> > +Maintainers:
> > +David Reveman <reveman@chromium.org>
> > +Daniel Stone <daniels@collabora.com>
> > +Alexandros Frantzis <alexandros.frantzis@collabora.com>  
> diff --git a/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml > b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> > new file mode 100644
> > index 0000000..b87663f
> > --- /dev/null
> > +++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> > @@ -0,0 +1,234 @@
> > +<?xml version="1.0" encoding="UTF-8"?>
> > +<protocol name="zwp_linux_explicit_synchronization_unstable_v1">
> > +
> > +  <copyright>
> > +    Copyright 2016 The Chromium Authors.
> > +    Copyright 2017 Intel Corporation
> > +    Copyright 2018 Collabora, Ltd
> > +
> > +    Permission is hereby granted, free of charge, to any person obtaining a
> > +    copy of this software and associated documentation files (the "Software"),
> > +    to deal in the Software without restriction, including without limitation
> > +    the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > +    and/or sell copies of the Software, and to permit persons to whom the
> > +    Software is furnished to do so, subject to the following conditions:
> > +
> > +    The above copyright notice and this permission notice (including the next
> > +    paragraph) shall be included in all copies or substantial portions of the
> > +    Software.
> > +
> > +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > +    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > +    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > +    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > +    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > +    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > +    DEALINGS IN THE SOFTWARE.
> > +  </copyright>
> > +
> > +  <interface name="zwp_linux_explicit_synchronization_v1" version="1">
> > +    <description summary="protocol for providing explicit synchronization">
> > +      This global is a factory interface, allowing clients to request
> > +      explicit synchronization for buffers on a per-surface basis.
> > +
> > +      See zwp_surface_synchronization_v1 for more information.
> > +
> > +      This interface is derived from Chromium's
> > +      zcr_linux_explicit_synchronization_v1.
> > +
> > +      Warning! The protocol described in this file is experimental and
> > +      backward incompatible changes may be made. Backward compatible changes
> > +      may be added together with the corresponding interface version bump.
> > +      Backward incompatible changes are done by bumping the version number in
> > +      the protocol and interface names and resetting the interface version.
> > +      Once the protocol is to be declared stable, the 'z' prefix and the
> > +      version number in the protocol and interface names are removed and the
> > +      interface version number is reset.
> > +    </description>
> > +
> > +    <request name="destroy" type="destructor">
> > +      <description summary="destroy explicit synchronization factory object">
> > +        Destroy this explicit synchronization factory object. Other objects,
> > +        including zwp_surface_synchronization_v1 objects created by this
> > +        factory, shall not be affected by this request.
> > +      </description>
> > +    </request>
> > +
> > +    <enum name="error">
> > +      <entry name="synchronization_exists" value="0"
> > +             summary="the surface already has a synchronization object associated"/>
> > +    </enum>
> > +
> > +    <request name="get_synchronization">
> > +      <description summary="extend surface interface for explicit synchronization">
> > +        Instantiate an interface extension for the given wl_surface to provide
> > +        explicit synchronization.
> > +
> > +        If the given wl_surface already has an explicit synchronization object
> > +        associated, the synchronization_exists protocol error is raised.
> > +      </description>
> > +
> > +      <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
> > +           summary="the new synchronization interface id"/>
> > +      <arg name="surface" type="object" interface="wl_surface"
> > +           summary="the surface"/>
> > +    </request>
> > +  </interface>
> > +
> > +  <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.

> > +  <interface name="zwp_buffer_release_v1" version="1">
> > +    <description summary="buffer release explicit synchronization">
> > +      This object is instantiated in response to a
> > +      zwp_surface_synchronization_v1.get_release request.


Thanks,
pq
Daniel Stone Nov. 8, 2018, 2:12 p.m.
Hi,

On Thu, 8 Nov 2018 at 13:01, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> On Wed, 07 Nov 2018 20:22:38 +0000 Simon Ser <contact@emersion.fr> wrote:
> > 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.

There's precedence in Vulkan's external-sharing interfaces: one of the
semaphore import/export types is an 'opaque FD' which has
implementation-defined meaning. On the other hand, when this came up
last, NVIDIA said that it was only intended for reusable semaphores
whose lifetime roughly matched swapchain images. Apparently on their
driver stack, doing per-frame import of syncpoint-style fences has
crippling performance implications, so given that this extension's
semantics are so clearly tied to sync_file, it wouldn't be a good
match.

Wildcard option: if we could export DRM syncobjs between processes,
maybe that would be a better long-term solution than sync_file.

Cheers,
Daniel
Alexandros Frantzis Nov. 8, 2018, 2:34 p.m.
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@emersion.fr> wrote:
> 
> > Thanks! With these fixes, this is now
> > 
> > Reviewed-by: Simon Ser <contact@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).

Thoughts?

Thanks,
Alexandros
Pekka Paalanen Nov. 8, 2018, 3:43 p.m.
On Thu, 8 Nov 2018 16:34:52 +0200
Alexandros Frantzis <alexandros.frantzis@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@emersion.fr> wrote:
> >   
> > > Thanks! With these fixes, this is now
> > > 
> > > Reviewed-by: Simon Ser <contact@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?

You could get away with an opaque fd, if the spec could say which API
the compositor and client needs to use to get/use the fd. But then
we're essentially back to saying it's a Linux DRM sync_file thingy in
this case.

We could have an opaque fd with an enum saying what it is. But that
brings complication: how do you pick or know which one to use. Do you
have to support all the kinds.

One option would be to say the fd is platform specific. An example of
such (attempt) is in wp_presentation.clock_id.

The multiple factory interfaces approach has the caveat that the
interface will be stuck at version 1 then and cannot realistically ever
be extended. If in some case the fence was not an fd, then there would
be nothing to share anyway. Also the interface already defines that the
fences are one-shot, so it would not be appropriate for the semaphores
Daniel mentioned.

Adding new factory methods in the global interface is possible though,
and will not pose a versioning problem. We can also add new requests
and events for new types of fences in a perfectly backwards-compatible
fashion.

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


Thanks,
pq
Pekka Paalanen Nov. 8, 2018, 3:45 p.m.
On Wed,  7 Nov 2018 16:59:25 +0200
Alexandros Frantzis <alexandros.frantzis@collabora.com> wrote:

> Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
> ---
> 
> Changes in patch v6:
>   - Fixed wl_buffer.attach -> wl_surface.attach typos.
>   - Added NO_BUFFER error and updated request descriptions.
>   - Consistently used "error is raised" phrasing.
>   - Added comment about buffer_release object destruction in event
>     descriptions.

Hi Alf,

looks good!

Now just to finish the bikeshed with the interface names.


Thanks,
pq

> 
> Changes in patch v5:
>   - Further clarified the per-commit nature of buffer_release.
>   - Used the wp_linux_dmabuf name to refer to all versions of that protocol.
>   - Fixed wl_buffer.attach typo.
>   - Clarified when an INVALID_FENCE error is raised.
>   - Improved wording explaining the double-buffer state of buffer_release.
>   - Allowed get_release requests on all non-null buffers.
>   - Clarified that the compositor is free to select buffer_release events
>     on a release by release basis.
>   - Changed buffer_release to be self-destroyed after it emits an event.
> 
> Changes in patch v4:
>   - Guaranteed protocol compatibility only with zwp_linux_dmabuf buffers.
>   - Added the UNSUPPORTED_BUFFER error.
> 
> Changes in patch v3:
>   - Reworded implicit/explicit synchronization intro in
>     zwp_surface_synchronization_v1 description.
>   - Removed confusing mention of wl_buffer.release in
>     zwp_surface_synchronization_v1 description.
>   - Clarified which fences are affected on sync object destruction.
>   - Removed unclear mention about wl_buffer destruction
>     in fenced_release description.
>   - Clarified that the release events and their guarantees apply to
>     the relevant commit only.
>   - Reformatted text.
> 
> Changes in patch v2:
>   - Added NO_SURFACE error for zwp_surface_synchronization_v1 requests.
>   - Removed restriction to destroy a zwp_surface_synchronization_v1 object
>     after the associated wl_surface is destroyed.
>   - Clarified which buffer the acquire fence is associated with.
>   - Clarified that exactly one event, either a fenced_release or a
>     immediate_release, will be emitted for each commit.
> 
>  Makefile.am                                   |   1 +
>  .../linux-explicit-synchronization/README     |   6 +
>  ...x-explicit-synchronization-unstable-v1.xml | 234 ++++++++++++++++++
>  3 files changed, 241 insertions(+)
>  create mode 100644 unstable/linux-explicit-synchronization/README
>  create mode 100644 unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> 
> diff --git a/Makefile.am b/Makefile.am
> index 6394e26..7dfbb9e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -21,6 +21,7 @@ unstable_protocols =								\
>  	unstable/xdg-output/xdg-output-unstable-v1.xml				\
>  	unstable/input-timestamps/input-timestamps-unstable-v1.xml	\
>  	unstable/xdg-decoration/xdg-decoration-unstable-v1.xml	\
> +	unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml \
>  	$(NULL)
>  
>  stable_protocols =								\
> diff --git a/unstable/linux-explicit-synchronization/README b/unstable/linux-explicit-synchronization/README
> new file mode 100644
> index 0000000..f13b404
> --- /dev/null
> +++ b/unstable/linux-explicit-synchronization/README
> @@ -0,0 +1,6 @@
> +Linux explicit synchronization (dma-fence) protocol
> +
> +Maintainers:
> +David Reveman <reveman@chromium.org>
> +Daniel Stone <daniels@collabora.com>
> +Alexandros Frantzis <alexandros.frantzis@collabora.com>
> diff --git a/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> new file mode 100644
> index 0000000..b87663f
> --- /dev/null
> +++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> @@ -0,0 +1,234 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="zwp_linux_explicit_synchronization_unstable_v1">
> +
> +  <copyright>
> +    Copyright 2016 The Chromium Authors.
> +    Copyright 2017 Intel Corporation
> +    Copyright 2018 Collabora, Ltd
> +
> +    Permission is hereby granted, free of charge, to any person obtaining a
> +    copy of this software and associated documentation files (the "Software"),
> +    to deal in the Software without restriction, including without limitation
> +    the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +    and/or sell copies of the Software, and to permit persons to whom the
> +    Software is furnished to do so, subject to the following conditions:
> +
> +    The above copyright notice and this permission notice (including the next
> +    paragraph) shall be included in all copies or substantial portions of the
> +    Software.
> +
> +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> +    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +    DEALINGS IN THE SOFTWARE.
> +  </copyright>
> +
> +  <interface name="zwp_linux_explicit_synchronization_v1" version="1">
> +    <description summary="protocol for providing explicit synchronization">
> +      This global is a factory interface, allowing clients to request
> +      explicit synchronization for buffers on a per-surface basis.
> +
> +      See zwp_surface_synchronization_v1 for more information.
> +
> +      This interface is derived from Chromium's
> +      zcr_linux_explicit_synchronization_v1.
> +
> +      Warning! The protocol described in this file is experimental and
> +      backward incompatible changes may be made. Backward compatible changes
> +      may be added together with the corresponding interface version bump.
> +      Backward incompatible changes are done by bumping the version number in
> +      the protocol and interface names and resetting the interface version.
> +      Once the protocol is to be declared stable, the 'z' prefix and the
> +      version number in the protocol and interface names are removed and the
> +      interface version number is reset.
> +    </description>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy explicit synchronization factory object">
> +        Destroy this explicit synchronization factory object. Other objects,
> +        including zwp_surface_synchronization_v1 objects created by this
> +        factory, shall not be affected by this request.
> +      </description>
> +    </request>
> +
> +    <enum name="error">
> +      <entry name="synchronization_exists" value="0"
> +             summary="the surface already has a synchronization object associated"/>
> +    </enum>
> +
> +    <request name="get_synchronization">
> +      <description summary="extend surface interface for explicit synchronization">
> +        Instantiate an interface extension for the given wl_surface to provide
> +        explicit synchronization.
> +
> +        If the given wl_surface already has an explicit synchronization object
> +        associated, the synchronization_exists protocol error is raised.
> +      </description>
> +
> +      <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
> +           summary="the new synchronization interface id"/>
> +      <arg name="surface" type="object" interface="wl_surface"
> +           summary="the surface"/>
> +    </request>
> +  </interface>
> +
> +  <interface name="zwp_surface_synchronization_v1" version="1">
> +    <description summary="per-surface explicit synchronization support">
> +      This object implements per-surface explicit synchronization.
> +
> +      Synchronization refers to co-ordination of pipelined operations performed
> +      on buffers. Most GPU clients will schedule an asynchronous operation to
> +      render to the buffer, then immediately send the buffer to the compositor
> +      to be attached to a surface.
> +
> +      In implicit synchronization, ensuring that the rendering operation is
> +      complete before the compositor displays the buffer is an implementation
> +      detail handled by either the kernel or userspace graphics driver.
> +
> +      By contrast, in explicit synchronization, dma_fence objects mark when the
> +      asynchronous operations are complete. When submitting a buffer, the
> +      client provides an acquire fence which will be waited on before the
> +      compositor accesses the buffer. The Wayland server, through a
> +      zwp_buffer_release_v1 object, will inform the client with an event which
> +      may be accompanied by a release fence, when the compositor will no longer
> +      access the buffer contents due to the specific commit that requested the
> +      release event.
> +
> +      Each surface can be associated with only one object of this interface at
> +      any time.
> +
> +      Explicit synchronization is guaranteed to be supported only for buffers
> +      created with any version of the wp_linux_dmabuf buffer factory.
> +    </description>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy synchronization object">
> +        Destroy this explicit synchronization object.
> +
> +        Any fence set by this object with set_acquire_fence since the last
> +        commit will be discarded by the server. Any fences set by this object
> +        before the last commit are not affected.
> +
> +        zwp_buffer_release_v1 objects created by this object are not affected
> +        by this request.
> +      </description>
> +    </request>
> +
> +    <enum name="error">
> +      <entry name="invalid_fence" value="0"
> +             summary="the fence specified by the client could not be imported"/>
> +      <entry name="duplicate_fence" value="1"
> +             summary="multiple fences added for a single surface commit"/>
> +      <entry name="duplicate_release" value="2"
> +             summary="multiple releases added for a single surface commit"/>
> +      <entry name="no_surface" value="3"
> +             summary="the associated wl_surface was destroyed"/>
> +      <entry name="unsupported_buffer" value="4"
> +             summary="the buffer does not support explicit synchronization"/>
> +      <entry name="no_buffer" value="5"
> +             summary="no buffer was attached"/>
> +    </enum>
> +
> +    <request name="set_acquire_fence">
> +      <description summary="set the acquire fence">
> +        Set the acquire fence that must be signaled before the compositor
> +        may sample from the buffer attached with wl_surface.attach. The fence
> +        is a dma_fence kernel object.
> +
> +        The acquire fence is double-buffered state, and will be applied on the
> +        next wl_surface.commit request for the associated surface. Thus, it
> +        applies only to the buffer that is attached to the surface at commit
> +        time.
> +
> +        If the provided fd is not a valid dma_fence fd, then an INVALID_FENCE
> +        error is raised.
> +
> +        If a fence has already been attached during the same commit cycle, a
> +        DUPLICATE_FENCE error is raised.
> +
> +        If the associated wl_surface was destroyed, a NO_SURFACE error is
> +        raised.
> +
> +        If at surface commit time the attached buffer does not support explicit
> +        synchronization, an UNSUPPORTED_BUFFER error is raised.
> +
> +        If at surface commit time there is no buffer attached, a NO_BUFFER
> +        error is raised.
> +      </description>
> +      <arg name="fd" type="fd" summary="acquire fence fd"/>
> +    </request>
> +
> +    <request name="get_release">
> +      <description summary="release fence for last-attached buffer">
> +        Create a listener for the release of the buffer attached by the
> +        client with wl_surface.attach. See zwp_buffer_release_v1
> +        documentation for more information.
> +
> +        The release object is double-buffered state, and will be associated
> +        with the buffer that is attached to the surface at wl_surface.commit
> +        time.
> +
> +        If a zwp_buffer_release_v1 object has already been requested for
> +        the surface in the same commit cycle, a DUPLICATE_RELEASE error
> +        is raised.
> +
> +        If the associated wl_surface was destroyed, a NO_SURFACE error
> +        is raised.
> +
> +        If at surface commit time there is no buffer attached, a NO_BUFFER
> +        error is raised.
> +      </description>
> +      <arg name="release" type="new_id" interface="zwp_buffer_release_v1"
> +           summary="new zwp_buffer_release_v1 object"/>
> +    </request>
> +  </interface>
> +
> +  <interface name="zwp_buffer_release_v1" version="1">
> +    <description summary="buffer release explicit synchronization">
> +      This object is instantiated in response to a
> +      zwp_surface_synchronization_v1.get_release request.
> +
> +      It provides an alternative to wl_buffer.release events, providing a
> +      unique release from a single wl_surface.commit request. The release event
> +      also supports explicit synchronization, providing a fence FD for the
> +      client to synchronize against.
> +
> +      Exactly one event, either a fenced_release or an immediate_release, will
> +      be emitted for the wl_surface.commit request. The compositor can choose
> +      release by release which event it uses.
> +
> +      This event does not replace wl_buffer.release events; servers are still
> +      required to send those events.
> +
> +      Once a buffer release object has delivered a 'fenced_release' or an
> +      'immediate_release' event it is automatically destroyed.
> +    </description>
> +
> +    <event name="fenced_release">
> +      <description summary="release buffer with fence">
> +        Sent when the compositor has finalised its usage of the associated
> +        buffer for the relevant commit, providing a dma_fence which will be
> +        signaled when all operations by the compositor on that buffer for that
> +        commit have finished.
> +
> +        This event destroys the zwp_buffer_release_v1 object.
> +      </description>
> +      <arg name="fence" type="fd" summary="fence for last operation on buffer"/>
> +    </event>
> +
> +    <event name="immediate_release">
> +      <description summary="release buffer immediately">
> +        Sent when the compositor has finalised its usage of the associated
> +        buffer for the relevant commit, and either performed no operations
> +        using it, or has a guarantee that all its operations on that buffer for
> +        that commit have finished.
> +
> +        This event destroys the zwp_buffer_release_v1 object.
> +      </description>
> +    </event>
> +  </interface>
> +
> +</protocol>
Alexandros Frantzis Nov. 8, 2018, 4:37 p.m.
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@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@emersion.fr> wrote:
> > >   
> > > > Thanks! With these fixes, this is now
> > > > 
> > > > Reviewed-by: Simon Ser <contact@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
Pekka Paalanen Nov. 9, 2018, 10:18 a.m.
On Thu, 8 Nov 2018 18:37:50 +0200
Alexandros Frantzis <alexandros.frantzis@collabora.com> wrote:

> 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@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@emersion.fr> wrote:
> > > >     
> > > > > Thanks! With these fixes, this is now
> > > > > 
> > > > > Reviewed-by: Simon Ser <contact@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.

Hi Alf,

I really don't see the benefit from that complexity in this case, given
how much would be shared between different types.

EGLImage and wl_buffer are both opaque "data types" that need to be
defined to have interoperability (a common language) between different
extensions. There the extension layering makes sense. You can create
them in several ways and probably use them in several places as well.

However, with fences, the object type is a file descriptor which is a
fundamental data type in Wayland. So the fence base extension would
only say "a fence is represented as a file descriptor", and not much
more. Whether the fence fd is single-shot or a reusable semaphore
already gets into the type of the fence, and that changes what the
interface for using it would look like: single-shot is passed or
created per wl_surface.commit, while a reusable fence would probably be
tied to the lifetime of a wl_buffer instead. Therefore I don't think
you'd actually want to mandate sharing any of the protocol interfaces
defined here.

With fences, the fd is the handle already, we don't need Wayland
protocol to establish an object of type "a fence" like we do with "a
buffer".

We could wrap the fd in a wp_fence protocol object, possibly with an
explicit note about what kind of fd it is. The reason to do that is
that then it could be used in more places or ways than just
wl_surface.commits. But if the fd type restricts the ways it can be
used, then it mostly adds possible error cases. Other places to use a
wp_fence could be, say, a screencasting video streaming interface. What
would be the benefit there over using the DRM fence fd directly in a
video streaming interface?

That last point might give a reason to invent a wp_fence, but I think
it's premature to do it right now. But, we likely do want to revisit
this question when discussing about stabilizing the explicit
synchronization extension, since who knows, maybe by that time there
are more use cases.

Until then, let's not get ahead of ourselves.

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

Thanks,
pq