[wayland-protocols,v7] Add zwp_linux_explicit_synchronization_v1

Submitted by Alexandros Frantzis on Nov. 9, 2018, 7:46 a.m.

Details

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

Commit Message

Alexandros Frantzis Nov. 9, 2018, 7:46 a.m.
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
---

Changes in patch v7:
  - Added linux_ prefix to interface names.

Changes in patch v6:
  - Fixed wl_buffer.attach -> wl_surface.attach typos.
  - Added NO_BUFFER error and update request descriptions.
  - Consistently used "error is raised" phrasing.
  - Add 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 | 235 ++++++++++++++++++
 3 files changed, 242 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..db36284
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
@@ -0,0 +1,235 @@ 
+<?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_linux_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_linux_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_linux_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_linux_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_linux_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_linux_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_linux_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_linux_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_linux_buffer_release_v1"
+           summary="new zwp_linux_buffer_release_v1 object"/>
+    </request>
+  </interface>
+
+  <interface name="zwp_linux_buffer_release_v1" version="1">
+    <description summary="buffer release explicit synchronization">
+      This object is instantiated in response to a
+      zwp_linux_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_linux_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_linux_buffer_release_v1 object.
+      </description>
+    </event>
+  </interface>
+
+</protocol>

Comments

Simon Ser Nov. 9, 2018, 8:14 a.m.
Reviewed-by: Simon Ser <contact@emersion.fr>

Thanks!
Pekka Paalanen Nov. 9, 2018, 10:48 a.m.
On Fri,  9 Nov 2018 09:46:36 +0200
Alexandros Frantzis <alexandros.frantzis@collabora.com> wrote:

> Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>

Hi Alf,

could you come up with some rationale to put in the commit message on
why this extension is being added?

I can add that while pushing upstream, if there are no other changes
coming.

Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>

You have ensured that the C files generated from this revision build
fine in Weston, right?

David, Daniel, since your name is in the maintainers, can I have your
R-b, please?

I aim to land this on Monday, so if anyone has anything to say, try to
be quick. Jonas told me more than a week ago that he is looking to make
a wayland-protocols release soon. If anyone has opinions on whether
this should or should not make that release, let us know.


Thanks,
pq

> ---
> 
> Changes in patch v7:
>   - Added linux_ prefix to interface names.
> 
> Changes in patch v6:
>   - Fixed wl_buffer.attach -> wl_surface.attach typos.
>   - Added NO_BUFFER error and update request descriptions.
>   - Consistently used "error is raised" phrasing.
>   - Add 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 | 235 ++++++++++++++++++
>  3 files changed, 242 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..db36284
> --- /dev/null
> +++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> @@ -0,0 +1,235 @@
> +<?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_linux_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_linux_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_linux_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_linux_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_linux_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_linux_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_linux_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_linux_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_linux_buffer_release_v1"
> +           summary="new zwp_linux_buffer_release_v1 object"/>
> +    </request>
> +  </interface>
> +
> +  <interface name="zwp_linux_buffer_release_v1" version="1">
> +    <description summary="buffer release explicit synchronization">
> +      This object is instantiated in response to a
> +      zwp_linux_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_linux_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_linux_buffer_release_v1 object.
> +      </description>
> +    </event>
> +  </interface>
> +
> +</protocol>
Alexandros Frantzis Nov. 9, 2018, 2:24 p.m.
On Fri, Nov 09, 2018 at 12:48:09PM +0200, Pekka Paalanen wrote:
> On Fri,  9 Nov 2018 09:46:36 +0200
> Alexandros Frantzis <alexandros.frantzis@collabora.com> wrote:
> 
> > Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
> 
> Hi Alf,

Hi Pekka,

thanks for the review!

> could you come up with some rationale to put in the commit message on
> why this extension is being added?
> 
> I can add that while pushing upstream, if there are no other changes
> coming.

How about:

This protocol enables explicit synchronization of asynchronous graphics
operations on buffers on a per-commit basis. Support is currently
limited to dmabuf buffers and dma_fence fence FDs.

Explicit synchronization provides a more versatile notification
mechanism for buffer readiness and availability, and can be used to
improve efficiency by integrating with related functionality in display
and graphics APIs.

Finally, the per-commit nature of the release events provided by this
protocol offer a solution to a deficiency of the wl_buffer.release event
(see https://gitlab.freedesktop.org/wayland/wayland/issues/46).

> Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
> 
> You have ensured that the C files generated from this revision build
> fine in Weston, right?

Yes, I have been working locally with this revision in Weston, and
everything builds and runs fine.

Thanks,
Alexandros
Daniel Stone Nov. 12, 2018, 11:15 a.m.
On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> I can add that while pushing upstream, if there are no other changes
> coming.
>
> Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
>
> You have ensured that the C files generated from this revision build
> fine in Weston, right?
>
> David, Daniel, since your name is in the maintainers, can I have your
> R-b, please?

The protocol is:
Reviewed-by: Daniel Stone <daniels@collabora.com>

The Weston implementation looks pretty good so far, though there's no
full implementation of release yet.

Cheers,
Daniel
Alexandros Frantzis Nov. 12, 2018, 11:19 a.m.
On Fri, Nov 09, 2018 at 04:24:56PM +0200, Alexandros Frantzis wrote:
> On Fri, Nov 09, 2018 at 12:48:09PM +0200, Pekka Paalanen wrote:
> > On Fri,  9 Nov 2018 09:46:36 +0200
> > Alexandros Frantzis <alexandros.frantzis@collabora.com> wrote:
> > 
> > > Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>
> > 
> > Hi Alf,
> 
> Hi Pekka,
> 
> thanks for the review!
> 
> > could you come up with some rationale to put in the commit message on
> > why this extension is being added?
> > 
> > I can add that while pushing upstream, if there are no other changes
> > coming.
> 
> How about:
> 
> This protocol enables explicit synchronization of asynchronous graphics
> operations on buffers on a per-commit basis. Support is currently
> limited to dmabuf buffers and dma_fence fence FDs.
> 
> Explicit synchronization provides a more versatile notification
> mechanism for buffer readiness and availability, and can be used to
> improve efficiency by integrating with related functionality in display
> and graphics APIs.
> 
> Finally, the per-commit nature of the release events provided by this
> protocol offer a solution to a deficiency of the wl_buffer.release event
> (see https://gitlab.freedesktop.org/wayland/wayland/issues/46).

v2 of the text, with some more ChromeOS related information:

This protocol enables explicit synchronization of asynchronous graphics
operations on buffers on a per-commit basis. Support is currently
limited to dmabuf buffers and dma_fence fence FDs.

Explicit synchronization provides a more versatile notification
mechanism for buffer readiness and availability, and can be used to
improve efficiency by integrating with related functionality in display
and graphics APIs.

This protocol is also useful in ChromeOS ARC++ (running Android apps
inside ChromeOS, using Wayland as the communication protocol), where it
can enable integration of the ChromeOS compositor with the explicit
synchronization mechanisms of the Android display subsystem.

Finally, the per-commit nature of the release events provided by this
protocol potentially offers a solution to a deficiency of the
wl_buffer.release event (see
https://gitlab.freedesktop.org/wayland/wayland/issues/46).

Thanks,
Alexandros
Tomek Bury Nov. 12, 2018, 12:39 p.m.
HI Daniel,

Where can I find the work-in-progress implementation? I'd like to try it
out with Broadcom driver which doesn't have implicit cross-process sync. I
can add the explicit sync protocol implementation on the driver side but
I'd need a reference to test it against.

Cheers,
Tomek

On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <daniel@fooishbar.org> wrote:

> On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > I can add that while pushing upstream, if there are no other changes
> > coming.
> >
> > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
> >
> > You have ensured that the C files generated from this revision build
> > fine in Weston, right?
> >
> > David, Daniel, since your name is in the maintainers, can I have your
> > R-b, please?
>
> The protocol is:
> Reviewed-by: Daniel Stone <daniels@collabora.com>
>
> The Weston implementation looks pretty good so far, though there's no
> full implementation of release yet.
>
> Cheers,
> Daniel
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
Pekka Paalanen Nov. 12, 2018, 2:40 p.m.
On Mon, 12 Nov 2018 11:15:25 +0000
Daniel Stone <daniel@fooishbar.org> wrote:

> On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > I can add that while pushing upstream, if there are no other changes
> > coming.
> >
> > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
> >
> > You have ensured that the C files generated from this revision build
> > fine in Weston, right?
> >
> > David, Daniel, since your name is in the maintainers, can I have your
> > R-b, please?  
> 
> The protocol is:
> Reviewed-by: Daniel Stone <daniels@collabora.com>
> 
> The Weston implementation looks pretty good so far, though there's no
> full implementation of release yet.

Pushed:
   18032f6..19ec5dc  master -> master

I dropped Reveman from the maintainers due to lack of R-b or S-o-b. I
welcome a patch to add him back if he wants to.


Thanks,
pq
Alexandros Frantzis Nov. 13, 2018, 9:08 a.m.
On Mon, Nov 12, 2018 at 12:39:58PM +0000, Tomek Bury wrote:
> On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <daniel@fooishbar.org> wrote:
> 
> > On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > > I can add that while pushing upstream, if there are no other changes
> > > coming.
> > >
> > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
> > >
> > > You have ensured that the C files generated from this revision build
> > > fine in Weston, right?
> > >
> > > David, Daniel, since your name is in the maintainers, can I have your
> > > R-b, please?
> >
> > The protocol is:
> > Reviewed-by: Daniel Stone <daniels@collabora.com>
> >
> > The Weston implementation looks pretty good so far, though there's no
> > full implementation of release yet.
> >
> > Cheers,
> > Daniel
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >
> 
> HI Daniel,
> 
> Where can I find the work-in-progress implementation? I'd like to try it
> out with Broadcom driver which doesn't have implicit cross-process sync. I
> can add the explicit sync protocol implementation on the driver side but
> I'd need a reference to test it against.
> 
> Cheers,
> Tomek

Hi Tomek,

the WIP implementation can be found here [1]. I hope to push an update,
including some zwp_buffer_release_v1 correctness fixes, in the following
days.

Thanks,
Alexandros

[1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
Tomek Bury Nov. 13, 2018, 9:33 a.m.
Thanks!

On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
alexandros.frantzis@collabora.com> wrote:

> On Mon, Nov 12, 2018 at 12:39:58PM +0000, Tomek Bury wrote:
> > On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <daniel@fooishbar.org>
> wrote:
> >
> > > On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <ppaalanen@gmail.com>
> wrote:
> > > > I can add that while pushing upstream, if there are no other changes
> > > > coming.
> > > >
> > > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
> > > >
> > > > You have ensured that the C files generated from this revision build
> > > > fine in Weston, right?
> > > >
> > > > David, Daniel, since your name is in the maintainers, can I have your
> > > > R-b, please?
> > >
> > > The protocol is:
> > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > >
> > > The Weston implementation looks pretty good so far, though there's no
> > > full implementation of release yet.
> > >
> > > Cheers,
> > > Daniel
> > > _______________________________________________
> > > wayland-devel mailing list
> > > wayland-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > >
> >
> > HI Daniel,
> >
> > Where can I find the work-in-progress implementation? I'd like to try it
> > out with Broadcom driver which doesn't have implicit cross-process sync.
> I
> > can add the explicit sync protocol implementation on the driver side but
> > I'd need a reference to test it against.
> >
> > Cheers,
> > Tomek
>
> Hi Tomek,
>
> the WIP implementation can be found here [1]. I hope to push an update,
> including some zwp_buffer_release_v1 correctness fixes, in the following
> days.
>
> Thanks,
> Alexandros
>
> [1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
>
Tomek Bury Nov. 23, 2018, 1:07 p.m.
Hi Alexandros,

Sorry for a delay. I've finally got an end-to-end system to test it out. It
took some time because Weston backend I wrote a while back needed serious
rework to catch up with latest changes.

There's one thing that didn't work for me. In compositor you reject
anything that isn't a DMA buffer and then in glrenderer you put an extra
assertion. Why? All you do is use an EGL extension in order to import
external fence_fd. There's no dmabuf dependency there. As long as the EGL
implementation exposes EGL_SYNC_NATIVE_FENCE_ANDROID extension this should
"just work" (tm) for the GL renderer. It certainly did for me. CPU-based
renderers can poll() to wait.

The type of buffer used is an orthogonal problem. The
EGL_WL_bind_wayland_display
extension takes care of GL clients' buffers in GL renderer, for anything
else the renderer needs to know how to get pixels and use whatever means to
put those pixels on screen.

Cheers,
Tomek

On Tue, 13 Nov 2018 at 09:33, Tomek Bury <tomek.bury@gmail.com> wrote:

> Thanks!
>
> On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
> alexandros.frantzis@collabora.com> wrote:
>
>> On Mon, Nov 12, 2018 at 12:39:58PM +0000, Tomek Bury wrote:
>> > On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <daniel@fooishbar.org>
>> wrote:
>> >
>> > > On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <ppaalanen@gmail.com>
>> wrote:
>> > > > I can add that while pushing upstream, if there are no other changes
>> > > > coming.
>> > > >
>> > > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
>> > > >
>> > > > You have ensured that the C files generated from this revision build
>> > > > fine in Weston, right?
>> > > >
>> > > > David, Daniel, since your name is in the maintainers, can I have
>> your
>> > > > R-b, please?
>> > >
>> > > The protocol is:
>> > > Reviewed-by: Daniel Stone <daniels@collabora.com>
>> > >
>> > > The Weston implementation looks pretty good so far, though there's no
>> > > full implementation of release yet.
>> > >
>> > > Cheers,
>> > > Daniel
>> > > _______________________________________________
>> > > wayland-devel mailing list
>> > > wayland-devel@lists.freedesktop.org
>> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>> > >
>> >
>> > HI Daniel,
>> >
>> > Where can I find the work-in-progress implementation? I'd like to try it
>> > out with Broadcom driver which doesn't have implicit cross-process
>> sync. I
>> > can add the explicit sync protocol implementation on the driver side but
>> > I'd need a reference to test it against.
>> >
>> > Cheers,
>> > Tomek
>>
>> Hi Tomek,
>>
>> the WIP implementation can be found here [1]. I hope to push an update,
>> including some zwp_buffer_release_v1 correctness fixes, in the following
>> days.
>>
>> Thanks,
>> Alexandros
>>
>> [1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
>>
>
Pekka Paalanen Nov. 23, 2018, 1:47 p.m.
On Fri, 23 Nov 2018 13:07:37 +0000
Tomek Bury <tomek.bury@gmail.com> wrote:

> Hi Alexandros,
> 
> Sorry for a delay. I've finally got an end-to-end system to test it out. It
> took some time because Weston backend I wrote a while back needed serious
> rework to catch up with latest changes.
> 
> There's one thing that didn't work for me. In compositor you reject
> anything that isn't a DMA buffer and then in glrenderer you put an extra
> assertion. Why? All you do is use an EGL extension in order to import
> external fence_fd. There's no dmabuf dependency there. As long as the EGL
> implementation exposes EGL_SYNC_NATIVE_FENCE_ANDROID extension this should
> "just work" (tm) for the GL renderer. It certainly did for me. CPU-based
> renderers can poll() to wait.

Hi Tomek,

with Weston it was decided not to implement a poll() based wait at
first as implementing that properly (not blocking the compositor) would
be a big hassle and no-one could see the benefit of it given what
clients could actually produce.

Therefore the acquire fence can only apply to buffers which can be
pipelined to a GPU. Mesa EGL is using zwp_linux_dmabuf, but the support
could be extended to opaque EGL buffers very well. We just chose to
start small and bring up the infrastructure around fences first.

Restrictions on buffer types etc. can certainly be lifted in the future
if there are good use cases. I presume you have a driver stack that
relies on the opaque EGL buffers and not zwp_linux_dmabuf any time soon?

Would anyone ever use an acquire fence with wl_shm buffers? That sounds
fundamentally wrong to me as one cannot create fences to be signalled
by userspace AFAIK. Therefore buffers whose wait cannot be pipelined to
the GPU or the display device do not make much sense to me.

> The type of buffer used is an orthogonal problem. The
> EGL_WL_bind_wayland_display
> extension takes care of GL clients' buffers in GL renderer, for anything
> else the renderer needs to know how to get pixels and use whatever means to
> put those pixels on screen.

Yeah, support for opaque EGL buffers could be added, just need to think
of a good wording, since acquire fences do not make sense for all
buffer types. A compositor must be allowed to raise protocol errors for
fence+buffer combinations it cannot use, which means that clients must
know in advance what they cannot use.


Thanks,
pq

> On Tue, 13 Nov 2018 at 09:33, Tomek Bury <tomek.bury@gmail.com> wrote:
> 
> > Thanks!
> >
> > On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <  
> > alexandros.frantzis@collabora.com> wrote:  
> >  
> >> On Mon, Nov 12, 2018 at 12:39:58PM +0000, Tomek Bury wrote:  
> >> > On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <daniel@fooishbar.org>  
> >> wrote:  
> >> >  
> >> > > On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <ppaalanen@gmail.com>  
> >> wrote:  
> >> > > > I can add that while pushing upstream, if there are no other changes
> >> > > > coming.
> >> > > >
> >> > > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
> >> > > >
> >> > > > You have ensured that the C files generated from this revision build
> >> > > > fine in Weston, right?
> >> > > >
> >> > > > David, Daniel, since your name is in the maintainers, can I have  
> >> your  
> >> > > > R-b, please?  
> >> > >
> >> > > The protocol is:
> >> > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> >> > >
> >> > > The Weston implementation looks pretty good so far, though there's no
> >> > > full implementation of release yet.
> >> > >
> >> > > Cheers,
> >> > > Daniel
> >> > > _______________________________________________
> >> > > wayland-devel mailing list
> >> > > wayland-devel@lists.freedesktop.org
> >> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >> > >  
> >> >
> >> > HI Daniel,
> >> >
> >> > Where can I find the work-in-progress implementation? I'd like to try it
> >> > out with Broadcom driver which doesn't have implicit cross-process  
> >> sync. I  
> >> > can add the explicit sync protocol implementation on the driver side but
> >> > I'd need a reference to test it against.
> >> >
> >> > Cheers,
> >> > Tomek  
> >>
> >> Hi Tomek,
> >>
> >> the WIP implementation can be found here [1]. I hope to push an update,
> >> including some zwp_buffer_release_v1 correctness fixes, in the following
> >> days.
> >>
> >> Thanks,
> >> Alexandros
> >>
> >> [1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
> >>  
> >
Tomek Bury Nov. 23, 2018, 4:26 p.m.
Hi Pekka,

> I presume you have a driver stack that relies on the opaque EGL buffers
and not zwp_linux_dmabuf any time soon?
Yes, exactly. I've added a protocol extension for sharing those buffers and
our eglCreateImage() implementation can import such buffers into the driver
on the compositor end. The buffers are carried by an fd to the compositor
that's the only similarity. They're not dma-buf.

> Yeah, support for opaque EGL buffers could be added, just need to think
of a good wording, since acquire fences do not make sense for all buffer
types.
Isn't that renderer's and/or backend's decision? The GL renderer can accept
fence with any buffer it can send to the 3D driver, so, effectively,
anything backed by available EGL image extensions. Someone may add a custom
backend and/or renderer using whatever hardware or API they have at hand. A
Vulkan renderer could potentially use fences with anything a Vulkan driver
is capable of importing. A renderer that does the CPU wait could be useful
at least for debugging. So I wouln't block the explicit sync at the
compositor level based on the white list.

Cheers,
Tomek


On Fri, 23 Nov 2018 at 13:47, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Fri, 23 Nov 2018 13:07:37 +0000
> Tomek Bury <tomek.bury@gmail.com> wrote:
>
> > Hi Alexandros,
> >
> > Sorry for a delay. I've finally got an end-to-end system to test it out.
> It
> > took some time because Weston backend I wrote a while back needed serious
> > rework to catch up with latest changes.
> >
> > There's one thing that didn't work for me. In compositor you reject
> > anything that isn't a DMA buffer and then in glrenderer you put an extra
> > assertion. Why? All you do is use an EGL extension in order to import
> > external fence_fd. There's no dmabuf dependency there. As long as the EGL
> > implementation exposes EGL_SYNC_NATIVE_FENCE_ANDROID extension this
> should
> > "just work" (tm) for the GL renderer. It certainly did for me. CPU-based
> > renderers can poll() to wait.
>
> Hi Tomek,
>
> with Weston it was decided not to implement a poll() based wait at
> first as implementing that properly (not blocking the compositor) would
> be a big hassle and no-one could see the benefit of it given what
> clients could actually produce.
>
> Therefore the acquire fence can only apply to buffers which can be
> pipelined to a GPU. Mesa EGL is using zwp_linux_dmabuf, but the support
> could be extended to opaque EGL buffers very well. We just chose to
> start small and bring up the infrastructure around fences first.
>
> Restrictions on buffer types etc. can certainly be lifted in the future
> if there are good use cases. I presume you have a driver stack that
> relies on the opaque EGL buffers and not zwp_linux_dmabuf any time soon?
>
> Would anyone ever use an acquire fence with wl_shm buffers? That sounds
> fundamentally wrong to me as one cannot create fences to be signalled
> by userspace AFAIK. Therefore buffers whose wait cannot be pipelined to
> the GPU or the display device do not make much sense to me.
>
> > The type of buffer used is an orthogonal problem. The
> > EGL_WL_bind_wayland_display
> > extension takes care of GL clients' buffers in GL renderer, for anything
> > else the renderer needs to know how to get pixels and use whatever means
> to
> > put those pixels on screen.
>
> Yeah, support for opaque EGL buffers could be added, just need to think
> of a good wording, since acquire fences do not make sense for all
> buffer types. A compositor must be allowed to raise protocol errors for
> fence+buffer combinations it cannot use, which means that clients must
> know in advance what they cannot use.
>
>
> Thanks,
> pq
>
> > On Tue, 13 Nov 2018 at 09:33, Tomek Bury <tomek.bury@gmail.com> wrote:
> >
> > > Thanks!
> > >
> > > On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
> > > alexandros.frantzis@collabora.com> wrote:
> > >
> > >> On Mon, Nov 12, 2018 at 12:39:58PM +0000, Tomek Bury wrote:
> > >> > On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <daniel@fooishbar.org>
>
> > >> wrote:
> > >> >
> > >> > > On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <ppaalanen@gmail.com>
>
> > >> wrote:
> > >> > > > I can add that while pushing upstream, if there are no other
> changes
> > >> > > > coming.
> > >> > > >
> > >> > > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
> > >> > > >
> > >> > > > You have ensured that the C files generated from this revision
> build
> > >> > > > fine in Weston, right?
> > >> > > >
> > >> > > > David, Daniel, since your name is in the maintainers, can I
> have
> > >> your
> > >> > > > R-b, please?
> > >> > >
> > >> > > The protocol is:
> > >> > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > >> > >
> > >> > > The Weston implementation looks pretty good so far, though
> there's no
> > >> > > full implementation of release yet.
> > >> > >
> > >> > > Cheers,
> > >> > > Daniel
> > >> > > _______________________________________________
> > >> > > wayland-devel mailing list
> > >> > > wayland-devel@lists.freedesktop.org
> > >> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > >> > >
> > >> >
> > >> > HI Daniel,
> > >> >
> > >> > Where can I find the work-in-progress implementation? I'd like to
> try it
> > >> > out with Broadcom driver which doesn't have implicit cross-process
> > >> sync. I
> > >> > can add the explicit sync protocol implementation on the driver
> side but
> > >> > I'd need a reference to test it against.
> > >> >
> > >> > Cheers,
> > >> > Tomek
> > >>
> > >> Hi Tomek,
> > >>
> > >> the WIP implementation can be found here [1]. I hope to push an
> update,
> > >> including some zwp_buffer_release_v1 correctness fixes, in the
> following
> > >> days.
> > >>
> > >> Thanks,
> > >> Alexandros
> > >>
> > >> [1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
> > >>
> > >
>
>
Pekka Paalanen Nov. 26, 2018, 9:21 a.m.
On Fri, 23 Nov 2018 16:26:19 +0000
Tomek Bury <tomek.bury@gmail.com> wrote:

> Hi Pekka,
> 
> > I presume you have a driver stack that relies on the opaque EGL buffers  
> and not zwp_linux_dmabuf any time soon?
> Yes, exactly. I've added a protocol extension for sharing those buffers and
> our eglCreateImage() implementation can import such buffers into the driver
> on the compositor end. The buffers are carried by an fd to the compositor
> that's the only similarity. They're not dma-buf.
> 
> > Yeah, support for opaque EGL buffers could be added, just need to think  
> of a good wording, since acquire fences do not make sense for all buffer
> types.

> Isn't that renderer's and/or backend's decision? The GL renderer can accept
> fence with any buffer it can send to the 3D driver, so, effectively,
> anything backed by available EGL image extensions. Someone may add a custom
> backend and/or renderer using whatever hardware or API they have at hand. A
> Vulkan renderer could potentially use fences with anything a Vulkan driver
> is capable of importing. A renderer that does the CPU wait could be useful
> at least for debugging. So I wouln't block the explicit sync at the
> compositor level based on the white list.

Hi Tomek,

fences do not make sense to all buffer types to begin with, today. My
objection is to allowing fencing buffer types that cannot be sent to
the 3D driver, e.g. wl_shm which is usually copied through glTexImage2D
and friends. We cannot ignore those in the spec language.

A renderer (a compositor really, we're not talking about just Weston)
decides what buffer types it accepts, yes. This is communicated to
clients through which buffer factory interface globals are being
advertised. Each type is a different protocol extension. The fence
extension OTOH is just a single extension, and currently there is no
protocol to negotiate which buffer types are usable with acquire
fences. The first attempt is to define in the spec language what will
always be supported, provided the buffer factory exists.

The opaque EGL buffer type is really just one type in practise:
compositors and clients use it through a well-known, single API: EGL.
It does not matter that there are multiple incompatible EGL
implementations, it all looks like just one opaque buffer type to
compositors. I think this makes it easier to extend the fence spec
wording to require opaque EGL buffers to be supported.

Either the fence protocol spec needs to be clear on what works, or we
need advertisement events to let clients know in advance what the
compositor supports. A client sending a fence that the compositor
cannot use must not be possible; compositor, client, EGL, driver, etc.
bugs notwithstanding.

Btw. I just realized that if client-side EGL uses the fence extension
internally, that means the client app code must not attempt to add or
request fences of its own, because the spec disallows multiple acquire
fences and multiple release notification requests. I suppose that's
fine?

Alf, can you come up with changes to the spec wording and Weston to
require opaque EGL buffers are supported?

On one hand it is actually a little strange to couple opaque EGL
buffers (a private, EGL implementation specific protocol interface)
with a generic fencing extension, but maybe that is necessary because
there is not enough compositor-side GBM and EGL API so that the EGL
implementation could handle it all in an EGL implementation specific
interface?


Thanks,
pq

> On Fri, 23 Nov 2018 at 13:47, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > On Fri, 23 Nov 2018 13:07:37 +0000
> > Tomek Bury <tomek.bury@gmail.com> wrote:
> >  
> > > Hi Alexandros,
> > >
> > > Sorry for a delay. I've finally got an end-to-end system to test it out.  
> > It  
> > > took some time because Weston backend I wrote a while back needed serious
> > > rework to catch up with latest changes.
> > >
> > > There's one thing that didn't work for me. In compositor you reject
> > > anything that isn't a DMA buffer and then in glrenderer you put an extra
> > > assertion. Why? All you do is use an EGL extension in order to import
> > > external fence_fd. There's no dmabuf dependency there. As long as the EGL
> > > implementation exposes EGL_SYNC_NATIVE_FENCE_ANDROID extension this  
> > should  
> > > "just work" (tm) for the GL renderer. It certainly did for me. CPU-based
> > > renderers can poll() to wait.  
> >
> > Hi Tomek,
> >
> > with Weston it was decided not to implement a poll() based wait at
> > first as implementing that properly (not blocking the compositor) would
> > be a big hassle and no-one could see the benefit of it given what
> > clients could actually produce.
> >
> > Therefore the acquire fence can only apply to buffers which can be
> > pipelined to a GPU. Mesa EGL is using zwp_linux_dmabuf, but the support
> > could be extended to opaque EGL buffers very well. We just chose to
> > start small and bring up the infrastructure around fences first.
> >
> > Restrictions on buffer types etc. can certainly be lifted in the future
> > if there are good use cases. I presume you have a driver stack that
> > relies on the opaque EGL buffers and not zwp_linux_dmabuf any time soon?
> >
> > Would anyone ever use an acquire fence with wl_shm buffers? That sounds
> > fundamentally wrong to me as one cannot create fences to be signalled
> > by userspace AFAIK. Therefore buffers whose wait cannot be pipelined to
> > the GPU or the display device do not make much sense to me.
> >  
> > > The type of buffer used is an orthogonal problem. The
> > > EGL_WL_bind_wayland_display
> > > extension takes care of GL clients' buffers in GL renderer, for anything
> > > else the renderer needs to know how to get pixels and use whatever means  
> > to  
> > > put those pixels on screen.  
> >
> > Yeah, support for opaque EGL buffers could be added, just need to think
> > of a good wording, since acquire fences do not make sense for all
> > buffer types. A compositor must be allowed to raise protocol errors for
> > fence+buffer combinations it cannot use, which means that clients must
> > know in advance what they cannot use.
> >
> >
> > Thanks,
> > pq
> >  
> > > On Tue, 13 Nov 2018 at 09:33, Tomek Bury <tomek.bury@gmail.com> wrote:
> > >  
> > > > Thanks!
> > > >
> > > > On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <  
> > > > alexandros.frantzis@collabora.com> wrote:  
> > > >  
> > > >> On Mon, Nov 12, 2018 at 12:39:58PM +0000, Tomek Bury wrote:  
> > > >> > On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <daniel@fooishbar.org>  
> >  
> > > >> wrote:  
> > > >> >  
> > > >> > > On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <ppaalanen@gmail.com>  
> >  
> > > >> wrote:  
> > > >> > > > I can add that while pushing upstream, if there are no other  
> > changes  
> > > >> > > > coming.
> > > >> > > >
> > > >> > > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
> > > >> > > >
> > > >> > > > You have ensured that the C files generated from this revision  
> > build  
> > > >> > > > fine in Weston, right?
> > > >> > > >
> > > >> > > > David, Daniel, since your name is in the maintainers, can I  
> > have  
> > > >> your  
> > > >> > > > R-b, please?  
> > > >> > >
> > > >> > > The protocol is:
> > > >> > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > > >> > >
> > > >> > > The Weston implementation looks pretty good so far, though  
> > there's no  
> > > >> > > full implementation of release yet.
> > > >> > >
> > > >> > > Cheers,
> > > >> > > Daniel
> > > >> > > _______________________________________________
> > > >> > > wayland-devel mailing list
> > > >> > > wayland-devel@lists.freedesktop.org
> > > >> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > > >> > >  
> > > >> >
> > > >> > HI Daniel,
> > > >> >
> > > >> > Where can I find the work-in-progress implementation? I'd like to  
> > try it  
> > > >> > out with Broadcom driver which doesn't have implicit cross-process  
> > > >> sync. I  
> > > >> > can add the explicit sync protocol implementation on the driver  
> > side but  
> > > >> > I'd need a reference to test it against.
> > > >> >
> > > >> > Cheers,
> > > >> > Tomek  
> > > >>
> > > >> Hi Tomek,
> > > >>
> > > >> the WIP implementation can be found here [1]. I hope to push an  
> > update,  
> > > >> including some zwp_buffer_release_v1 correctness fixes, in the  
> > following  
> > > >> days.
> > > >>
> > > >> Thanks,
> > > >> Alexandros
> > > >>
> > > >> [1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
> > > >>  
> > > >  
> >
> >
Tomek Bury Nov. 26, 2018, 3:14 p.m.
Hi Pekka,

Yes, sorry, I was writing specifically about Weston implementation. In the
merge request from Alexandros the actual compatibility check is in the main
compositor, while compositor doesn't have enough information to decide
whether the selected renderer can handle buffer+fence combination or not.

As for the opaque wl_buffer, that's an internal implementation detail of
Vulkan WSI or EGL so different drivers can choose to do different things
here. It's the 3D driver, and *NOT* the client application, that creates
those buffers and sends attach/damage/commit sequence to the compositor.
The 3D driver makes a decision what type of buffer to use, an EGL or Vulkan
client application doesn't have any means of accessing wl_buffer objects,
it's all hidden away.

On the compositor side buffers are received through one of the protocol
extensions you've mentioned. The compositor has a choice to make. It can
either probe the wl_buffer for known buffer types or leave that task to the
EGL implementation. Let's say a client-side driver uses dma-buf buffers for
its swapchain images. If the compositor knows how to handle dma-buf, it can
either directly access those buffers or it can hand them over to the
compositor-side 3D driver through
eglCreateImage(dpy,  EGL_LINUX_DMA_BUF_EXT, ...). If the compositor doesn't
know the particular type of buffer, it can check if EGL can handle it
either through eglCreateImage(dpy,  EGL_WAYLAND_BUFFER_WL, ...) or
eglQueryWaylandBufferWL(...).
If the client and compositor use the same 3D driver (that's the most likely
scenario) this is bound to work. In multiple-GPU configurations your
mileage may vary though.

If the EGL implementation can use the type of the wl_buffer and can import
the fence fd, you're home. Having said that, I can't think of a way of
figuring out ahead of time what type of wl_buffer the client-side driver is
going to use for its swapchain and whether this particular type will be
accepted by the compositor-side driver.

This is how I use it at the moment: I've written a custom Weston backend
because the code runs on top of an embedded middleware. My backend always
uses GL renderer. The GL renderer has to call eglBindWaylandDisplayWL() at
startup, and the implementation of that API in the 3D driver adds a custom
Wayland protocol extension for sharing buffers. Now the scene is set. When
a Wayland client application starts, the EGL or Vulkan WSI implementation
driver goes for that extension and bails out if unavailable. This way
swapchain buffers from EGL and Vulkan client can be used by Weston's GL
renderer without any knowledge of the embedded platform details.

With regards to using fences directly by the client app, I guess it's the
same principle as drawing into the window. Either client app does
everything "by hand" or lets the Vulkan or EGL/GLES do it. If the app is in
charge, the app manages the window swap chain buffers and synchronization,
otherwise the 3D driver does it. You shouldn't allow more than one thing
managing the Wayland window at the same time. Perhaps you could use wording
similar to Vulkan WSI or EGL window surface when describing what is and
what isn't allowed when it comes to Wayland windows.

Cheers,
Tomek



On Mon, 26 Nov 2018 at 09:22, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Fri, 23 Nov 2018 16:26:19 +0000
> Tomek Bury <tomek.bury@gmail.com> wrote:
>
> > Hi Pekka,
> >
> > > I presume you have a driver stack that relies on the opaque EGL
> buffers
> > and not zwp_linux_dmabuf any time soon?
> > Yes, exactly. I've added a protocol extension for sharing those buffers
> and
> > our eglCreateImage() implementation can import such buffers into the
> driver
> > on the compositor end. The buffers are carried by an fd to the compositor
> > that's the only similarity. They're not dma-buf.
> >
> > > Yeah, support for opaque EGL buffers could be added, just need to
> think
> > of a good wording, since acquire fences do not make sense for all buffer
> > types.
>
> > Isn't that renderer's and/or backend's decision? The GL renderer can
> accept
> > fence with any buffer it can send to the 3D driver, so, effectively,
> > anything backed by available EGL image extensions. Someone may add a
> custom
> > backend and/or renderer using whatever hardware or API they have at
> hand. A
> > Vulkan renderer could potentially use fences with anything a Vulkan
> driver
> > is capable of importing. A renderer that does the CPU wait could be
> useful
> > at least for debugging. So I wouln't block the explicit sync at the
> > compositor level based on the white list.
>
> Hi Tomek,
>
> fences do not make sense to all buffer types to begin with, today. My
> objection is to allowing fencing buffer types that cannot be sent to
> the 3D driver, e.g. wl_shm which is usually copied through glTexImage2D
> and friends. We cannot ignore those in the spec language.
>
> A renderer (a compositor really, we're not talking about just Weston)
> decides what buffer types it accepts, yes. This is communicated to
> clients through which buffer factory interface globals are being
> advertised. Each type is a different protocol extension. The fence
> extension OTOH is just a single extension, and currently there is no
> protocol to negotiate which buffer types are usable with acquire
> fences. The first attempt is to define in the spec language what will
> always be supported, provided the buffer factory exists.
>
> The opaque EGL buffer type is really just one type in practise:
> compositors and clients use it through a well-known, single API: EGL.
> It does not matter that there are multiple incompatible EGL
> implementations, it all looks like just one opaque buffer type to
> compositors. I think this makes it easier to extend the fence spec
> wording to require opaque EGL buffers to be supported.
>
> Either the fence protocol spec needs to be clear on what works, or we
> need advertisement events to let clients know in advance what the
> compositor supports. A client sending a fence that the compositor
> cannot use must not be possible; compositor, client, EGL, driver, etc.
> bugs notwithstanding.
>
> Btw. I just realized that if client-side EGL uses the fence extension
> internally, that means the client app code must not attempt to add or
> request fences of its own, because the spec disallows multiple acquire
> fences and multiple release notification requests. I suppose that's
> fine?
>
> Alf, can you come up with changes to the spec wording and Weston to
> require opaque EGL buffers are supported?
>
> On one hand it is actually a little strange to couple opaque EGL
> buffers (a private, EGL implementation specific protocol interface)
> with a generic fencing extension, but maybe that is necessary because
> there is not enough compositor-side GBM and EGL API so that the EGL
> implementation could handle it all in an EGL implementation specific
> interface?
>
>
> Thanks,
> pq
>
> > On Fri, 23 Nov 2018 at 13:47, Pekka Paalanen <ppaalanen@gmail.com>
> wrote:
> >
> > > On Fri, 23 Nov 2018 13:07:37 +0000
> > > Tomek Bury <tomek.bury@gmail.com> wrote:
> > >
> > > > Hi Alexandros,
> > > >
> > > > Sorry for a delay. I've finally got an end-to-end system to test it
> out.
> > > It
> > > > took some time because Weston backend I wrote a while back needed
> serious
> > > > rework to catch up with latest changes.
> > > >
> > > > There's one thing that didn't work for me. In compositor you reject
> > > > anything that isn't a DMA buffer and then in glrenderer you put an
> extra
> > > > assertion. Why? All you do is use an EGL extension in order to import
> > > > external fence_fd. There's no dmabuf dependency there. As long as
> the EGL
> > > > implementation exposes EGL_SYNC_NATIVE_FENCE_ANDROID extension this
> > > should
> > > > "just work" (tm) for the GL renderer. It certainly did for me.
> CPU-based
> > > > renderers can poll() to wait.
> > >
> > > Hi Tomek,
> > >
> > > with Weston it was decided not to implement a poll() based wait at
> > > first as implementing that properly (not blocking the compositor) would
> > > be a big hassle and no-one could see the benefit of it given what
> > > clients could actually produce.
> > >
> > > Therefore the acquire fence can only apply to buffers which can be
> > > pipelined to a GPU. Mesa EGL is using zwp_linux_dmabuf, but the support
> > > could be extended to opaque EGL buffers very well. We just chose to
> > > start small and bring up the infrastructure around fences first.
> > >
> > > Restrictions on buffer types etc. can certainly be lifted in the future
> > > if there are good use cases. I presume you have a driver stack that
> > > relies on the opaque EGL buffers and not zwp_linux_dmabuf any time
> soon?
> > >
> > > Would anyone ever use an acquire fence with wl_shm buffers? That sounds
> > > fundamentally wrong to me as one cannot create fences to be signalled
> > > by userspace AFAIK. Therefore buffers whose wait cannot be pipelined to
> > > the GPU or the display device do not make much sense to me.
> > >
> > > > The type of buffer used is an orthogonal problem. The
> > > > EGL_WL_bind_wayland_display
> > > > extension takes care of GL clients' buffers in GL renderer, for
> anything
> > > > else the renderer needs to know how to get pixels and use whatever
> means
> > > to
> > > > put those pixels on screen.
> > >
> > > Yeah, support for opaque EGL buffers could be added, just need to think
> > > of a good wording, since acquire fences do not make sense for all
> > > buffer types. A compositor must be allowed to raise protocol errors for
> > > fence+buffer combinations it cannot use, which means that clients must
> > > know in advance what they cannot use.
> > >
> > >
> > > Thanks,
> > > pq
> > >
> > > > On Tue, 13 Nov 2018 at 09:33, Tomek Bury <tomek.bury@gmail.com>
> wrote:
> > > >
> > > > > Thanks!
> > > > >
> > > > > On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
> > > > > alexandros.frantzis@collabora.com> wrote:
> > > > >
> > > > >> On Mon, Nov 12, 2018 at 12:39:58PM +0000, Tomek Bury wrote:
> > > > >> > On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <
> daniel@fooishbar.org>
> > >
> > > > >> wrote:
> > > > >> >
> > > > >> > > On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <
> ppaalanen@gmail.com>
> > >
> > > > >> wrote:
> > > > >> > > > I can add that while pushing upstream, if there are no
> other
> > > changes
> > > > >> > > > coming.
> > > > >> > > >
> > > > >> > > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk
> >
> > > > >> > > >
> > > > >> > > > You have ensured that the C files generated from this
> revision
> > > build
> > > > >> > > > fine in Weston, right?
> > > > >> > > >
> > > > >> > > > David, Daniel, since your name is in the maintainers, can
> I
> > > have
> > > > >> your
> > > > >> > > > R-b, please?
> > > > >> > >
> > > > >> > > The protocol is:
> > > > >> > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > > > >> > >
> > > > >> > > The Weston implementation looks pretty good so far, though
> > > there's no
> > > > >> > > full implementation of release yet.
> > > > >> > >
> > > > >> > > Cheers,
> > > > >> > > Daniel
> > > > >> > > _______________________________________________
> > > > >> > > wayland-devel mailing list
> > > > >> > > wayland-devel@lists.freedesktop.org
> > > > >> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > > > >> > >
> > > > >> >
> > > > >> > HI Daniel,
> > > > >> >
> > > > >> > Where can I find the work-in-progress implementation? I'd like
> to
> > > try it
> > > > >> > out with Broadcom driver which doesn't have implicit
> cross-process
> > > > >> sync. I
> > > > >> > can add the explicit sync protocol implementation on the
> driver
> > > side but
> > > > >> > I'd need a reference to test it against.
> > > > >> >
> > > > >> > Cheers,
> > > > >> > Tomek
> > > > >>
> > > > >> Hi Tomek,
> > > > >>
> > > > >> the WIP implementation can be found here [1]. I hope to push an
> > > update,
> > > > >> including some zwp_buffer_release_v1 correctness fixes, in the
> > > following
> > > > >> days.
> > > > >>
> > > > >> Thanks,
> > > > >> Alexandros
> > > > >>
> > > > >> [1]
> https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
> > > > >>
> > > > >
> > >
> > >
>
>
Pekka Paalanen Nov. 27, 2018, 8:53 a.m.
On Mon, 26 Nov 2018 15:14:45 +0000
Tomek Bury <tomek.bury@gmail.com> wrote:

> Hi Pekka,
> 
> Yes, sorry, I was writing specifically about Weston implementation. In the
> merge request from Alexandros the actual compatibility check is in the main
> compositor, while compositor doesn't have enough information to decide
> whether the selected renderer can handle buffer+fence combination or not.
> 
> As for the opaque wl_buffer, that's an internal implementation detail of
> Vulkan WSI or EGL so different drivers can choose to do different things
> here. It's the 3D driver, and *NOT* the client application, that creates
> those buffers and sends attach/damage/commit sequence to the compositor.
> The 3D driver makes a decision what type of buffer to use, an EGL or Vulkan
> client application doesn't have any means of accessing wl_buffer objects,
> it's all hidden away.

Hi Tomek,

I suppose that applies to the opaque EGL buffers only?

Otherwise, a client could use e.g. EGL in a different way to get dmabuf,
send those manually to the compositor and set up fences manually as
well. weston-simple-dmabuf-egl in the MR is one variant of that. Of
course, it depends on the EGL stack supporting dmabuf to begin with, so
if yours doesn't then it's not possible.

However, in the fence protocol extension specification we must take all
possible use cases into account. The client side of the fence extension
is not exclusive to EGL implementations, and the server side is even
less, because the server side implementation cannot be an EGL
implementation detail.

This is why I'm talking about protocol spec - the Weston implementation
just reflects the spec, or at least attempts to.

> On the compositor side buffers are received through one of the protocol
> extensions you've mentioned. The compositor has a choice to make. It can
> either probe the wl_buffer for known buffer types or leave that task to the
> EGL implementation. Let's say a client-side driver uses dma-buf buffers for
> its swapchain images. If the compositor knows how to handle dma-buf, it can
> either directly access those buffers or it can hand them over to the
> compositor-side 3D driver through
> eglCreateImage(dpy,  EGL_LINUX_DMA_BUF_EXT, ...). If the compositor doesn't
> know the particular type of buffer, it can check if EGL can handle it
> either through eglCreateImage(dpy,  EGL_WAYLAND_BUFFER_WL, ...) or
> eglQueryWaylandBufferWL(...).
> If the client and compositor use the same 3D driver (that's the most likely
> scenario) this is bound to work. In multiple-GPU configurations your
> mileage may vary though.

Right.

A wl_buffer can be used with either EGL_WAYLAND_BUFFER_WL or as a
dmabuf via EGL_LINUX_DMA_BUF, but there is no choice for a compositor
to make as at most only one these can work for a given wl_buffer.

> If the EGL implementation can use the type of the wl_buffer and can import
> the fence fd, you're home. Having said that, I can't think of a way of
> figuring out ahead of time what type of wl_buffer the client-side driver is
> going to use for its swapchain and whether this particular type will be
> accepted by the compositor-side driver.

There is no need for the compositor to know in advance. Instead, the
client must know what the compositor supports, since the client is
making the decisions on buffer allocation etc. (or the decision to use
EGL or Vulkan and not care about buffers explicitly, in which case the
EGL/WSI implementation must know).

The opaque EGL buffers already rely on using the same EGL
implementation on both compositor and client, so if a compositor also
exposes the fence extension, the client-side EGL implementation can be
sure it can use the fence extension with the opaque EGL buffers (or if
it cannot, the client-side EGL implementation already knows that
because it knows the details of the compositor-side EGL implementation).

That is, if we write that down in the fence extension spec. I think we
should.

> This is how I use it at the moment: I've written a custom Weston backend
> because the code runs on top of an embedded middleware. My backend always
> uses GL renderer. The GL renderer has to call eglBindWaylandDisplayWL() at
> startup, and the implementation of that API in the 3D driver adds a custom
> Wayland protocol extension for sharing buffers. Now the scene is set. When
> a Wayland client application starts, the EGL or Vulkan WSI implementation
> driver goes for that extension and bails out if unavailable. This way
> swapchain buffers from EGL and Vulkan client can be used by Weston's GL
> renderer without any knowledge of the embedded platform details.

Yup, that is how the "opaque EGL buffer" type is supposed to work.

> With regards to using fences directly by the client app, I guess it's the
> same principle as drawing into the window. Either client app does
> everything "by hand" or lets the Vulkan or EGL/GLES do it. If the app is in
> charge, the app manages the window swap chain buffers and synchronization,
> otherwise the 3D driver does it. You shouldn't allow more than one thing
> managing the Wayland window at the same time. Perhaps you could use wording
> similar to Vulkan WSI or EGL window surface when describing what is and
> what isn't allowed when it comes to Wayland windows.

Yes, we probably should have some wording that if a client is letting
something like EGL to commit the buffers, it must not attempt to use
the fence extension on that wl_surface itself because EGL will probably
be using the extension already.

Alf?


Thanks,
pq

> On Mon, 26 Nov 2018 at 09:22, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > On Fri, 23 Nov 2018 16:26:19 +0000
> > Tomek Bury <tomek.bury@gmail.com> wrote:
> >  
> > > Hi Pekka,
> > >  
> > > > I presume you have a driver stack that relies on the opaque EGL  
> > buffers  
> > > and not zwp_linux_dmabuf any time soon?
> > > Yes, exactly. I've added a protocol extension for sharing those buffers  
> > and  
> > > our eglCreateImage() implementation can import such buffers into the  
> > driver  
> > > on the compositor end. The buffers are carried by an fd to the compositor
> > > that's the only similarity. They're not dma-buf.
> > >  
> > > > Yeah, support for opaque EGL buffers could be added, just need to  
> > think  
> > > of a good wording, since acquire fences do not make sense for all buffer
> > > types.  
> >  
> > > Isn't that renderer's and/or backend's decision? The GL renderer can  
> > accept  
> > > fence with any buffer it can send to the 3D driver, so, effectively,
> > > anything backed by available EGL image extensions. Someone may add a  
> > custom  
> > > backend and/or renderer using whatever hardware or API they have at  
> > hand. A  
> > > Vulkan renderer could potentially use fences with anything a Vulkan  
> > driver  
> > > is capable of importing. A renderer that does the CPU wait could be  
> > useful  
> > > at least for debugging. So I wouln't block the explicit sync at the
> > > compositor level based on the white list.  
> >
> > Hi Tomek,
> >
> > fences do not make sense to all buffer types to begin with, today. My
> > objection is to allowing fencing buffer types that cannot be sent to
> > the 3D driver, e.g. wl_shm which is usually copied through glTexImage2D
> > and friends. We cannot ignore those in the spec language.
> >
> > A renderer (a compositor really, we're not talking about just Weston)
> > decides what buffer types it accepts, yes. This is communicated to
> > clients through which buffer factory interface globals are being
> > advertised. Each type is a different protocol extension. The fence
> > extension OTOH is just a single extension, and currently there is no
> > protocol to negotiate which buffer types are usable with acquire
> > fences. The first attempt is to define in the spec language what will
> > always be supported, provided the buffer factory exists.
> >
> > The opaque EGL buffer type is really just one type in practise:
> > compositors and clients use it through a well-known, single API: EGL.
> > It does not matter that there are multiple incompatible EGL
> > implementations, it all looks like just one opaque buffer type to
> > compositors. I think this makes it easier to extend the fence spec
> > wording to require opaque EGL buffers to be supported.
> >
> > Either the fence protocol spec needs to be clear on what works, or we
> > need advertisement events to let clients know in advance what the
> > compositor supports. A client sending a fence that the compositor
> > cannot use must not be possible; compositor, client, EGL, driver, etc.
> > bugs notwithstanding.
> >
> > Btw. I just realized that if client-side EGL uses the fence extension
> > internally, that means the client app code must not attempt to add or
> > request fences of its own, because the spec disallows multiple acquire
> > fences and multiple release notification requests. I suppose that's
> > fine?
> >
> > Alf, can you come up with changes to the spec wording and Weston to
> > require opaque EGL buffers are supported?
> >
> > On one hand it is actually a little strange to couple opaque EGL
> > buffers (a private, EGL implementation specific protocol interface)
> > with a generic fencing extension, but maybe that is necessary because
> > there is not enough compositor-side GBM and EGL API so that the EGL
> > implementation could handle it all in an EGL implementation specific
> > interface?
> >
> >
> > Thanks,
> > pq
> >  
> > > On Fri, 23 Nov 2018 at 13:47, Pekka Paalanen <ppaalanen@gmail.com>  
> > wrote:  
> > >  
> > > > On Fri, 23 Nov 2018 13:07:37 +0000
> > > > Tomek Bury <tomek.bury@gmail.com> wrote:
> > > >  
> > > > > Hi Alexandros,
> > > > >
> > > > > Sorry for a delay. I've finally got an end-to-end system to test it  
> > out.  
> > > > It  
> > > > > took some time because Weston backend I wrote a while back needed  
> > serious  
> > > > > rework to catch up with latest changes.
> > > > >
> > > > > There's one thing that didn't work for me. In compositor you reject
> > > > > anything that isn't a DMA buffer and then in glrenderer you put an  
> > extra  
> > > > > assertion. Why? All you do is use an EGL extension in order to import
> > > > > external fence_fd. There's no dmabuf dependency there. As long as  
> > the EGL  
> > > > > implementation exposes EGL_SYNC_NATIVE_FENCE_ANDROID extension this  
> > > > should  
> > > > > "just work" (tm) for the GL renderer. It certainly did for me.  
> > CPU-based  
> > > > > renderers can poll() to wait.  
> > > >
> > > > Hi Tomek,
> > > >
> > > > with Weston it was decided not to implement a poll() based wait at
> > > > first as implementing that properly (not blocking the compositor) would
> > > > be a big hassle and no-one could see the benefit of it given what
> > > > clients could actually produce.
> > > >
> > > > Therefore the acquire fence can only apply to buffers which can be
> > > > pipelined to a GPU. Mesa EGL is using zwp_linux_dmabuf, but the support
> > > > could be extended to opaque EGL buffers very well. We just chose to
> > > > start small and bring up the infrastructure around fences first.
> > > >
> > > > Restrictions on buffer types etc. can certainly be lifted in the future
> > > > if there are good use cases. I presume you have a driver stack that
> > > > relies on the opaque EGL buffers and not zwp_linux_dmabuf any time  
> > soon?  
> > > >
> > > > Would anyone ever use an acquire fence with wl_shm buffers? That sounds
> > > > fundamentally wrong to me as one cannot create fences to be signalled
> > > > by userspace AFAIK. Therefore buffers whose wait cannot be pipelined to
> > > > the GPU or the display device do not make much sense to me.
> > > >  
> > > > > The type of buffer used is an orthogonal problem. The
> > > > > EGL_WL_bind_wayland_display
> > > > > extension takes care of GL clients' buffers in GL renderer, for  
> > anything  
> > > > > else the renderer needs to know how to get pixels and use whatever  
> > means  
> > > > to  
> > > > > put those pixels on screen.  
> > > >
> > > > Yeah, support for opaque EGL buffers could be added, just need to think
> > > > of a good wording, since acquire fences do not make sense for all
> > > > buffer types. A compositor must be allowed to raise protocol errors for
> > > > fence+buffer combinations it cannot use, which means that clients must
> > > > know in advance what they cannot use.
> > > >
> > > >
> > > > Thanks,
> > > > pq
> > > >  
> > > > > On Tue, 13 Nov 2018 at 09:33, Tomek Bury <tomek.bury@gmail.com>  
> > wrote:  
> > > > >  
> > > > > > Thanks!
> > > > > >
> > > > > > On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <  
> > > > > > alexandros.frantzis@collabora.com> wrote:  
> > > > > >  
> > > > > >> On Mon, Nov 12, 2018 at 12:39:58PM +0000, Tomek Bury wrote:  
> > > > > >> > On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <  
> > daniel@fooishbar.org>  
> > > >  
> > > > > >> wrote:  
> > > > > >> >  
> > > > > >> > > On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <  
> > ppaalanen@gmail.com>  
> > > >  
> > > > > >> wrote:  
> > > > > >> > > > I can add that while pushing upstream, if there are no  
> > other  
> > > > changes  
> > > > > >> > > > coming.
> > > > > >> > > >
> > > > > >> > > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk  
> > >  
> > > > > >> > > >
> > > > > >> > > > You have ensured that the C files generated from this  
> > revision  
> > > > build  
> > > > > >> > > > fine in Weston, right?
> > > > > >> > > >
> > > > > >> > > > David, Daniel, since your name is in the maintainers, can  
> > I  
> > > > have  
> > > > > >> your  
> > > > > >> > > > R-b, please?  
> > > > > >> > >
> > > > > >> > > The protocol is:
> > > > > >> > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > > > > >> > >
> > > > > >> > > The Weston implementation looks pretty good so far, though  
> > > > there's no  
> > > > > >> > > full implementation of release yet.
> > > > > >> > >
> > > > > >> > > Cheers,
> > > > > >> > > Daniel
> > > > > >> > > _______________________________________________
> > > > > >> > > wayland-devel mailing list
> > > > > >> > > wayland-devel@lists.freedesktop.org
> > > > > >> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > > > > >> > >  
> > > > > >> >
> > > > > >> > HI Daniel,
> > > > > >> >
> > > > > >> > Where can I find the work-in-progress implementation? I'd like  
> > to  
> > > > try it  
> > > > > >> > out with Broadcom driver which doesn't have implicit  
> > cross-process  
> > > > > >> sync. I  
> > > > > >> > can add the explicit sync protocol implementation on the  
> > driver  
> > > > side but  
> > > > > >> > I'd need a reference to test it against.
> > > > > >> >
> > > > > >> > Cheers,
> > > > > >> > Tomek  
> > > > > >>
> > > > > >> Hi Tomek,
> > > > > >>
> > > > > >> the WIP implementation can be found here [1]. I hope to push an  
> > > > update,  
> > > > > >> including some zwp_buffer_release_v1 correctness fixes, in the  
> > > > following  
> > > > > >> days.
> > > > > >>
> > > > > >> Thanks,
> > > > > >> Alexandros
> > > > > >>
> > > > > >> [1]  
> > https://gitlab.freedesktop.org/wayland/weston/merge_requests/32  
> > > > > >>  
> > > > > >  
> > > >
> > > >  
> >
> >
Tomek Bury Nov. 27, 2018, 10:56 a.m.
Hi Pekka,

> I suppose that applies to the opaque EGL buffers only?
Sort of. As far as I understand it, the divide is between wl_surface
managed by EGL/WSI vs. wl_surface managed directly by the client
application. For EGL case the wl_surface managed by EGL would be set-up
more or less like this:
struct wl_egl_window *window = wl_egl_window_create(surface, w, h);
EGLDisplay dpy = eglGetPlatformDisplay(EGL_PLATFORM_WAYLAND_EXT, ...);
EGLSurface surface = eglCreatePlatformWindowSurface(dpy, config, window);
With this setup EGL becomes responsible for managing the swapchain buffers,
keeping up with window resizes, sending wl_buffers to compositor when
client app calls eglSwapBuffers() etc. This is how weston-simple-egl works.

> Otherwise, a client could use e.g. EGL in a different way to get dmabuf,
> send those manually to the compositor and set up fences manually as
> well. weston-simple-dmabuf-egl in the MR is one variant of that. Of
> course, it depends on the EGL stack supporting dmabuf to begin with, so
> if yours doesn't then it's not possible.
Yes, exactly. This is the example where the application manages the Wayland
window directly and the EGL driver is not involved. EGL is only used to set
up rendering into an off-screen buffer(s) of some description.

In both cases the compositor receives a wl_buffer and has to figure out how
to render the contents of such buffer. In both cases the wl_buffer could
encapsulate dma-buf and the compositor wouldn't be able to tell whether it
came directly from application (weston-simple-dmabuf-egl) or from the EGL
driver (weston-simple-egl). in fact the Waylad client could choose dma-buf,
prime, flink, shm, something else, regardless of how the wl_surface is
managed, whether it's done directly by the application, by the EGL or
Vulkan driver. And some of those wl_buffer types will be known to the
compositor, some to the 3D driver and perhaps some to both. At least that's
my understanding.

Cheers,
Tomek





On Tue, 27 Nov 2018 at 08:53, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Mon, 26 Nov 2018 15:14:45 +0000
> Tomek Bury <tomek.bury@gmail.com> wrote:
>
> > Hi Pekka,
> >
> > Yes, sorry, I was writing specifically about Weston implementation. In
> the
> > merge request from Alexandros the actual compatibility check is in the
> main
> > compositor, while compositor doesn't have enough information to decide
> > whether the selected renderer can handle buffer+fence combination or not.
> >
> > As for the opaque wl_buffer, that's an internal implementation detail of
> > Vulkan WSI or EGL so different drivers can choose to do different things
> > here. It's the 3D driver, and *NOT* the client application, that creates
> > those buffers and sends attach/damage/commit sequence to the compositor.
> > The 3D driver makes a decision what type of buffer to use, an EGL or
> Vulkan
> > client application doesn't have any means of accessing wl_buffer objects,
> > it's all hidden away.
>
> Hi Tomek,
>
> I suppose that applies to the opaque EGL buffers only?
>
> Otherwise, a client could use e.g. EGL in a different way to get dmabuf,
> send those manually to the compositor and set up fences manually as
> well. weston-simple-dmabuf-egl in the MR is one variant of that. Of
> course, it depends on the EGL stack supporting dmabuf to begin with, so
> if yours doesn't then it's not possible.
>
> However, in the fence protocol extension specification we must take all
> possible use cases into account. The client side of the fence extension
> is not exclusive to EGL implementations, and the server side is even
> less, because the server side implementation cannot be an EGL
> implementation detail.
>
> This is why I'm talking about protocol spec - the Weston implementation
> just reflects the spec, or at least attempts to.
>
> > On the compositor side buffers are received through one of the protocol
> > extensions you've mentioned. The compositor has a choice to make. It can
> > either probe the wl_buffer for known buffer types or leave that task to
> the
> > EGL implementation. Let's say a client-side driver uses dma-buf buffers
> for
> > its swapchain images. If the compositor knows how to handle dma-buf, it
> can
> > either directly access those buffers or it can hand them over to the
> > compositor-side 3D driver through
> > eglCreateImage(dpy,  EGL_LINUX_DMA_BUF_EXT, ...). If the compositor
> doesn't
> > know the particular type of buffer, it can check if EGL can handle it
> > either through eglCreateImage(dpy,  EGL_WAYLAND_BUFFER_WL, ...) or
> > eglQueryWaylandBufferWL(...).
> > If the client and compositor use the same 3D driver (that's the most
> likely
> > scenario) this is bound to work. In multiple-GPU configurations your
> > mileage may vary though.
>
> Right.
>
> A wl_buffer can be used with either EGL_WAYLAND_BUFFER_WL or as a
> dmabuf via EGL_LINUX_DMA_BUF, but there is no choice for a compositor
> to make as at most only one these can work for a given wl_buffer.
>
> > If the EGL implementation can use the type of the wl_buffer and can
> import
> > the fence fd, you're home. Having said that, I can't think of a way of
> > figuring out ahead of time what type of wl_buffer the client-side driver
> is
> > going to use for its swapchain and whether this particular type will be
> > accepted by the compositor-side driver.
>
> There is no need for the compositor to know in advance. Instead, the
> client must know what the compositor supports, since the client is
> making the decisions on buffer allocation etc. (or the decision to use
> EGL or Vulkan and not care about buffers explicitly, in which case the
> EGL/WSI implementation must know).
>
> The opaque EGL buffers already rely on using the same EGL
> implementation on both compositor and client, so if a compositor also
> exposes the fence extension, the client-side EGL implementation can be
> sure it can use the fence extension with the opaque EGL buffers (or if
> it cannot, the client-side EGL implementation already knows that
> because it knows the details of the compositor-side EGL implementation).
>
> That is, if we write that down in the fence extension spec. I think we
> should.
>
> > This is how I use it at the moment: I've written a custom Weston backend
> > because the code runs on top of an embedded middleware. My backend always
> > uses GL renderer. The GL renderer has to call eglBindWaylandDisplayWL()
> at
> > startup, and the implementation of that API in the 3D driver adds a
> custom
> > Wayland protocol extension for sharing buffers. Now the scene is set.
> When
> > a Wayland client application starts, the EGL or Vulkan WSI implementation
> > driver goes for that extension and bails out if unavailable. This way
> > swapchain buffers from EGL and Vulkan client can be used by Weston's GL
> > renderer without any knowledge of the embedded platform details.
>
> Yup, that is how the "opaque EGL buffer" type is supposed to work.
>
> > With regards to using fences directly by the client app, I guess it's the
> > same principle as drawing into the window. Either client app does
> > everything "by hand" or lets the Vulkan or EGL/GLES do it. If the app is
> in
> > charge, the app manages the window swap chain buffers and
> synchronization,
> > otherwise the 3D driver does it. You shouldn't allow more than one thing
> > managing the Wayland window at the same time. Perhaps you could use
> wording
> > similar to Vulkan WSI or EGL window surface when describing what is and
> > what isn't allowed when it comes to Wayland windows.
>
> Yes, we probably should have some wording that if a client is letting
> something like EGL to commit the buffers, it must not attempt to use
> the fence extension on that wl_surface itself because EGL will probably
> be using the extension already.
>
> Alf?
>
>
> Thanks,
> pq
>
> > On Mon, 26 Nov 2018 at 09:22, Pekka Paalanen <ppaalanen@gmail.com>
> wrote:
> >
> > > On Fri, 23 Nov 2018 16:26:19 +0000
> > > Tomek Bury <tomek.bury@gmail.com> wrote:
> > >
> > > > Hi Pekka,
> > > >
> > > > > I presume you have a driver stack that relies on the opaque EGL
> > > buffers
> > > > and not zwp_linux_dmabuf any time soon?
> > > > Yes, exactly. I've added a protocol extension for sharing those
> buffers
> > > and
> > > > our eglCreateImage() implementation can import such buffers into
> the
> > > driver
> > > > on the compositor end. The buffers are carried by an fd to the
> compositor
> > > > that's the only similarity. They're not dma-buf.
> > > >
> > > > > Yeah, support for opaque EGL buffers could be added, just need to
> > > think
> > > > of a good wording, since acquire fences do not make sense for all
> buffer
> > > > types.
> > >
> > > > Isn't that renderer's and/or backend's decision? The GL renderer
> can
> > > accept
> > > > fence with any buffer it can send to the 3D driver, so, effectively,
> > > > anything backed by available EGL image extensions. Someone may add
> a
> > > custom
> > > > backend and/or renderer using whatever hardware or API they have at
> > > hand. A
> > > > Vulkan renderer could potentially use fences with anything a Vulkan
> > > driver
> > > > is capable of importing. A renderer that does the CPU wait could be
> > > useful
> > > > at least for debugging. So I wouln't block the explicit sync at the
> > > > compositor level based on the white list.
> > >
> > > Hi Tomek,
> > >
> > > fences do not make sense to all buffer types to begin with, today. My
> > > objection is to allowing fencing buffer types that cannot be sent to
> > > the 3D driver, e.g. wl_shm which is usually copied through glTexImage2D
> > > and friends. We cannot ignore those in the spec language.
> > >
> > > A renderer (a compositor really, we're not talking about just Weston)
> > > decides what buffer types it accepts, yes. This is communicated to
> > > clients through which buffer factory interface globals are being
> > > advertised. Each type is a different protocol extension. The fence
> > > extension OTOH is just a single extension, and currently there is no
> > > protocol to negotiate which buffer types are usable with acquire
> > > fences. The first attempt is to define in the spec language what will
> > > always be supported, provided the buffer factory exists.
> > >
> > > The opaque EGL buffer type is really just one type in practise:
> > > compositors and clients use it through a well-known, single API: EGL.
> > > It does not matter that there are multiple incompatible EGL
> > > implementations, it all looks like just one opaque buffer type to
> > > compositors. I think this makes it easier to extend the fence spec
> > > wording to require opaque EGL buffers to be supported.
> > >
> > > Either the fence protocol spec needs to be clear on what works, or we
> > > need advertisement events to let clients know in advance what the
> > > compositor supports. A client sending a fence that the compositor
> > > cannot use must not be possible; compositor, client, EGL, driver, etc.
> > > bugs notwithstanding.
> > >
> > > Btw. I just realized that if client-side EGL uses the fence extension
> > > internally, that means the client app code must not attempt to add or
> > > request fences of its own, because the spec disallows multiple acquire
> > > fences and multiple release notification requests. I suppose that's
> > > fine?
> > >
> > > Alf, can you come up with changes to the spec wording and Weston to
> > > require opaque EGL buffers are supported?
> > >
> > > On one hand it is actually a little strange to couple opaque EGL
> > > buffers (a private, EGL implementation specific protocol interface)
> > > with a generic fencing extension, but maybe that is necessary because
> > > there is not enough compositor-side GBM and EGL API so that the EGL
> > > implementation could handle it all in an EGL implementation specific
> > > interface?
> > >
> > >
> > > Thanks,
> > > pq
> > >
> > > > On Fri, 23 Nov 2018 at 13:47, Pekka Paalanen <ppaalanen@gmail.com>
> > > wrote:
> > > >
> > > > > On Fri, 23 Nov 2018 13:07:37 +0000
> > > > > Tomek Bury <tomek.bury@gmail.com> wrote:
> > > > >
> > > > > > Hi Alexandros,
> > > > > >
> > > > > > Sorry for a delay. I've finally got an end-to-end system to test
> it
> > > out.
> > > > > It
> > > > > > took some time because Weston backend I wrote a while back
> needed
> > > serious
> > > > > > rework to catch up with latest changes.
> > > > > >
> > > > > > There's one thing that didn't work for me. In compositor you
> reject
> > > > > > anything that isn't a DMA buffer and then in glrenderer you put
> an
> > > extra
> > > > > > assertion. Why? All you do is use an EGL extension in order to
> import
> > > > > > external fence_fd. There's no dmabuf dependency there. As long
> as
> > > the EGL
> > > > > > implementation exposes EGL_SYNC_NATIVE_FENCE_ANDROID extension
> this
> > > > > should
> > > > > > "just work" (tm) for the GL renderer. It certainly did for me.
> > > CPU-based
> > > > > > renderers can poll() to wait.
> > > > >
> > > > > Hi Tomek,
> > > > >
> > > > > with Weston it was decided not to implement a poll() based wait at
> > > > > first as implementing that properly (not blocking the compositor)
> would
> > > > > be a big hassle and no-one could see the benefit of it given what
> > > > > clients could actually produce.
> > > > >
> > > > > Therefore the acquire fence can only apply to buffers which can be
> > > > > pipelined to a GPU. Mesa EGL is using zwp_linux_dmabuf, but the
> support
> > > > > could be extended to opaque EGL buffers very well. We just chose to
> > > > > start small and bring up the infrastructure around fences first.
> > > > >
> > > > > Restrictions on buffer types etc. can certainly be lifted in the
> future
> > > > > if there are good use cases. I presume you have a driver stack that
> > > > > relies on the opaque EGL buffers and not zwp_linux_dmabuf any
> time
> > > soon?
> > > > >
> > > > > Would anyone ever use an acquire fence with wl_shm buffers? That
> sounds
> > > > > fundamentally wrong to me as one cannot create fences to be
> signalled
> > > > > by userspace AFAIK. Therefore buffers whose wait cannot be
> pipelined to
> > > > > the GPU or the display device do not make much sense to me.
> > > > >
> > > > > > The type of buffer used is an orthogonal problem. The
> > > > > > EGL_WL_bind_wayland_display
> > > > > > extension takes care of GL clients' buffers in GL renderer, for
> > > anything
> > > > > > else the renderer needs to know how to get pixels and use
> whatever
> > > means
> > > > > to
> > > > > > put those pixels on screen.
> > > > >
> > > > > Yeah, support for opaque EGL buffers could be added, just need to
> think
> > > > > of a good wording, since acquire fences do not make sense for all
> > > > > buffer types. A compositor must be allowed to raise protocol
> errors for
> > > > > fence+buffer combinations it cannot use, which means that clients
> must
> > > > > know in advance what they cannot use.
> > > > >
> > > > >
> > > > > Thanks,
> > > > > pq
> > > > >
> > > > > > On Tue, 13 Nov 2018 at 09:33, Tomek Bury <tomek.bury@gmail.com>
>
> > > wrote:
> > > > > >
> > > > > > > Thanks!
> > > > > > >
> > > > > > > On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
> > > > > > > alexandros.frantzis@collabora.com> wrote:
> > > > > > >
> > > > > > >> On Mon, Nov 12, 2018 at 12:39:58PM +0000, Tomek Bury wrote:
> > > > > > >> > On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <
> > > daniel@fooishbar.org>
> > > > >
> > > > > > >> wrote:
> > > > > > >> >
> > > > > > >> > > On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <
> > > ppaalanen@gmail.com>
> > > > >
> > > > > > >> wrote:
> > > > > > >> > > > I can add that while pushing upstream, if there are no
> > > other
> > > > > changes
> > > > > > >> > > > coming.
> > > > > > >> > > >
> > > > > > >> > > > Reviewed-by: Pekka Paalanen <
> pekka.paalanen@collabora.co.uk
> > > >
> > > > > > >> > > >
> > > > > > >> > > > You have ensured that the C files generated from this
> > > revision
> > > > > build
> > > > > > >> > > > fine in Weston, right?
> > > > > > >> > > >
> > > > > > >> > > > David, Daniel, since your name is in the maintainers,
> can
> > > I
> > > > > have
> > > > > > >> your
> > > > > > >> > > > R-b, please?
> > > > > > >> > >
> > > > > > >> > > The protocol is:
> > > > > > >> > > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > > > > > >> > >
> > > > > > >> > > The Weston implementation looks pretty good so far,
> though
> > > > > there's no
> > > > > > >> > > full implementation of release yet.
> > > > > > >> > >
> > > > > > >> > > Cheers,
> > > > > > >> > > Daniel
> > > > > > >> > > _______________________________________________
> > > > > > >> > > wayland-devel mailing list
> > > > > > >> > > wayland-devel@lists.freedesktop.org
> > > > > > >> > >
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > > > > > >> > >
> > > > > > >> >
> > > > > > >> > HI Daniel,
> > > > > > >> >
> > > > > > >> > Where can I find the work-in-progress implementation? I'd
> like
> > > to
> > > > > try it
> > > > > > >> > out with Broadcom driver which doesn't have implicit
> > > cross-process
> > > > > > >> sync. I
> > > > > > >> > can add the explicit sync protocol implementation on the
> > > driver
> > > > > side but
> > > > > > >> > I'd need a reference to test it against.
> > > > > > >> >
> > > > > > >> > Cheers,
> > > > > > >> > Tomek
> > > > > > >>
> > > > > > >> Hi Tomek,
> > > > > > >>
> > > > > > >> the WIP implementation can be found here [1]. I hope to push
> an
> > > > > update,
> > > > > > >> including some zwp_buffer_release_v1 correctness fixes, in
> the
> > > > > following
> > > > > > >> days.
> > > > > > >>
> > > > > > >> Thanks,
> > > > > > >> Alexandros
> > > > > > >>
> > > > > > >> [1]
> > > https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
> > > > > > >>
> > > > > > >
> > > > >
> > > > >
> > >
> > >
>
>
Pekka Paalanen Nov. 27, 2018, 1:09 p.m.
On Tue, 27 Nov 2018 10:56:39 +0000
Tomek Bury <tomek.bury@gmail.com> wrote:

> Hi Pekka,
> 
> > I suppose that applies to the opaque EGL buffers only?  
> Sort of. As far as I understand it, the divide is between wl_surface
> managed by EGL/WSI vs. wl_surface managed directly by the client
> application. For EGL case the wl_surface managed by EGL would be set-up
> more or less like this:
> struct wl_egl_window *window = wl_egl_window_create(surface, w, h);
> EGLDisplay dpy = eglGetPlatformDisplay(EGL_PLATFORM_WAYLAND_EXT, ...);
> EGLSurface surface = eglCreatePlatformWindowSurface(dpy, config, window);
> With this setup EGL becomes responsible for managing the swapchain buffers,
> keeping up with window resizes, sending wl_buffers to compositor when
> client app calls eglSwapBuffers() etc. This is how weston-simple-egl works.

Hi Tomek,

that is how most apps work, but nothing is forbidding a client from
switching.

If a client wants to, it could start with wl_shm and CPU-rendered
content, then switch to EGL managed surface, then shut down EGL and
switch back to wl_shm all on the very same wl_surface, for example.

I cannot point to any example of such client, but it is not forbidden
anywhere. If apps start supporting GPU hotplug, then I imagine such
switching would become useful.

> > Otherwise, a client could use e.g. EGL in a different way to get dmabuf,
> > send those manually to the compositor and set up fences manually as
> > well. weston-simple-dmabuf-egl in the MR is one variant of that. Of
> > course, it depends on the EGL stack supporting dmabuf to begin with, so
> > if yours doesn't then it's not possible.  
> Yes, exactly. This is the example where the application manages the Wayland
> window directly and the EGL driver is not involved. EGL is only used to set
> up rendering into an off-screen buffer(s) of some description.
> 
> In both cases the compositor receives a wl_buffer and has to figure out how
> to render the contents of such buffer. In both cases the wl_buffer could
> encapsulate dma-buf and the compositor wouldn't be able to tell whether it
> came directly from application (weston-simple-dmabuf-egl) or from the EGL
> driver (weston-simple-egl). in fact the Waylad client could choose dma-buf,
> prime, flink, shm, something else, regardless of how the wl_surface is
> managed, whether it's done directly by the application, by the EGL or
> Vulkan driver. And some of those wl_buffer types will be known to the
> compositor, some to the 3D driver and perhaps some to both. At least that's
> my understanding.

Sure. I'm not sure how that is relevant though, or what we are debating
about.

A compositor cares about how it will access a buffer: is the access
off-loaded somewhere which can or cannot import acquire fences, or does it
need to read the buffer with CPU directly. I would like to avoid
requiring fence support from compositors when they do not have a way to
off-load the fence wait.

Is it possible to have some variant of glTexImage2D wait for an EGLSync
before it actually reads from the source? If there is, then I'll
reconsider about the buffer type requirements.

It is a happy coincidence, that in the cases where a compositor does
not have a way to off-load a fence wait, it would also be very hard for
a client to arrange a fence to be waited on in the first place. But,
the protocol specification cannot rely on such practicalities, instead
it needs to set clear expectations on what should work.

Saying, that if a compositor supports the fence extension and opaque EGL
buffers then it needs to support opaque EGL buffers with acquire
fences, seems like a good idea. Neither condition needs to be written
out explicitly: it is the fence extension spec and a compositor either
supports all of it or none of it; whether it is possible to have opaque
EGL buffers depends on the compositor initializing the support which is
well discoverable to clients. It would be enough for the fence spec to
define what "opaque EGL buffer" is and then say that they also work in
addition to zwp_linux_dmabuf buffers.


Thanks,
pq
Tomek Bury Nov. 27, 2018, 2:57 p.m.
Hi Pekka,

I'm just trying to figure out how to use this extension in a driver.
Without this extension the only option I have is to take a copy if each
wl_buffer on the compositor-side. I can't really do implicit sync so I'm
really happy with anything that allows for explicit sync.

> Is it possible to have some variant of glTexImage2D wait for an EGLSync
> before it actually reads from the source?
I don't think so. The glTexImage2D mustn't hold onto any client pointers.
GLES3 buffers require mmap and CPU copy. Pixmaps are unsupported so EGL
images are the only remaining thing I can think of.

>  It would be enough for the fence spec to
> define what "opaque EGL buffer" is and then say that they also work in
> addition to zwp_linux_dmabuf buffers.
Yup, that works for me.

> If a client wants to, it could start with wl_shm and CPU-rendered
> content, then switch to EGL managed surface, then shut down EGL and
> switch back to wl_shm all on the very same wl_surface, for example.
Hmmm, perhaps the set_acquire_fence() should be a buffer operation, not a
surface operation then? What you wrote means to me that the compositor
knows of every buffer the client created but doesn't know which buffer the
client is going to attach next. The earliest moment the compositor learns
about the buffer for the next frame is when the attach request reaches the
compositor. And that would be most likely after the attach/damage/commit
gets flushed at the end of frame by the client. But that's a battle for
another day.

Cheers,
Tomek





On Tue, 27 Nov 2018 at 13:10, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Tue, 27 Nov 2018 10:56:39 +0000
> Tomek Bury <tomek.bury@gmail.com> wrote:
>
> > Hi Pekka,
> >
> > > I suppose that applies to the opaque EGL buffers only?
> > Sort of. As far as I understand it, the divide is between wl_surface
> > managed by EGL/WSI vs. wl_surface managed directly by the client
> > application. For EGL case the wl_surface managed by EGL would be set-up
> > more or less like this:
> > struct wl_egl_window *window = wl_egl_window_create(surface, w, h);
> > EGLDisplay dpy = eglGetPlatformDisplay(EGL_PLATFORM_WAYLAND_EXT, ...);
> > EGLSurface surface = eglCreatePlatformWindowSurface(dpy, config, window);
> > With this setup EGL becomes responsible for managing the swapchain
> buffers,
> > keeping up with window resizes, sending wl_buffers to compositor when
> > client app calls eglSwapBuffers() etc. This is how weston-simple-egl
> works.
>
> Hi Tomek,
>
> that is how most apps work, but nothing is forbidding a client from
> switching.
>
> If a client wants to, it could start with wl_shm and CPU-rendered
> content, then switch to EGL managed surface, then shut down EGL and
> switch back to wl_shm all on the very same wl_surface, for example.
>
> I cannot point to any example of such client, but it is not forbidden
> anywhere. If apps start supporting GPU hotplug, then I imagine such
> switching would become useful.
>
> > > Otherwise, a client could use e.g. EGL in a different way to get
> dmabuf,
> > > send those manually to the compositor and set up fences manually as
> > > well. weston-simple-dmabuf-egl in the MR is one variant of that. Of
> > > course, it depends on the EGL stack supporting dmabuf to begin with, so
> > > if yours doesn't then it's not possible.
> > Yes, exactly. This is the example where the application manages the
> Wayland
> > window directly and the EGL driver is not involved. EGL is only used to
> set
> > up rendering into an off-screen buffer(s) of some description.
> >
> > In both cases the compositor receives a wl_buffer and has to figure out
> how
> > to render the contents of such buffer. In both cases the wl_buffer could
> > encapsulate dma-buf and the compositor wouldn't be able to tell whether
> it
> > came directly from application (weston-simple-dmabuf-egl) or from the EGL
> > driver (weston-simple-egl). in fact the Waylad client could choose
> dma-buf,
> > prime, flink, shm, something else, regardless of how the wl_surface is
> > managed, whether it's done directly by the application, by the EGL or
> > Vulkan driver. And some of those wl_buffer types will be known to the
> > compositor, some to the 3D driver and perhaps some to both. At least
> that's
> > my understanding.
>
> Sure. I'm not sure how that is relevant though, or what we are debating
> about.
>
> A compositor cares about how it will access a buffer: is the access
> off-loaded somewhere which can or cannot import acquire fences, or does it
> need to read the buffer with CPU directly. I would like to avoid
> requiring fence support from compositors when they do not have a way to
> off-load the fence wait.
>
> Is it possible to have some variant of glTexImage2D wait for an EGLSync
> before it actually reads from the source? If there is, then I'll
> reconsider about the buffer type requirements.
>
> It is a happy coincidence, that in the cases where a compositor does
> not have a way to off-load a fence wait, it would also be very hard for
> a client to arrange a fence to be waited on in the first place. But,
> the protocol specification cannot rely on such practicalities, instead
> it needs to set clear expectations on what should work.
>
> Saying, that if a compositor supports the fence extension and opaque EGL
> buffers then it needs to support opaque EGL buffers with acquire
> fences, seems like a good idea. Neither condition needs to be written
> out explicitly: it is the fence extension spec and a compositor either
> supports all of it or none of it; whether it is possible to have opaque
> EGL buffers depends on the compositor initializing the support which is
> well discoverable to clients. It would be enough for the fence spec to
> define what "opaque EGL buffer" is and then say that they also work in
> addition to zwp_linux_dmabuf buffers.
>
>
> Thanks,
> pq
>
Alexandros Frantzis Nov. 27, 2018, 3:17 p.m.
On Tue, Nov 27, 2018 at 10:53:45AM +0200, Pekka Paalanen wrote:
> Yes, we probably should have some wording that if a client is letting
> something like EGL to commit the buffers, it must not attempt to use
> the fence extension on that wl_surface itself because EGL will probably
> be using the extension already.
> 
> Alf?

Hi Pekka and Tomek,

I will send a patch with a proposal for the discussed wording updates to
the list soon (probably tomorrow).

I agree it's fine for the spec to be relaxed for the opaque EGL buffers.
As Pekka mentioned, we explicitly limited the spec to support only
zwp_linux_dmabuf to avoid the problem of having to deal with unsupported
buffer/fence combinations, and opaque EGL buffers are not likely to pose
problems in this regard.

In practice we can probably support all buffers that can be imported
into the graphics API without requiring a client-side wait in the
compositor ("client" from a graphics API perspective) for proper use,
but that's harder to specify. There were discussions about allowing
everything other than wl_shm, but we decided against it, since that
would make the protocol too fragile.

Relaxing the spec further will probably require more radical changes,
though, e.g., advertising supported buffer types, or similar.

I will also add some notes/warnings about using this protocol with
graphics APIs that handle buffers internally.

This is also a good chance to propose some other clarifications to the
spec I have been thinking about.

Thanks,
Alexandros
Alexandros Frantzis Nov. 27, 2018, 4:25 p.m.
On Tue, Nov 27, 2018 at 05:17:40PM +0200, Alexandros Frantzis wrote:
> On Tue, Nov 27, 2018 at 10:53:45AM +0200, Pekka Paalanen wrote:
> > Yes, we probably should have some wording that if a client is letting
> > something like EGL to commit the buffers, it must not attempt to use
> > the fence extension on that wl_surface itself because EGL will probably
> > be using the extension already.
> > 
> > Alf?
> 
> Hi Pekka and Tomek,
> 
> I will send a patch with a proposal for the discussed wording updates to
> the list soon (probably tomorrow).
> 
> I agree it's fine for the spec to be relaxed for the opaque EGL buffers.
> As Pekka mentioned, we explicitly limited the spec to support only
> zwp_linux_dmabuf to avoid the problem of having to deal with unsupported
> buffer/fence combinations, and opaque EGL buffers are not likely to pose
> problems in this regard.

Note that this will require a v2 of this protocol since we will be
requiring more from implementations compared to v1 (unless we can cheat
and not bump?). The current protocol says:

"Explicit synchronization is guaranteed to be supported only for buffers                       
 created with any version of the wp_linux_dmabuf buffer factory."

which upon rereading isn't clear enough about if implementations are
required to support *only* linux_dmabuf, or if implementations need
to support *at least* linux_dmabuf.

If we don't want to commit to general opaque EGL buffer support,
perhaps an option here would be to change this to the more clear:

"Explicit synchronization is only guaranteed to be supported for buffers                       
 created with any version of the wp_linux_dmabuf buffer factory, but
 implementations are free to also support it for other buffer types."

This allows us to stay at v1 without explicitly naming out opaque EGL
buffers, while still allowing Weston to support opaque EGL buffers.

I guess it depends if we think the explicit sync + opaque EGL buffer
case will be interesting enough to be used outside of environments with
a controlled compositor and clients.

Thanks,
Alexandros
Pekka Paalanen Nov. 28, 2018, 1:32 p.m.
On Tue, 27 Nov 2018 14:57:07 +0000
Tomek Bury <tomek.bury@gmail.com> wrote:

> Hi Pekka,
> 
> I'm just trying to figure out how to use this extension in a driver.
> Without this extension the only option I have is to take a copy if each
> wl_buffer on the compositor-side. I can't really do implicit sync so I'm
> really happy with anything that allows for explicit sync.

Ah, ok. So for both acquire and release fences:

In your client-side implementation of EGL or Vulkan, in the case where
you, the implementation, will be calling wl_surface.attach and
wl_surface.commit, you will be the exclusive user of the explicit sync
extension on that wl_surface. Go wild. :-)

If the sync extension is not available from the compositor, then you
have to do without on the client-side.

In the server-side implementation, you don't need to do anything. The
compositor will implement the explicit sync protocol and translate it
into EGL etc. calls.

I suppose in the server-side implementation you *cannot* do anything
unless you can infer whether the compositor is actually supporting the
explicit sync extension or not.

> > Is it possible to have some variant of glTexImage2D wait for an EGLSync
> > before it actually reads from the source?  
> I don't think so. The glTexImage2D mustn't hold onto any client pointers.
> GLES3 buffers require mmap and CPU copy. Pixmaps are unsupported so EGL
> images are the only remaining thing I can think of.
> 
> >  It would be enough for the fence spec to
> > define what "opaque EGL buffer" is and then say that they also work in
> > addition to zwp_linux_dmabuf buffers.  
> Yup, that works for me.
> 
> > If a client wants to, it could start with wl_shm and CPU-rendered
> > content, then switch to EGL managed surface, then shut down EGL and
> > switch back to wl_shm all on the very same wl_surface, for example.  
> Hmmm, perhaps the set_acquire_fence() should be a buffer operation, not a
> surface operation then? What you wrote means to me that the compositor
> knows of every buffer the client created but doesn't know which buffer the
> client is going to attach next. The earliest moment the compositor learns
> about the buffer for the next frame is when the attach request reaches the
> compositor. And that would be most likely after the attach/damage/commit
> gets flushed at the end of frame by the client. But that's a battle for
> another day.

Correct, the compositor cannot know which buffer a client might attach
to which surface at any time. There is absolutely no connection between
which buffer goes with which surface, there has never been, and from
protocol perspective clients could use the same buffer on multiple
surfaces too.

For technical reasons, we cannot add any wl_buffer requests or events.
It would have to be done with another interface that takes the
wl_buffer as an argument. I guess that would be possible, but I'm not
sure what the difference would be.

Your EGL implementation cannot expose the same interface as the
compositor does for dmabufs. So you would have buffer-type specific
interfaces for adding acquire fences. Maybe that's not actually a bad
idea, given that the acquire fences are so closely related to buffer
types. It would also allow to split the release event and fence into
their own extension, to be shared across all buffer types including
wl_shm. OTOH, it's more typing to implement.

If we did the buffer-type specific explicit sync interfaces design,
then a compositor itself would implement set_acquire_fence for dmabufs,
and a compositor-side EGL implementation could implement
set_acquire_fence for opaque EGL buffers which would be proprietary to
the EGL implementation, but that is ok because the client-side of the
same is too.

In fact, you could have implemented an explicit sync extension for
acquire fences inside your EGL implementation since day one. It is just
the release fence side which probably wouldn't work out too well as
hidden inside EGL.

I suppose the compositor-side copy of buffers you mentioned is for the
lack of release fences, not acquire fences?


Thanks,
pq
Pekka Paalanen Nov. 28, 2018, 2:09 p.m.
On Tue, 27 Nov 2018 18:25:04 +0200
Alexandros Frantzis <alexandros.frantzis@collabora.com> wrote:

> On Tue, Nov 27, 2018 at 05:17:40PM +0200, Alexandros Frantzis wrote:
> > On Tue, Nov 27, 2018 at 10:53:45AM +0200, Pekka Paalanen wrote:  
> > > Yes, we probably should have some wording that if a client is letting
> > > something like EGL to commit the buffers, it must not attempt to use
> > > the fence extension on that wl_surface itself because EGL will probably
> > > be using the extension already.
> > > 
> > > Alf?  
> > 
> > Hi Pekka and Tomek,
> > 
> > I will send a patch with a proposal for the discussed wording updates to
> > the list soon (probably tomorrow).
> > 
> > I agree it's fine for the spec to be relaxed for the opaque EGL buffers.
> > As Pekka mentioned, we explicitly limited the spec to support only
> > zwp_linux_dmabuf to avoid the problem of having to deal with unsupported
> > buffer/fence combinations, and opaque EGL buffers are not likely to pose
> > problems in this regard.  
> 
> Note that this will require a v2 of this protocol since we will be
> requiring more from implementations compared to v1 (unless we can cheat
> and not bump?). The current protocol says:

It's not a backwards incompatible change, so I think there is no need
for a major bump. You can do a minor bump if you want, it would be
strictly correct.

> 
> "Explicit synchronization is guaranteed to be supported only for buffers                       
>  created with any version of the wp_linux_dmabuf buffer factory."
> 
> which upon rereading isn't clear enough about if implementations are
> required to support *only* linux_dmabuf, or if implementations need
> to support *at least* linux_dmabuf.

I think the "guaranteed" makes it "at least".

> 
> If we don't want to commit to general opaque EGL buffer support,
> perhaps an option here would be to change this to the more clear:
> 
> "Explicit synchronization is only guaranteed to be supported for buffers                       
>  created with any version of the wp_linux_dmabuf buffer factory, but
>  implementations are free to also support it for other buffer types."
> 
> This allows us to stay at v1 without explicitly naming out opaque EGL
> buffers, while still allowing Weston to support opaque EGL buffers.

It would not help clients to know what they can use though. It would
leave them in the dark even after the protocol was stabilized.

The fence support for opaque EGL buffers depends on the compositor
implementation, not on EGL implementation. So it varies across
compositors even on the same platform or system.

> 
> I guess it depends if we think the explicit sync + opaque EGL buffer
> case will be interesting enough to be used outside of environments with
> a controlled compositor and clients.

I believe it is.


Thanks,
pq
Tomek Bury Nov. 28, 2018, 2:35 p.m.
Hi Pekka,

> I suppose the compositor-side copy of buffers you mentioned is for the
> lack of release fences, not acquire fences?
Yes, the lack of release fences is the very reason for the copy. I could
cook something up for the acquire fence, but that wouldn't synchronise the
buffer access anyway. The only way I can be sure the client doesn't
overwrite a buffer still in use by the GPU was to texture from a copy and
let the compositor release the original without waiting for the GPU and
without a fence.

Cheers,
Tomek