Discussion:
[PATCH wayland-protocols v2] Add zwp_linux_explicit_synchronization_v1
(too old to reply)
Alexandros Frantzis
2018-10-09 15:11:22 UTC
Permalink
Signed-off-by: Alexandros Frantzis <***@collabora.com>
---

patch v1 here: https://patchwork.freedesktop.org/patch/177866/
Changes since patch v1:
- Add NO_SURFACE error for zwp_surface_synchronization_v1 requests.
- Remove restriction to destroy a zwp_surface_synchronization_v1 object
after the associated wl_surface is destroyed.
- Clarify which buffer the acquire fence is associated with.
- Clarify 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 | 222 ++++++++++++++++++
3 files changed, 229 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 <***@chromium.org>
+Daniel Stone <***@collabora.com>
+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..11ef3f0
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
@@ -0,0 +1,222 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="zwp_linux_explicit_synchronization_unstable_v1">
+
+ <copyright>
+ Copyright 2016 The Chromium Authors.
+ Copyright 2017 Intel Corporation
+ Copyright 2018 Collabora, Ltd
+
+ Permission is hereby granted, free of charge, to any person obtaining a
+ copy of this software and associated documentation files (the "Software"),
+ to deal in the Software without restriction, including without limitation
+ the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ and/or sell copies of the Software, and to permit persons to whom the
+ Software is furnished to do so, subject to the following conditions:
+
+ The above copyright notice and this permission notice (including the next
+ paragraph) shall be included in all copies or substantial portions of the
+ Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ DEALINGS IN THE SOFTWARE.
+ </copyright>
+
+ <interface name="zwp_linux_explicit_synchronization_v1" version="1">
+ <description summary="protocol for providing explicit synchronization">
+ This global is a factory interface, allowing clients to request
+ explicit synchronization for buffers on a per-surface basis.
+
+ See zwp_surface_synchronization_v1 for more information.
+
+ This interface is derived from Chromium's
+ zcr_linux_explicit_synchronization_v1.
+
+ Warning! The protocol described in this file is experimental and
+ backward incompatible changes may be made. Backward compatible changes
+ may be added together with the corresponding interface version bump.
+ Backward incompatible changes are done by bumping the version number in
+ the protocol and interface names and resetting the interface version.
+ Once the protocol is to be declared stable, the 'z' prefix and the
+ version number in the protocol and interface names are removed and the
+ interface version number is reset.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy explicit synchronization factory object">
+ Destroy this explicit synchronization factory object. Other objects,
+ including zwp_surface_synchronization_v1 objects created by this
+ factory, shall not be affected by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="synchronization_exists" value="0"
+ summary="the surface already has a synchronization object associated"/>
+ </enum>
+
+ <request name="get_synchronization">
+ <description summary="extend surface interface for explicit synchronization">
+ Instantiate an interface extension for the given wl_surface to
+ provide explicit synchronization.
+
+ If the given wl_surface already has an explicit synchronization object
+ associated, the synchronization_exists protocol error is raised.
+ </description>
+
+ <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
+ summary="the new synchronization interface id"/>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_surface_synchronization_v1" version="1">
+ <description summary="per-surface explicit synchronization support">
+ This object implements per-surface explicit synchronization.
+
+ Explicit 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.
+ 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. This is referred
+ to as 'implicit synchronization'.
+
+ By contrast, explicit synchronization takes dma_fence objects as a
+ marker of when the asynchronous operations are complete. The fence
+ provided by the client will be waited on before the compositor
+ accesses the buffer. Conversely, in place of wl_buffer.release
+ events, the Wayland server will inform the client when the buffer
+ can be destructively accessed through a zwp_buffer_release_v1
+ object.
+
+ This interface cannot be instantiated multiple times for a single
+ surface, and as such should only be used by the component which
+ performs the wl_surface.attach request. Whenever control of
+ buffer attachments is released, the releasing component should
+ ensure it destroys the zwp_surface_synchronization_v1 object.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy synchronization object">
+ Destroy this explicit synchronization object.
+
+ Any fence set with set_acquire_fence in this commit cycle will
+ be invalidated in the server.
+
+ zwp_buffer_release_v1 objects created by this object are not affected
+ by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="invalid_fence" value="0"
+ summary="the fence specified by the client could not be imported"/>
+ <entry name="duplicate_fence" value="1"
+ summary="multiple fences added for a single surface commit"/>
+ <entry name="duplicate_release" value="2"
+ summary="multiple releases added for a single surface commit"/>
+ <entry name="no_surface" value="3"
+ summary="the associated wl_surface was destroyed"/>
+ </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_buffer_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 fence could not be imported, an INVALID_FENCE error is signaled
+ to the client.
+
+ If a fence has already been attached during the same commit cycle,
+ a DUPLICATE_FENCE error is signaled to the client.
+
+ If the associated wl_surface was destroyed, a NO_SURFACE error
+ is signaled to the client.
+ </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_buffer.attach. See zwp_buffer_release_v1
+ documentation for more information.
+
+ The release object is double-buffered state, and will be applied
+ on the next wl_surface.commit request for the associated surface.
+
+ If a zwp_buffer_release_v1 object has already been requested for
+ the surface in the same commit cycle, a DUPLICATE_RELEASE error
+ is signaled to the client.
+
+ If the associated wl_surface was destroyed, a NO_SURFACE error
+ is signaled to the client.
+ </description>
+
+ <arg name="release" type="new_id" interface="zwp_buffer_release_v1"
+ summary="new zwp_buffer_release_v1 object"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1 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
+ where relevant for the client to synchronize against before reusing
+ the buffer.
+
+ Exactly one event, either a fenced_release or an immediate_release,
+ will be emitted for each wl_surface.commit request.
+
+ This event does not replace wl_buffer.release events; servers are still
+ required to send those events.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy buffer release synchronization object">
+ Destroy this buffer release explicit synchronization object. The object
+ may be destroyed at any time.
+ </description>
+ </request>
+
+ <event name="fenced_release">
+ <description summary="release buffer with fence">
+ Sent when the compositor has finalised its usage of the associated
+ buffer, providing a dma_fence which will be signaled when all
+ operations by the compositor on that buffer have finished.
+
+ Clients must not perform any destructive operations (e.g. clearing or
+ rendering) on the buffer until that fence has passed. They may,
+ however, destroy the wl_buffer 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, and either performed no operations using it, or has a
+ guarantee that all its operations have finished and no more external
+ effects will be observed from these operations.
+ </description>
+ </event>
+ </interface>
+
+</protocol>
--
2.19.1
Pekka Paalanen
2018-10-12 12:24:30 UTC
Permalink
Hi,

this is a good specification, all my comments are clarifications or
minor adjustments. The fundamental design looks fine to me.


On Tue, 9 Oct 2018 18:11:22 +0300
Post by Alexandros Frantzis
---
patch v1 here: https://patchwork.freedesktop.org/patch/177866/
- Add NO_SURFACE error for zwp_surface_synchronization_v1 requests.
- Remove restriction to destroy a zwp_surface_synchronization_v1 object
after the associated wl_surface is destroyed.
- Clarify which buffer the acquire fence is associated with.
- Clarify 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 | 222 ++++++++++++++++++
3 files changed, 229 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
+
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..11ef3f0
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
@@ -0,0 +1,222 @@
+<?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
+
+ The above copyright notice and this permission notice (including the next
+ paragraph) shall be included in all copies or substantial portions of the
+ Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ DEALINGS IN THE SOFTWARE.
+ </copyright>
+
+ <interface name="zwp_linux_explicit_synchronization_v1" version="1">
+ <description summary="protocol for providing explicit synchronization">
+ This global is a factory interface, allowing clients to request
+ explicit synchronization for buffers on a per-surface basis.
+
+ See zwp_surface_synchronization_v1 for more information.
+
+ This interface is derived from Chromium's
+ zcr_linux_explicit_synchronization_v1.
+
+ Warning! The protocol described in this file is experimental and
+ backward incompatible changes may be made. Backward compatible changes
+ may be added together with the corresponding interface version bump.
+ Backward incompatible changes are done by bumping the version number in
+ the protocol and interface names and resetting the interface version.
+ Once the protocol is to be declared stable, the 'z' prefix and the
+ version number in the protocol and interface names are removed and the
+ interface version number is reset.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy explicit synchronization factory object">
+ Destroy this explicit synchronization factory object. Other objects,
+ including zwp_surface_synchronization_v1 objects created by this
+ factory, shall not be affected by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="synchronization_exists" value="0"
+ summary="the surface already has a synchronization object associated"/>
+ </enum>
+
+ <request name="get_synchronization">
+ <description summary="extend surface interface for explicit synchronization">
+ Instantiate an interface extension for the given wl_surface to
+ provide explicit synchronization.
+
+ If the given wl_surface already has an explicit synchronization object
+ associated, the synchronization_exists protocol error is raised.
+ </description>
+
+ <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
+ summary="the new synchronization interface id"/>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_surface_synchronization_v1" version="1">
+ <description summary="per-surface explicit synchronization support">
+ This object implements per-surface explicit synchronization.
+
+ Explicit synchronization refers to co-ordination of pipelined
Strike "Explicit"? It's odd because this starts with "explicit
synchronization refers to..." and then ends saying this is "implicit
synchronization.
Post by Alexandros Frantzis
+ 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.
Should there be a paragraph break here?
Post by Alexandros Frantzis
+ 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. This is referred
+ to as 'implicit synchronization'.
+
+ By contrast, explicit synchronization takes dma_fence objects as a
+ marker of when the asynchronous operations are complete. The fence
+ provided by the client will be waited on before the compositor
+ accesses the buffer. Conversely, in place of wl_buffer.release
+ events, the Wayland server will inform the client when the buffer
+ can be destructively accessed through a zwp_buffer_release_v1
+ object.
So this guarantees that no more wl_buffer.release events can be triggered
by commits that used explicit sync?
Post by Alexandros Frantzis
+
+ This interface cannot be instantiated multiple times for a single
+ surface, and as such should only be used by the component which
+ performs the wl_surface.attach request. Whenever control of
+ buffer attachments is released, the releasing component should
+ ensure it destroys the zwp_surface_synchronization_v1 object.
Is the last sentence necessary? It might confuse things as it can
easily be read as if zwp_surface_synchronization_v1 was a one-shot
thing.
Post by Alexandros Frantzis
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy synchronization object">
+ Destroy this explicit synchronization object.
+
+ Any fence set with set_acquire_fence in this commit cycle will
+ be invalidated in the server.
This means that if zwp_surface_synchronization_v1 object is
destroyed before issuing wl_surface.commit, then the pending acquire
fence is discarded by the server, right?
Post by Alexandros Frantzis
+
+ zwp_buffer_release_v1 objects created by this object are not affected
+ by this request.
And after wl_surface.commit, destruction has no effect on the commit.
Post by Alexandros Frantzis
+ </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"/>
+ </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_buffer_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 fence could not be imported, an INVALID_FENCE error is signaled
+ to the client.
"... error is raised." There is no need to be able to recover from a
failed fence import, disconnection is fine, right?
Post by Alexandros Frantzis
+
+ If a fence has already been attached during the same commit cycle,
+ a DUPLICATE_FENCE error is signaled to the client.
+
+ If the associated wl_surface was destroyed, a NO_SURFACE error
+ is signaled to the client.
+ </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_buffer.attach. See zwp_buffer_release_v1
+ documentation for more information.
+
+ The release object is double-buffered state, and will be applied
+ on the next wl_surface.commit request for the associated surface.
+
+ If a zwp_buffer_release_v1 object has already been requested for
+ the surface in the same commit cycle, a DUPLICATE_RELEASE error
+ is signaled to the client.
+
+ If the associated wl_surface was destroyed, a NO_SURFACE error
+ is signaled to the client.
+ </description>
+
+ <arg name="release" type="new_id" interface="zwp_buffer_release_v1"
+ summary="new zwp_buffer_release_v1 object"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1 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
+ where relevant for the client to synchronize against before reusing
+ the buffer.
+
+ Exactly one event, either a fenced_release or an immediate_release,
+ will be emitted for each wl_surface.commit request.
+
+ This event does not replace wl_buffer.release events; servers are still
+ required to send those events.
Ok. The introduction lead me to believe otherwise, that should probably
be worded differently.
Post by Alexandros Frantzis
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy buffer release synchronization object">
+ Destroy this buffer release explicit synchronization object. The object
+ may be destroyed at any time.
+ </description>
Is it inconceivable to ever add other requests in this interface?

Could there ever be a benefit from destroying this object before it has
sent an event?

Could this object ever be used as an argument to a request?

If not, this seems like the perfect candidate for destroy-on-event
behaviour, similar to wp_presentation_feedback. But if there is any
suspicion about extending this interface, then it's better to keep the
destroy request.
Post by Alexandros Frantzis
+ </request>
+
+ <event name="fenced_release">
+ <description summary="release buffer with fence">
+ Sent when the compositor has finalised its usage of the associated
+ buffer, providing a dma_fence which will be signaled when all
+ operations by the compositor on that buffer have finished.
+
+ Clients must not perform any destructive operations (e.g. clearing or
+ rendering) on the buffer until that fence has passed. They may,
+ however, destroy the wl_buffer object.
"...without any visible effects" I would add. Clients may destroy a
wl_buffer at any time, but at a "wrong" time it may lead to visual
glitches. This even here says that no glitches can happen. Unless, of
course, the wl_buffer has been submitted again or elsewhere already.

Or maybe just remove the note about destroying the wl_buffer? Strictly
speaking this event alone is not sufficient, there must not be other
unreleased uses of the wl_buffer.
Post by Alexandros Frantzis
+ </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, and either performed no operations using it, or has a
+ guarantee that all its operations have finished and no more external
+ effects will be observed from these operations.
Right. Should there be explicit wording that this applies to the
specific commit, and not the buffer's usage in general?

After all, this is the extension that makes it well-defined to use a
wl_buffer again (with its contents intact) before it has been released,
providing a solution to
https://gitlab.freedesktop.org/wayland/wayland/issues/46 .
Post by Alexandros Frantzis
+ </description>
+ </event>
+ </interface>
+
+</protocol>
Thanks,
pq
Alexandros Frantzis
2018-10-15 14:15:13 UTC
Permalink
Signed-off-by: Alexandros Frantzis <***@collabora.com>
---

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.

patch v1 here: https://patchwork.freedesktop.org/patch/177866/

Makefile.am | 1 +
.../linux-explicit-synchronization/README | 6 +
...x-explicit-synchronization-unstable-v1.xml | 219 ++++++++++++++++++
3 files changed, 226 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 <***@chromium.org>
+Daniel Stone <***@collabora.com>
+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..c2f44a9
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
@@ -0,0 +1,219 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="zwp_linux_explicit_synchronization_unstable_v1">
+
+ <copyright>
+ Copyright 2016 The Chromium Authors.
+ Copyright 2017 Intel Corporation
+ Copyright 2018 Collabora, Ltd
+
+ Permission is hereby granted, free of charge, to any person obtaining a
+ copy of this software and associated documentation files (the "Software"),
+ to deal in the Software without restriction, including without limitation
+ the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ and/or sell copies of the Software, and to permit persons to whom the
+ Software is furnished to do so, subject to the following conditions:
+
+ The above copyright notice and this permission notice (including the next
+ paragraph) shall be included in all copies or substantial portions of the
+ Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ DEALINGS IN THE SOFTWARE.
+ </copyright>
+
+ <interface name="zwp_linux_explicit_synchronization_v1" version="1">
+ <description summary="protocol for providing explicit synchronization">
+ This global is a factory interface, allowing clients to request
+ explicit synchronization for buffers on a per-surface basis.
+
+ See zwp_surface_synchronization_v1 for more information.
+
+ This interface is derived from Chromium's
+ zcr_linux_explicit_synchronization_v1.
+
+ Warning! The protocol described in this file is experimental and
+ backward incompatible changes may be made. Backward compatible changes
+ may be added together with the corresponding interface version bump.
+ Backward incompatible changes are done by bumping the version number in
+ the protocol and interface names and resetting the interface version.
+ Once the protocol is to be declared stable, the 'z' prefix and the
+ version number in the protocol and interface names are removed and the
+ interface version number is reset.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy explicit synchronization factory object">
+ Destroy this explicit synchronization factory object. Other objects,
+ including zwp_surface_synchronization_v1 objects created by this
+ factory, shall not be affected by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="synchronization_exists" value="0"
+ summary="the surface already has a synchronization object associated"/>
+ </enum>
+
+ <request name="get_synchronization">
+ <description summary="extend surface interface for explicit synchronization">
+ Instantiate an interface extension for the given wl_surface to provide
+ explicit synchronization.
+
+ If the given wl_surface already has an explicit synchronization object
+ associated, the synchronization_exists protocol error is raised.
+ </description>
+
+ <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
+ summary="the new synchronization interface id"/>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_surface_synchronization_v1" version="1">
+ <description summary="per-surface explicit synchronization support">
+ This object implements per-surface explicit synchronization.
+
+ Synchronization refers to co-ordination of pipelined operations performed
+ on buffers. Most GPU clients will schedule an asynchronous operation to
+ render to the buffer, then immediately send the buffer to the compositor
+ to be attached to a surface.
+
+ In implicit synchronization, ensuring that the rendering operation is
+ complete before the compositor displays the buffer is an implementation
+ detail handled by either the kernel or userspace graphics driver.
+
+ By contrast, in explicit synchronization, dma_fence objects mark when the
+ asynchronous operations are complete. When submitting a buffer, the
+ client provides an acquire fence which will be waited on before the
+ compositor accesses the buffer. The Wayland server, through a
+ zwp_buffer_release_v1 object, will inform the client with an event which
+ may be accompanied by a release fence, when the buffer can be
+ destructively accessed.
+
+ Each surface can be associated with only one object of this interface at
+ any time.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy synchronization object">
+ Destroy this explicit synchronization object.
+
+ Any fence set by this object with set_acquire_fence since the last
+ commit will be discarded by the server. Any fences set by this object
+ before the last commit are not affected.
+
+ zwp_buffer_release_v1 objects created by this object are not affected
+ by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="invalid_fence" value="0"
+ summary="the fence specified by the client could not be imported"/>
+ <entry name="duplicate_fence" value="1"
+ summary="multiple fences added for a single surface commit"/>
+ <entry name="duplicate_release" value="2"
+ summary="multiple releases added for a single surface commit"/>
+ <entry name="no_surface" value="3"
+ summary="the associated wl_surface was destroyed"/>
+ </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_buffer_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 fence could not be imported, 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.
+ </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_buffer.attach. See zwp_buffer_release_v1
+ documentation for more information.
+
+ The release object is double-buffered state, and will be applied
+ on the next wl_surface.commit request for the associated surface.
+
+ If a zwp_buffer_release_v1 object has already been requested for
+ the surface in the same commit cycle, a DUPLICATE_RELEASE error
+ is signaled to the client.
+
+ If the associated wl_surface was destroyed, a NO_SURFACE error
+ is signaled to the client.
+ </description>
+
+ <arg name="release" type="new_id" interface="zwp_buffer_release_v1"
+ summary="new zwp_buffer_release_v1 object"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1.get_release request.
+
+ It provides an alternative to wl_buffer.release events, providing a unique
+ release from a single wl_surface.commit request. The release event also
+ supports explicit synchronization, providing a fence FD where relevant for
+ the client to synchronize against before reusing the buffer.
+
+ Exactly one event, either a fenced_release or an immediate_release, will
+ be emitted for each wl_surface.commit request.
+
+ This event does not replace wl_buffer.release events; servers are still
+ required to send those events.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy buffer release synchronization object">
+ Destroy this buffer release explicit synchronization object. The object
+ may be destroyed at any time.
+ </description>
+ </request>
+
+ <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.
+
+ Clients must not perform any destructive operations (e.g. clearing or
+ rendering) on the buffer until that fence has signaled.
+ </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, and any destructive operations on the buffer
+ will have no external effects.
+ </description>
+ </event>
+ </interface>
+
+</protocol>
--
2.19.1
Alexandros Frantzis
2018-10-18 13:48:53 UTC
Permalink
Signed-off-by: Alexandros Frantzis <***@collabora.com>
---

Changes in patch v4:
- Guarantee protocol compatibility only with zwp_linux_dmabuf buffers.
- Add 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.

patch v1 here: https://patchwork.freedesktop.org/patch/177866/

Makefile.am | 1 +
.../linux-explicit-synchronization/README | 6 +
...x-explicit-synchronization-unstable-v1.xml | 232 ++++++++++++++++++
3 files changed, 239 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 <***@chromium.org>
+Daniel Stone <***@collabora.com>
+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..4800756
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
@@ -0,0 +1,232 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="zwp_linux_explicit_synchronization_unstable_v1">
+
+ <copyright>
+ Copyright 2016 The Chromium Authors.
+ Copyright 2017 Intel Corporation
+ Copyright 2018 Collabora, Ltd
+
+ Permission is hereby granted, free of charge, to any person obtaining a
+ copy of this software and associated documentation files (the "Software"),
+ to deal in the Software without restriction, including without limitation
+ the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ and/or sell copies of the Software, and to permit persons to whom the
+ Software is furnished to do so, subject to the following conditions:
+
+ The above copyright notice and this permission notice (including the next
+ paragraph) shall be included in all copies or substantial portions of the
+ Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ DEALINGS IN THE SOFTWARE.
+ </copyright>
+
+ <interface name="zwp_linux_explicit_synchronization_v1" version="1">
+ <description summary="protocol for providing explicit synchronization">
+ This global is a factory interface, allowing clients to request
+ explicit synchronization for buffers on a per-surface basis.
+
+ See zwp_surface_synchronization_v1 for more information.
+
+ This interface is derived from Chromium's
+ zcr_linux_explicit_synchronization_v1.
+
+ Warning! The protocol described in this file is experimental and
+ backward incompatible changes may be made. Backward compatible changes
+ may be added together with the corresponding interface version bump.
+ Backward incompatible changes are done by bumping the version number in
+ the protocol and interface names and resetting the interface version.
+ Once the protocol is to be declared stable, the 'z' prefix and the
+ version number in the protocol and interface names are removed and the
+ interface version number is reset.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy explicit synchronization factory object">
+ Destroy this explicit synchronization factory object. Other objects,
+ including zwp_surface_synchronization_v1 objects created by this
+ factory, shall not be affected by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="synchronization_exists" value="0"
+ summary="the surface already has a synchronization object associated"/>
+ </enum>
+
+ <request name="get_synchronization">
+ <description summary="extend surface interface for explicit synchronization">
+ Instantiate an interface extension for the given wl_surface to provide
+ explicit synchronization.
+
+ If the given wl_surface already has an explicit synchronization object
+ associated, the synchronization_exists protocol error is raised.
+ </description>
+
+ <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
+ summary="the new synchronization interface id"/>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_surface_synchronization_v1" version="1">
+ <description summary="per-surface explicit synchronization support">
+ This object implements per-surface explicit synchronization.
+
+ Synchronization refers to co-ordination of pipelined operations performed
+ on buffers. Most GPU clients will schedule an asynchronous operation to
+ render to the buffer, then immediately send the buffer to the compositor
+ to be attached to a surface.
+
+ In implicit synchronization, ensuring that the rendering operation is
+ complete before the compositor displays the buffer is an implementation
+ detail handled by either the kernel or userspace graphics driver.
+
+ By contrast, in explicit synchronization, dma_fence objects mark when the
+ asynchronous operations are complete. When submitting a buffer, the
+ client provides an acquire fence which will be waited on before the
+ compositor accesses the buffer. The Wayland server, through a
+ zwp_buffer_release_v1 object, will inform the client with an event which
+ may be accompanied by a release fence, when the buffer can be
+ destructively accessed.
+
+ 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 zwp_linux_dmabuf buffer factory.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy synchronization object">
+ Destroy this explicit synchronization object.
+
+ Any fence set by this object with set_acquire_fence since the last
+ commit will be discarded by the server. Any fences set by this object
+ before the last commit are not affected.
+
+ zwp_buffer_release_v1 objects created by this object are not affected
+ by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="invalid_fence" value="0"
+ summary="the fence specified by the client could not be imported"/>
+ <entry name="duplicate_fence" value="1"
+ summary="multiple fences added for a single surface commit"/>
+ <entry name="duplicate_release" value="2"
+ summary="multiple releases added for a single surface commit"/>
+ <entry name="no_surface" value="3"
+ summary="the associated wl_surface was destroyed"/>
+ <entry name="unsupported_buffer" value="4"
+ summary="the buffer does not support explicit synchronization"/>
+ </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_buffer_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 fence could not be imported, 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, or there is no buffer attached, an UNSUPPORTED_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_buffer.attach. See zwp_buffer_release_v1
+ documentation for more information.
+
+ The release object is double-buffered state, and will be applied
+ on the next wl_surface.commit request for the associated surface.
+
+ If a zwp_buffer_release_v1 object has already been requested for
+ the surface in the same commit cycle, a DUPLICATE_RELEASE error
+ is signaled to the client.
+
+ If the associated wl_surface was destroyed, a NO_SURFACE error
+ is signaled to the client.
+
+ If at surface commit time the attached buffer does not support
+ explicit synchronization, or there is no buffer attached, an
+ UNSUPPORTED_BUFFER error is raised.
+ </description>
+
+ <arg name="release" type="new_id" interface="zwp_buffer_release_v1"
+ summary="new zwp_buffer_release_v1 object"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1.get_release request.
+
+ It provides an alternative to wl_buffer.release events, providing a unique
+ release from a single wl_surface.commit request. The release event also
+ supports explicit synchronization, providing a fence FD where relevant for
+ the client to synchronize against before reusing the buffer.
+
+ Exactly one event, either a fenced_release or an immediate_release, will
+ be emitted for each wl_surface.commit request.
+
+ This event does not replace wl_buffer.release events; servers are still
+ required to send those events.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy buffer release synchronization object">
+ Destroy this buffer release explicit synchronization object. The object
+ may be destroyed at any time.
+ </description>
+ </request>
+
+ <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.
+
+ Clients must not perform any destructive operations (e.g. clearing or
+ rendering) on the buffer until that fence has signaled.
+ </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, and any destructive operations on the buffer
+ will have no external effects.
+ </description>
+ </event>
+ </interface>
+
+</protocol>
--
2.19.1
Pekka Paalanen
2018-10-31 11:22:41 UTC
Permalink
On Thu, 18 Oct 2018 16:48:53 +0300
Post by Alexandros Frantzis
---
- Guarantee protocol compatibility only with zwp_linux_dmabuf buffers.
- Add the UNSUPPORTED_BUFFER error.
- 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.
- 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.
patch v1 here: https://patchwork.freedesktop.org/patch/177866/
Hi Alf,

looks even better! Just few details below I'd still like to clarify.
Post by Alexandros Frantzis
Makefile.am | 1 +
.../linux-explicit-synchronization/README | 6 +
...x-explicit-synchronization-unstable-v1.xml | 232 ++++++++++++++++++
3 files changed, 239 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
+
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..4800756
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
@@ -0,0 +1,232 @@
+<?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
+
+ The above copyright notice and this permission notice (including the next
+ paragraph) shall be included in all copies or substantial portions of the
+ Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ DEALINGS IN THE SOFTWARE.
+ </copyright>
+
+ <interface name="zwp_linux_explicit_synchronization_v1" version="1">
+ <description summary="protocol for providing explicit synchronization">
+ This global is a factory interface, allowing clients to request
+ explicit synchronization for buffers on a per-surface basis.
+
+ See zwp_surface_synchronization_v1 for more information.
+
+ This interface is derived from Chromium's
+ zcr_linux_explicit_synchronization_v1.
+
+ Warning! The protocol described in this file is experimental and
+ backward incompatible changes may be made. Backward compatible changes
+ may be added together with the corresponding interface version bump.
+ Backward incompatible changes are done by bumping the version number in
+ the protocol and interface names and resetting the interface version.
+ Once the protocol is to be declared stable, the 'z' prefix and the
+ version number in the protocol and interface names are removed and the
+ interface version number is reset.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy explicit synchronization factory object">
+ Destroy this explicit synchronization factory object. Other objects,
+ including zwp_surface_synchronization_v1 objects created by this
+ factory, shall not be affected by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="synchronization_exists" value="0"
+ summary="the surface already has a synchronization object associated"/>
+ </enum>
+
+ <request name="get_synchronization">
+ <description summary="extend surface interface for explicit synchronization">
+ Instantiate an interface extension for the given wl_surface to provide
+ explicit synchronization.
+
+ If the given wl_surface already has an explicit synchronization object
+ associated, the synchronization_exists protocol error is raised.
+ </description>
+
+ <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
+ summary="the new synchronization interface id"/>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_surface_synchronization_v1" version="1">
+ <description summary="per-surface explicit synchronization support">
+ This object implements per-surface explicit synchronization.
+
+ Synchronization refers to co-ordination of pipelined operations performed
+ on buffers. Most GPU clients will schedule an asynchronous operation to
+ render to the buffer, then immediately send the buffer to the compositor
+ to be attached to a surface.
+
+ In implicit synchronization, ensuring that the rendering operation is
+ complete before the compositor displays the buffer is an implementation
+ detail handled by either the kernel or userspace graphics driver.
+
+ By contrast, in explicit synchronization, dma_fence objects mark when the
+ asynchronous operations are complete. When submitting a buffer, the
+ client provides an acquire fence which will be waited on before the
+ compositor accesses the buffer. The Wayland server, through a
+ zwp_buffer_release_v1 object, will inform the client with an event which
+ may be accompanied by a release fence, when the buffer can be
+ destructively accessed.
Since this is actually handled per-commit, unlike the old
wl_buffer.release event which is defined as "can be destructively
accessed", the wording here could be more like:

"when the compositor will no longer access the buffer contents due to
the specific commit that requested the release event."

It's hard to word clearly and conscisely, but I hope you get the idea
of how this different from "can be destructively accessed" which is a
global concept rather than tied to a specific commit. A client may do
several commits on same or different wl_surfaces, and only after they
all have signalled the explicit fence release, the destructive access is
harmless.

The reason why https://gitlab.freedesktop.org/wayland/wayland/issues/46
is essentially unsolvable without new protocol is that
wl_buffer.release is not tied to a specific commit. If a client commits
the same buffer several times (same or different wl_surfaces), it
cannot know how many wl_buffer.release events it needs before the
buffer really is free due to asynchronicity of the protocol.

Therefore to me the point that the new release event is scoped only to
the specific commit is very important.
Post by Alexandros Frantzis
+
+ 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 zwp_linux_dmabuf buffer factory.
Maybe this limitation should be for set_acquire_fence only? See below
about get_release.
Post by Alexandros Frantzis
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy synchronization object">
+ Destroy this explicit synchronization object.
+
+ Any fence set by this object with set_acquire_fence since the last
+ commit will be discarded by the server. Any fences set by this object
+ before the last commit are not affected.
+
+ zwp_buffer_release_v1 objects created by this object are not affected
+ by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="invalid_fence" value="0"
+ summary="the fence specified by the client could not be imported"/>
+ <entry name="duplicate_fence" value="1"
+ summary="multiple fences added for a single surface commit"/>
+ <entry name="duplicate_release" value="2"
+ summary="multiple releases added for a single surface commit"/>
+ <entry name="no_surface" value="3"
+ summary="the associated wl_surface was destroyed"/>
+ <entry name="unsupported_buffer" value="4"
+ summary="the buffer does not support explicit synchronization"/>
+ </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_buffer_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 fence could not be imported, an INVALID_FENCE error is raised.
Should we allow raising INVALID_FENCE on wl_surface.commit, or are we
sure the import happens in full at set_acquire_fence time?
Post by Alexandros Frantzis
+
+ 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, or there is no buffer attached, an UNSUPPORTED_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_buffer.attach. See zwp_buffer_release_v1
+ documentation for more information.
+
+ The release object is double-buffered state, and will be applied
+ on the next wl_surface.commit request for the associated surface.
+
+ If a zwp_buffer_release_v1 object has already been requested for
+ the surface in the same commit cycle, a DUPLICATE_RELEASE error
+ is signaled to the client.
+
+ If the associated wl_surface was destroyed, a NO_SURFACE error
+ is signaled to the client.
+
+ If at surface commit time the attached buffer does not support
+ explicit synchronization, or there is no buffer attached, an
+ UNSUPPORTED_BUFFER error is raised.
Is UNSUPPORTED_BUFFER error actually necessary here? Why would the
buffer type affect whether the compositor can service a
zwp_buffer_release object or not?

In the worst case, the compositor will not be able to provide release
fence fd, in which case it can always rely on immediate_release event.

If we allow get_release for all buffer types, immediate_release becomes
like wl_buffer.release but scoped to the specific commit, fixing
https://gitlab.freedesktop.org/wayland/wayland/issues/46 for everything.

It will be up to the client to decide to use get_release on whether it
can handle both immediate_release and fenced_release.
Post by Alexandros Frantzis
+ </description>
+
+ <arg name="release" type="new_id" interface="zwp_buffer_release_v1"
+ summary="new zwp_buffer_release_v1 object"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1.get_release request.
+
+ It provides an alternative to wl_buffer.release events, providing a unique
+ release from a single wl_surface.commit request. The release event also
+ supports explicit synchronization, providing a fence FD where relevant for
+ the client to synchronize against before reusing the buffer.
+
+ Exactly one event, either a fenced_release or an immediate_release, will
+ be emitted for each wl_surface.commit request.
Should we add something like: "The compositor can choose release by
release which event it uses."

Just to underline that clients really need to be prepared to deal with
both kinds if they use get_release?

Or is already clear enough?
Post by Alexandros Frantzis
+
+ This event does not replace wl_buffer.release events; servers are still
+ required to send those events.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy buffer release synchronization object">
+ Destroy this buffer release explicit synchronization object. The object
+ may be destroyed at any time.
+ </description>
+ </request>
We can keep the destroy request for now, removing it would be just a
minor optimization.
Post by Alexandros Frantzis
+
+ <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.
+
+ Clients must not perform any destructive operations (e.g. clearing or
+ rendering) on the buffer until that fence has signaled.
+ </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, and any destructive operations on the buffer
+ will have no external effects.
+ </description>
+ </event>
+ </interface>
+
+</protocol>
Thanks,
pq
Simon Ser
2018-11-01 22:10:40 UTC
Permalink
Hi Alexandros,

Here are a few comments about someone who doesn't know a lot about
explicit synchronization. Let me know if I got something wrong. :)

Overall this looks pretty good.
Post by Alexandros Frantzis
---
- Guarantee protocol compatibility only with zwp_linux_dmabuf buffers.
- Add the UNSUPPORTED_BUFFER error.
- 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.
- 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.
patch v1 here: https://patchwork.freedesktop.org/patch/177866/
Makefile.am | 1 +
.../linux-explicit-synchronization/README | 6 +
...x-explicit-synchronization-unstable-v1.xml | 232 ++++++++++++++++++
3 files changed, 239 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
+
diff --git a/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml > b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
Post by Alexandros Frantzis
new file mode 100644
index 0000000..4800756
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
@@ -0,0 +1,232 @@
+<?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
+
+ The above copyright notice and this permission notice (including the next
+ paragraph) shall be included in all copies or substantial portions of the
+ Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ DEALINGS IN THE SOFTWARE.
+ </copyright>
+
+ <interface name="zwp_linux_explicit_synchronization_v1" version="1">
+ <description summary="protocol for providing explicit synchronization">
+ This global is a factory interface, allowing clients to request
+ explicit synchronization for buffers on a per-surface basis.
+
+ See zwp_surface_synchronization_v1 for more information.
+
+ This interface is derived from Chromium's
+ zcr_linux_explicit_synchronization_v1.
+
+ Warning! The protocol described in this file is experimental and
+ backward incompatible changes may be made. Backward compatible changes
+ may be added together with the corresponding interface version bump.
+ Backward incompatible changes are done by bumping the version number in
+ the protocol and interface names and resetting the interface version.
+ Once the protocol is to be declared stable, the 'z' prefix and the
+ version number in the protocol and interface names are removed and the
+ interface version number is reset.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy explicit synchronization factory object">
+ Destroy this explicit synchronization factory object. Other objects,
+ including zwp_surface_synchronization_v1 objects created by this
+ factory, shall not be affected by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="synchronization_exists" value="0"
+ summary="the surface already has a synchronization object associated"/>
+ </enum>
+
+ <request name="get_synchronization">
+ <description summary="extend surface interface for explicit synchronization">
+ Instantiate an interface extension for the given wl_surface to provide
+ explicit synchronization.
+
+ If the given wl_surface already has an explicit synchronization object
+ associated, the synchronization_exists protocol error is raised.
+ </description>
+
+ <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
+ summary="the new synchronization interface id"/>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_surface_synchronization_v1" version="1">
+ <description summary="per-surface explicit synchronization support">
+ This object implements per-surface explicit synchronization.
+
+ Synchronization refers to co-ordination of pipelined operations performed
+ on buffers. Most GPU clients will schedule an asynchronous operation to
+ render to the buffer, then immediately send the buffer to the compositor
+ to be attached to a surface.
+
+ In implicit synchronization, ensuring that the rendering operation is
+ complete before the compositor displays the buffer is an implementation
+ detail handled by either the kernel or userspace graphics driver.
+
+ By contrast, in explicit synchronization, dma_fence objects mark when the
+ asynchronous operations are complete. When submitting a buffer, the
+ client provides an acquire fence which will be waited on before the
+ compositor accesses the buffer. The Wayland server, through a
+ zwp_buffer_release_v1 object, will inform the client with an event which
+ may be accompanied by a release fence, when the buffer can be
+ destructively accessed.
+
+ 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 zwp_linux_dmabuf buffer factory.
I think we can drop the "z" prefix here.
Post by Alexandros Frantzis
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy synchronization object">
+ Destroy this explicit synchronization object.
+
+ Any fence set by this object with set_acquire_fence since the last
+ commit will be discarded by the server. Any fences set by this object
+ before the last commit are not affected.
+
+ zwp_buffer_release_v1 objects created by this object are not affected
+ by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="invalid_fence" value="0"
+ summary="the fence specified by the client could not be imported"/>
+ <entry name="duplicate_fence" value="1"
+ summary="multiple fences added for a single surface commit"/>
+ <entry name="duplicate_release" value="2"
+ summary="multiple releases added for a single surface commit"/>
+ <entry name="no_surface" value="3"
+ summary="the associated wl_surface was destroyed"/>
+ <entry name="unsupported_buffer" value="4"
+ summary="the buffer does not support explicit synchronization"/>
+ </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_buffer_attach. The fence
wl_buffer.attach
Post by Alexandros Frantzis
+ 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 fence could not be imported, an INVALID_FENCE error is raised.
I wonder if failures to import a fence should really be protocol errors.
Protocol errors are meant to be used for protocol violations. I understand that
a client can send an invalid fence, but are there other reasons why a fence
cannot be imported? Maybe we could change this to "if the file descriptor isn't
a dma_fence"?
Post by Alexandros Frantzis
+ 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, or there is no buffer attached, an UNSUPPORTED_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_buffer.attach. See zwp_buffer_release_v1
+ documentation for more information.
+
+ The release object is double-buffered state, and will be applied
+ on the next wl_surface.commit request for the associated surface.
"will be applied" is a little bit strange for this request. Maybe change to
"will provide release information about the next wl_surface.commit request"?
Post by Alexandros Frantzis
+ If a zwp_buffer_release_v1 object has already been requested for
+ the surface in the same commit cycle, a DUPLICATE_RELEASE error
+ is signaled to the client.
+
+ If the associated wl_surface was destroyed, a NO_SURFACE error
+ is signaled to the client.
+
+ If at surface commit time the attached buffer does not support
+ explicit synchronization, or there is no buffer attached, an
+ UNSUPPORTED_BUFFER error is raised.
I agree with pq here. It would be nice to be able to use this request on any
buffer type.
Post by Alexandros Frantzis
+ </description>
+
Extra line here
Post by Alexandros Frantzis
+ <arg name="release" type="new_id" interface="zwp_buffer_release_v1"
+ summary="new zwp_buffer_release_v1 object"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1.get_release request.
+
+ It provides an alternative to wl_buffer.release events, providing a unique
+ release from a single wl_surface.commit request. The release event also
+ supports explicit synchronization, providing a fence FD where relevant for
+ the client to synchronize against before reusing the buffer.
+
+ Exactly one event, either a fenced_release or an immediate_release, will
+ be emitted for each wl_surface.commit request.
This makes it sound like this object can be used for multiple commits. Maybe we
can change the wording to "will be emitted after the wl_surface.commit request".
Post by Alexandros Frantzis
+ This event does not replace wl_buffer.release events; servers are still
+ required to send those events.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy buffer release synchronization object">
+ Destroy this buffer release explicit synchronization object. The object
+ may be destroyed at any time.
+ </description>
+ </request>
+
+ <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.
+
+ Clients must not perform any destructive operations (e.g. clearing or
+ rendering) on the buffer until that fence has signaled.
We should probably add to this request and to immediate_release something among
Post by Alexandros Frantzis
Upon receiving this event, the client should destroy this 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, and any destructive operations on the buffer
+ will have no external effects.
+ </description>
+ </event>
+ </interface>
+
+</protocol>
--
2.19.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Alexandros Frantzis
2018-11-02 16:40:54 UTC
Permalink
Post by Simon Ser
Hi Alexandros,
Here are a few comments about someone who doesn't know a lot about
explicit synchronization. Let me know if I got something wrong. :)
Overall this looks pretty good.
Hi Simon,

thanks for the review. My comments are below. I agree with everything I
have left out.
Post by Simon Ser
Post by Alexandros Frantzis
+ Explicit synchronization is guaranteed to be supported only for buffers
+ created with any version of the zwp_linux_dmabuf buffer factory.
I think we can drop the "z" prefix here.
Hmm, not sure about this. We would be using a protocol prefix/name
combination that doesn't exist (yet). Of course the intention is clear,
but I think it would be better to update this to, e.g.,
(z)wp_linux_dmabuf, when linux_dmabuf actually becomes stable.
Post by Simon Ser
Post by Alexandros Frantzis
+ 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 fence could not be imported, an INVALID_FENCE error is raised.
I wonder if failures to import a fence should really be protocol errors.
Protocol errors are meant to be used for protocol violations. I understand that
a client can send an invalid fence, but are there other reasons why a fence
cannot be imported? Maybe we could change this to "if the file descriptor isn't
a dma_fence"?
Yes, the this is all about having a valid fence (and that's how it's
implemented in the WIP branch). Requiring something more from this
request would be asking too much, both from the server and the client. I
will rephrase.

It's unlikely that we won't be able to use valid fence at a later point.
If this happens it's most likely a driver issue (or we have messed up in
weston), in which case I think disconnecting the client is a valid
option (compared to allowing bad data on screen), but that's a different
case and error.
Post by Simon Ser
Post by Alexandros Frantzis
+ <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_buffer.attach. See zwp_buffer_release_v1
+ documentation for more information.
+
+ The release object is double-buffered state, and will be applied
+ on the next wl_surface.commit request for the associated surface.
"will be applied" is a little bit strange for this request. Maybe change to
"will provide release information about the next wl_surface.commit request"?
"will be applied" tries to convey that the release will be associated
with the buffer that is attached at commit time (instead of any
intermediate attachments).

So, perhaps "will be associated with the buffer that is attached to the
surface at wl_surface.commit time"?
Post by Simon Ser
Post by Alexandros Frantzis
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1.get_release request.
+
+ It provides an alternative to wl_buffer.release events, providing a unique
+ release from a single wl_surface.commit request. The release event also
+ supports explicit synchronization, providing a fence FD where relevant for
+ the client to synchronize against before reusing the buffer.
+
+ Exactly one event, either a fenced_release or an immediate_release, will
+ be emitted for each wl_surface.commit request.
This makes it sound like this object can be used for multiple commits. Maybe we
can change the wording to "will be emitted after the wl_surface.commit request".
Perhaps "will be emitted for the wl_surface.commit request", instead? My
concern is that "after" may be misread as the commit immediately
triggering an event (which, to be fair, doesn't make sense in this
context).
Post by Simon Ser
Post by Alexandros Frantzis
+ <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.
+
+ Clients must not perform any destructive operations (e.g. clearing or
+ rendering) on the buffer until that fence has signaled.
We should probably add to this request and to immediate_release something among
Post by Alexandros Frantzis
Upon receiving this event, the client should destroy this object.
In v5 I am changing zwp_buffer_release to use destroy-on-event, so this
doesn't apply any more. Of course, the client should still destroy the
proxy, but I don't think this is something we need to explicitly state.

Thanks,
Alexandros
Simon Ser
2018-11-03 14:44:53 UTC
Permalink
Post by Alexandros Frantzis
Post by Simon Ser
Post by Alexandros Frantzis
+ Explicit synchronization is guaranteed to be supported only for buffers
+ created with any version of the zwp_linux_dmabuf buffer factory.
I think we can drop the "z" prefix here.
Hmm, not sure about this. We would be using a protocol prefix/name
combination that doesn't exist (yet). Of course the intention is clear,
but I think it would be better to update this to, e.g.,
(z)wp_linux_dmabuf, when linux_dmabuf actually becomes stable.
Then add the v1 suffix zwp_linux_dmabuf_v1?
Post by Alexandros Frantzis
Post by Simon Ser
I wonder if failures to import a fence should really be protocol errors.
Protocol errors are meant to be used for protocol violations. I understand that
a client can send an invalid fence, but are there other reasons why a fence
cannot be imported? Maybe we could change this to "if the file descriptor isn't
a dma_fence"?
Yes, the this is all about having a valid fence (and that's how it's
implemented in the WIP branch). Requiring something more from this
request would be asking too much, both from the server and the client. I
will rephrase.
It's unlikely that we won't be able to use valid fence at a later point.
If this happens it's most likely a driver issue (or we have messed up in
weston), in which case I think disconnecting the client is a valid
option (compared to allowing bad data on screen), but that's a different
case and error.
Makes sense.
Post by Alexandros Frantzis
Post by Simon Ser
Post by Alexandros Frantzis
+ <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_buffer.attach. See zwp_buffer_release_v1
+ documentation for more information.
+
+ The release object is double-buffered state, and will be applied
+ on the next wl_surface.commit request for the associated surface.
"will be applied" is a little bit strange for this request. Maybe change to
"will provide release information about the next wl_surface.commit request"?
"will be applied" tries to convey that the release will be associated
with the buffer that is attached at commit time (instead of any
intermediate attachments).
So, perhaps "will be associated with the buffer that is attached to the
surface at wl_surface.commit time"?
+1
Post by Alexandros Frantzis
Post by Simon Ser
Post by Alexandros Frantzis
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1.get_release request.
+
+ It provides an alternative to wl_buffer.release events, providing a unique
+ release from a single wl_surface.commit request. The release event also
+ supports explicit synchronization, providing a fence FD where relevant for
+ the client to synchronize against before reusing the buffer.
+
+ Exactly one event, either a fenced_release or an immediate_release, will
+ be emitted for each wl_surface.commit request.
This makes it sound like this object can be used for multiple commits. Maybe we
can change the wording to "will be emitted after the wl_surface.commit request".
Perhaps "will be emitted for the wl_surface.commit request", instead? My
concern is that "after" may be misread as the commit immediately
triggering an event (which, to be fair, doesn't make sense in this
context).
Sounds good.
Post by Alexandros Frantzis
Post by Simon Ser
Post by Alexandros Frantzis
+ <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.
+
+ Clients must not perform any destructive operations (e.g. clearing or
+ rendering) on the buffer until that fence has signaled.
We should probably add to this request and to immediate_release something among
Post by Alexandros Frantzis
Upon receiving this event, the client should destroy this object.
In v5 I am changing zwp_buffer_release to use destroy-on-event, so this
doesn't apply any more. Of course, the client should still destroy the
proxy, but I don't think this is something we need to explicitly state.
Hmm. One should be careful when choosing destroy-on-event, it makes it
impossible to add requests to the interface later on.
Alexandros Frantzis
2018-11-05 09:50:19 UTC
Permalink
On Sat, Nov 03, 2018 at 02:44:53PM +0000, Simon Ser wrote:

Hi Simon,
Post by Simon Ser
Post by Alexandros Frantzis
Post by Simon Ser
Post by Alexandros Frantzis
+ Explicit synchronization is guaranteed to be supported only for buffers
+ created with any version of the zwp_linux_dmabuf buffer factory.
I think we can drop the "z" prefix here.
Hmm, not sure about this. We would be using a protocol prefix/name
combination that doesn't exist (yet). Of course the intention is clear,
but I think it would be better to update this to, e.g.,
(z)wp_linux_dmabuf, when linux_dmabuf actually becomes stable.
Then add the v1 suffix zwp_linux_dmabuf_v1?
The "any version" of zwp_linux_dmabuf was meant to cover both _v* and
version="*", but I guess that's not clear enough. Perhaps "with any
version of zwp_linux_dmabuf_v*"? In any case, I am also fine with just
adding _v1 for now and updating as needed in the future.
Post by Simon Ser
Post by Alexandros Frantzis
Post by Simon Ser
Post by Alexandros Frantzis
+ <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.
+
+ Clients must not perform any destructive operations (e.g. clearing or
+ rendering) on the buffer until that fence has signaled.
We should probably add to this request and to immediate_release something among
Post by Alexandros Frantzis
Upon receiving this event, the client should destroy this object.
In v5 I am changing zwp_buffer_release to use destroy-on-event, so this
doesn't apply any more. Of course, the client should still destroy the
proxy, but I don't think this is something we need to explicitly state.
Hmm. One should be careful when choosing destroy-on-event, it makes it
impossible to add requests to the interface later on.
We discussed a bit about this in an earlier email, and the conclusion
was that buffer_release is expected to remain a pure event interface.
It's not expected to gain any requests, or to be used as an argument to
other requests. Of course, if people have any such use cases in mind I
would be happy to hear about them and discuss further.

Thanks,
Alexandros
Simon Ser
2018-11-05 18:07:56 UTC
Permalink
Post by Alexandros Frantzis
Hi Simon,
Post by Simon Ser
Post by Alexandros Frantzis
Post by Simon Ser
Post by Alexandros Frantzis
+ Explicit synchronization is guaranteed to be supported only for buffers
+ created with any version of the zwp_linux_dmabuf buffer factory.
I think we can drop the "z" prefix here.
Hmm, not sure about this. We would be using a protocol prefix/name
combination that doesn't exist (yet). Of course the intention is clear,
but I think it would be better to update this to, e.g.,
(z)wp_linux_dmabuf, when linux_dmabuf actually becomes stable.
Then add the v1 suffix zwp_linux_dmabuf_v1?
The "any version" of zwp_linux_dmabuf was meant to cover both _v* and
version="*", but I guess that's not clear enough. Perhaps "with any
version of zwp_linux_dmabuf_v*"? In any case, I am also fine with just
adding _v1 for now and updating as needed in the future.
It's not uncommon to refer to the not-yet-existing stable versions of the
protocols in their unstable versions.

If you think all DMA-BUFs can use this protocol, use wp_linux_dmabuf. If it only
applies to the unstable-v1 protocol, use zwp_linux_dmabuf_v1.
Post by Alexandros Frantzis
Post by Simon Ser
Post by Alexandros Frantzis
Post by Simon Ser
Post by Alexandros Frantzis
+ <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.
+
+ Clients must not perform any destructive operations (e.g. clearing or
+ rendering) on the buffer until that fence has signaled.
We should probably add to this request and to immediate_release something among
Post by Alexandros Frantzis
Upon receiving this event, the client should destroy this object.
In v5 I am changing zwp_buffer_release to use destroy-on-event, so this
doesn't apply any more. Of course, the client should still destroy the
proxy, but I don't think this is something we need to explicitly state.
Hmm. One should be careful when choosing destroy-on-event, it makes it
impossible to add requests to the interface later on.
We discussed a bit about this in an earlier email, and the conclusion
was that buffer_release is expected to remain a pure event interface.
It's not expected to gain any requests, or to be used as an argument to
other requests. Of course, if people have any such use cases in mind I
would be happy to hear about them and discuss further.
Okay. In this case we need to make it clear which events make the compositor
destroy the object. See for instance the wl_surface.frame description:

The object returned by this request will be destroyed by the
compositor after the callback is fired and as such the client must not
attempt to use it after that point.
Alexandros Frantzis
2018-11-06 12:12:13 UTC
Permalink
Post by Simon Ser
Post by Alexandros Frantzis
Hi Simon,
Post by Simon Ser
Post by Alexandros Frantzis
Post by Simon Ser
Post by Alexandros Frantzis
+ Explicit synchronization is guaranteed to be supported only for buffers
+ created with any version of the zwp_linux_dmabuf buffer factory.
I think we can drop the "z" prefix here.
Hmm, not sure about this. We would be using a protocol prefix/name
combination that doesn't exist (yet). Of course the intention is clear,
but I think it would be better to update this to, e.g.,
(z)wp_linux_dmabuf, when linux_dmabuf actually becomes stable.
Then add the v1 suffix zwp_linux_dmabuf_v1?
The "any version" of zwp_linux_dmabuf was meant to cover both _v* and
version="*", but I guess that's not clear enough. Perhaps "with any
version of zwp_linux_dmabuf_v*"? In any case, I am also fine with just
adding _v1 for now and updating as needed in the future.
It's not uncommon to refer to the not-yet-existing stable versions of the
protocols in their unstable versions.
Since there are precedents in the repo of using the not-yet-existing
stable name, and as long as it's clear that this is a general reference
to all unstable and stable versions, I will go with that.

Thanks,
Alexandros
Alexandros Frantzis
2018-11-06 12:58:14 UTC
Permalink
Signed-off-by: Alexandros Frantzis <***@collabora.com>
---

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

only nitpicks below.
Post by Alexandros Frantzis
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
+
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..834f2e0
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
@@ -0,0 +1,226 @@
+<?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
+
+ The above copyright notice and this permission notice (including the next
+ paragraph) shall be included in all copies or substantial portions of the
+ Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ DEALINGS IN THE SOFTWARE.
+ </copyright>
+
+ <interface name="zwp_linux_explicit_synchronization_v1" version="1">
+ <description summary="protocol for providing explicit synchronization">
+ This global is a factory interface, allowing clients to request
+ explicit synchronization for buffers on a per-surface basis.
+
+ See zwp_surface_synchronization_v1 for more information.
+
+ This interface is derived from Chromium's
+ zcr_linux_explicit_synchronization_v1.
+
+ Warning! The protocol described in this file is experimental and
+ backward incompatible changes may be made. Backward compatible changes
+ may be added together with the corresponding interface version bump.
+ Backward incompatible changes are done by bumping the version number in
+ the protocol and interface names and resetting the interface version.
+ Once the protocol is to be declared stable, the 'z' prefix and the
+ version number in the protocol and interface names are removed and the
+ interface version number is reset.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy explicit synchronization factory object">
+ Destroy this explicit synchronization factory object. Other objects,
+ including zwp_surface_synchronization_v1 objects created by this
+ factory, shall not be affected by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="synchronization_exists" value="0"
+ summary="the surface already has a synchronization object associated"/>
+ </enum>
+
+ <request name="get_synchronization">
+ <description summary="extend surface interface for explicit synchronization">
+ Instantiate an interface extension for the given wl_surface to provide
+ explicit synchronization.
+
+ If the given wl_surface already has an explicit synchronization object
+ associated, the synchronization_exists protocol error is raised.
+ </description>
+
+ <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
+ summary="the new synchronization interface id"/>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_surface_synchronization_v1" version="1">
+ <description summary="per-surface explicit synchronization support">
+ This object implements per-surface explicit synchronization.
+
+ Synchronization refers to co-ordination of pipelined operations performed
+ on buffers. Most GPU clients will schedule an asynchronous operation to
+ render to the buffer, then immediately send the buffer to the compositor
+ to be attached to a surface.
+
+ In implicit synchronization, ensuring that the rendering operation is
+ complete before the compositor displays the buffer is an implementation
+ detail handled by either the kernel or userspace graphics driver.
+
+ By contrast, in explicit synchronization, dma_fence objects mark when the
+ asynchronous operations are complete. When submitting a buffer, the
+ client provides an acquire fence which will be waited on before the
+ compositor accesses the buffer. The Wayland server, through a
+ zwp_buffer_release_v1 object, will inform the client with an event which
+ may be accompanied by a release fence, when the compositor will no longer
+ access the buffer contents due to the specific commit that requested the
+ release event.
+
+ Each surface can be associated with only one object of this interface at
+ any time.
+
+ Explicit synchronization is guaranteed to be supported only for buffers
+ created with any version of the wp_linux_dmabuf buffer factory.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy synchronization object">
+ Destroy this explicit synchronization object.
+
+ Any fence set by this object with set_acquire_fence since the last
+ commit will be discarded by the server. Any fences set by this object
+ before the last commit are not affected.
+
+ zwp_buffer_release_v1 objects created by this object are not affected
+ by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="invalid_fence" value="0"
+ summary="the fence specified by the client could not be imported"/>
+ <entry name="duplicate_fence" value="1"
+ summary="multiple fences added for a single surface commit"/>
+ <entry name="duplicate_release" value="2"
+ summary="multiple releases added for a single surface commit"/>
+ <entry name="no_surface" value="3"
+ summary="the associated wl_surface was destroyed"/>
+ <entry name="unsupported_buffer" value="4"
+ summary="the buffer does not support explicit synchronization"/>
+ </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_buffer.attach. The fence
Still a wl_buffer.attach here, should be wl_surface.attach.
Post by Alexandros Frantzis
+ 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, or there is no buffer attached, an UNSUPPORTED_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_buffer.attach. See zwp_buffer_release_v1
wl_surface.attach
Post by Alexandros Frantzis
+ documentation for more information.
+
+ The release object is double-buffered state, and will be associated
+ with the buffer that is attached to the surface at wl_surface.commit
+ time.
+
+ If a zwp_buffer_release_v1 object has already been requested for
+ the surface in the same commit cycle, a DUPLICATE_RELEASE error
+ is signaled to the client.
"is raised"
Post by Alexandros Frantzis
+
+ If the associated wl_surface was destroyed, a NO_SURFACE error
+ is signaled to the client.
"is raised", just to use the language consistently.
Post by Alexandros Frantzis
+
+ If at surface commit time there is no buffer attached, an
+ UNSUPPORTED_BUFFER error is raised.
This reads a little odd, do we need another error code for "no buffer"?
Post by Alexandros Frantzis
+ </description>
+ <arg name="release" type="new_id" interface="zwp_buffer_release_v1"
+ summary="new zwp_buffer_release_v1 object"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1.get_release request.
+
+ It provides an alternative to wl_buffer.release events, providing a
+ unique release from a single wl_surface.commit request. The release event
+ also supports explicit synchronization, providing a fence FD for the
+ client to synchronize against.
+
+ Exactly one event, either a fenced_release or an immediate_release, will
+ be emitted for the wl_surface.commit request. The compositor can choose
+ release by release which event it uses.
+
+ This event does not replace wl_buffer.release events; servers are still
+ required to send those events.
+
+ Once a buffer release object has delivered a 'fenced_release' or an
+ 'immediate_release' event it is automatically destroyed.
+ </description>
+
+ <event name="fenced_release">
+ <description summary="release buffer with fence">
+ Sent when the compositor has finalised its usage of the associated
+ buffer for the relevant commit, providing a dma_fence which will be
+ signaled when all operations by the compositor on that buffer for that
+ commit have finished.
It wouldn't hurt to repeat here:

"This event destroys the zwp_buffer_release_v1 object."
Post by Alexandros Frantzis
+ </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.
And here.
Post by Alexandros Frantzis
+ </description>
+ </event>
+ </interface>
+
+</protocol>
Looks very good. One more minor revision, and I'll start a one day
count-down to merge this.


Thanks,
pq
Alexandros Frantzis
2018-11-07 14:59:25 UTC
Permalink
Signed-off-by: Alexandros Frantzis <***@collabora.com>
---

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

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

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

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

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

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

diff --git a/Makefile.am b/Makefile.am
index 6394e26..7dfbb9e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -21,6 +21,7 @@ unstable_protocols = \
unstable/xdg-output/xdg-output-unstable-v1.xml \
unstable/input-timestamps/input-timestamps-unstable-v1.xml \
unstable/xdg-decoration/xdg-decoration-unstable-v1.xml \
+ unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml \
$(NULL)

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

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

Below is a small comment, I don't know if it's relevant or not, I just wanted to
point it out.
Post by Alexandros Frantzis
---
- Fixed wl_buffer.attach -> wl_surface.attach typos.
- Added NO_BUFFER error and updated request descriptions.
- Consistently used "error is raised" phrasing.
- Added comment about buffer_release object destruction in event
descriptions.
- 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.
- Guaranteed protocol compatibility only with zwp_linux_dmabuf buffers.
- Added the UNSUPPORTED_BUFFER error.
- 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.
- Added NO_SURFACE error for zwp_surface_synchronization_v1 requests.
- Removed restriction to destroy a zwp_surface_synchronization_v1 object
after the associated wl_surface is destroyed.
- Clarified which buffer the acquire fence is associated with.
- Clarified that exactly one event, either a fenced_release or a
immediate_release, will be emitted for each commit.
Makefile.am | 1 +
.../linux-explicit-synchronization/README | 6 +
...x-explicit-synchronization-unstable-v1.xml | 234 ++++++++++++++++++
3 files changed, 241 insertions(+)
create mode 100644 unstable/linux-explicit-synchronization/README
create mode 100644 unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
diff --git a/Makefile.am b/Makefile.am
index 6394e26..7dfbb9e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -21,6 +21,7 @@ unstable_protocols = \
unstable/xdg-output/xdg-output-unstable-v1.xml \
unstable/input-timestamps/input-timestamps-unstable-v1.xml \
unstable/xdg-decoration/xdg-decoration-unstable-v1.xml \
+ unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml \
$(NULL)
stable_protocols = \
diff --git a/unstable/linux-explicit-synchronization/README b/unstable/linux-explicit-synchronization/README
new file mode 100644
index 0000000..f13b404
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/README
@@ -0,0 +1,6 @@
+Linux explicit synchronization (dma-fence) protocol
+
diff --git a/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml > b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
Post by Alexandros Frantzis
new file mode 100644
index 0000000..b87663f
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
@@ -0,0 +1,234 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="zwp_linux_explicit_synchronization_unstable_v1">
+
+ <copyright>
+ Copyright 2016 The Chromium Authors.
+ Copyright 2017 Intel Corporation
+ Copyright 2018 Collabora, Ltd
+
+ Permission is hereby granted, free of charge, to any person obtaining a
+ copy of this software and associated documentation files (the "Software"),
+ to deal in the Software without restriction, including without limitation
+ the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ and/or sell copies of the Software, and to permit persons to whom the
+
+ The above copyright notice and this permission notice (including the next
+ paragraph) shall be included in all copies or substantial portions of the
+ Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ DEALINGS IN THE SOFTWARE.
+ </copyright>
+
+ <interface name="zwp_linux_explicit_synchronization_v1" version="1">
+ <description summary="protocol for providing explicit synchronization">
+ This global is a factory interface, allowing clients to request
+ explicit synchronization for buffers on a per-surface basis.
+
+ See zwp_surface_synchronization_v1 for more information.
+
+ This interface is derived from Chromium's
+ zcr_linux_explicit_synchronization_v1.
+
+ Warning! The protocol described in this file is experimental and
+ backward incompatible changes may be made. Backward compatible changes
+ may be added together with the corresponding interface version bump.
+ Backward incompatible changes are done by bumping the version number in
+ the protocol and interface names and resetting the interface version.
+ Once the protocol is to be declared stable, the 'z' prefix and the
+ version number in the protocol and interface names are removed and the
+ interface version number is reset.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy explicit synchronization factory object">
+ Destroy this explicit synchronization factory object. Other objects,
+ including zwp_surface_synchronization_v1 objects created by this
+ factory, shall not be affected by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="synchronization_exists" value="0"
+ summary="the surface already has a synchronization object associated"/>
+ </enum>
+
+ <request name="get_synchronization">
+ <description summary="extend surface interface for explicit synchronization">
+ Instantiate an interface extension for the given wl_surface to provide
+ explicit synchronization.
+
+ If the given wl_surface already has an explicit synchronization object
+ associated, the synchronization_exists protocol error is raised.
+ </description>
+
+ <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
+ summary="the new synchronization interface id"/>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_surface_synchronization_v1" version="1">
I missed this before, but: is there a reason why the "linux" prefix has been
dropped here? Maybe it should be renamed to
zwp_linux_surface_synchronization_v1? What about zwp_buffer_release_v1?
Post by Alexandros Frantzis
+ <description summary="per-surface explicit synchronization support">
+ This object implements per-surface explicit synchronization.
+
+ Synchronization refers to co-ordination of pipelined operations performed
+ on buffers. Most GPU clients will schedule an asynchronous operation to
+ render to the buffer, then immediately send the buffer to the compositor
+ to be attached to a surface.
+
+ In implicit synchronization, ensuring that the rendering operation is
+ complete before the compositor displays the buffer is an implementation
+ detail handled by either the kernel or userspace graphics driver.
+
+ By contrast, in explicit synchronization, dma_fence objects mark when the
+ asynchronous operations are complete. When submitting a buffer, the
+ client provides an acquire fence which will be waited on before the
+ compositor accesses the buffer. The Wayland server, through a
+ zwp_buffer_release_v1 object, will inform the client with an event which
+ may be accompanied by a release fence, when the compositor will no longer
+ access the buffer contents due to the specific commit that requested the
+ release event.
+
+ Each surface can be associated with only one object of this interface at
+ any time.
+
+ Explicit synchronization is guaranteed to be supported only for buffers
+ created with any version of the wp_linux_dmabuf buffer factory.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy synchronization object">
+ Destroy this explicit synchronization object.
+
+ Any fence set by this object with set_acquire_fence since the last
+ commit will be discarded by the server. Any fences set by this object
+ before the last commit are not affected.
+
+ zwp_buffer_release_v1 objects created by this object are not affected
+ by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="invalid_fence" value="0"
+ summary="the fence specified by the client could not be imported"/>
+ <entry name="duplicate_fence" value="1"
+ summary="multiple fences added for a single surface commit"/>
+ <entry name="duplicate_release" value="2"
+ summary="multiple releases added for a single surface commit"/>
+ <entry name="no_surface" value="3"
+ summary="the associated wl_surface was destroyed"/>
+ <entry name="unsupported_buffer" value="4"
+ summary="the buffer does not support explicit synchronization"/>
+ <entry name="no_buffer" value="5"
+ summary="no buffer was attached"/>
+ </enum>
+
+ <request name="set_acquire_fence">
+ <description summary="set the acquire fence">
+ Set the acquire fence that must be signaled before the compositor
+ may sample from the buffer attached with wl_surface.attach. The fence
+ is a dma_fence kernel object.
+
+ The acquire fence is double-buffered state, and will be applied on the
+ next wl_surface.commit request for the associated surface. Thus, it
+ applies only to the buffer that is attached to the surface at commit
+ time.
+
+ If the provided fd is not a valid dma_fence fd, then an INVALID_FENCE
+ error is raised.
+
+ If a fence has already been attached during the same commit cycle, a
+ DUPLICATE_FENCE error is raised.
+
+ If the associated wl_surface was destroyed, a NO_SURFACE error is
+ raised.
+
+ If at surface commit time the attached buffer does not support explicit
+ synchronization, an UNSUPPORTED_BUFFER error is raised.
+
+ If at surface commit time there is no buffer attached, a NO_BUFFER
+ error is raised.
+ </description>
+ <arg name="fd" type="fd" summary="acquire fence fd"/>
+ </request>
+
+ <request name="get_release">
+ <description summary="release fence for last-attached buffer">
+ Create a listener for the release of the buffer attached by the
+ client with wl_surface.attach. See zwp_buffer_release_v1
+ documentation for more information.
+
+ The release object is double-buffered state, and will be associated
+ with the buffer that is attached to the surface at wl_surface.commit
+ time.
+
+ If a zwp_buffer_release_v1 object has already been requested for
+ the surface in the same commit cycle, a DUPLICATE_RELEASE error
+ is raised.
+
+ If the associated wl_surface was destroyed, a NO_SURFACE error
+ is raised.
+
+ If at surface commit time there is no buffer attached, a NO_BUFFER
+ error is raised.
+ </description>
+ <arg name="release" type="new_id" interface="zwp_buffer_release_v1"
+ summary="new zwp_buffer_release_v1 object"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1.get_release request.
+
+ It provides an alternative to wl_buffer.release events, providing a
+ unique release from a single wl_surface.commit request. The release event
+ also supports explicit synchronization, providing a fence FD for the
+ client to synchronize against.
+
+ Exactly one event, either a fenced_release or an immediate_release, will
+ be emitted for the wl_surface.commit request. The compositor can choose
+ release by release which event it uses.
+
+ This event does not replace wl_buffer.release events; servers are still
+ required to send those events.
+
+ Once a buffer release object has delivered a 'fenced_release' or an
+ 'immediate_release' event it is automatically destroyed.
+ </description>
+
+ <event name="fenced_release">
+ <description summary="release buffer with fence">
+ Sent when the compositor has finalised its usage of the associated
+ buffer for the relevant commit, providing a dma_fence which will be
+ signaled when all operations by the compositor on that buffer for that
+ commit have finished.
+
+ This event destroys the zwp_buffer_release_v1 object.
+ </description>
+ <arg name="fence" type="fd" summary="fence for last operation on buffer"/>
+ </event>
+
+ <event name="immediate_release">
+ <description summary="release buffer immediately">
+ Sent when the compositor has finalised its usage of the associated
+ buffer for the relevant commit, and either performed no operations
+ using it, or has a guarantee that all its operations on that buffer for
+ that commit have finished.
+
+ This event destroys the zwp_buffer_release_v1 object.
+ </description>
+ </event>
+ </interface>
+
+</protocol>
--
2.19.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-11-08 13:00:58 UTC
Permalink
On Wed, 07 Nov 2018 20:22:38 +0000
Post by Simon Ser
Thanks! With these fixes, this is now
Below is a small comment, I don't know if it's relevant or not, I just wanted to
point it out.
Post by Alexandros Frantzis
---
- Fixed wl_buffer.attach -> wl_surface.attach typos.
- Added NO_BUFFER error and updated request descriptions.
- Consistently used "error is raised" phrasing.
- Added comment about buffer_release object destruction in event
descriptions.
- 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.
- Guaranteed protocol compatibility only with zwp_linux_dmabuf buffers.
- Added the UNSUPPORTED_BUFFER error.
- 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.
- Added NO_SURFACE error for zwp_surface_synchronization_v1 requests.
- Removed restriction to destroy a zwp_surface_synchronization_v1 object
after the associated wl_surface is destroyed.
- Clarified which buffer the acquire fence is associated with.
- Clarified that exactly one event, either a fenced_release or a
immediate_release, will be emitted for each commit.
Makefile.am | 1 +
.../linux-explicit-synchronization/README | 6 +
...x-explicit-synchronization-unstable-v1.xml | 234 ++++++++++++++++++
3 files changed, 241 insertions(+)
create mode 100644 unstable/linux-explicit-synchronization/README
create mode 100644 unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
diff --git a/Makefile.am b/Makefile.am
index 6394e26..7dfbb9e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -21,6 +21,7 @@ unstable_protocols = \
unstable/xdg-output/xdg-output-unstable-v1.xml \
unstable/input-timestamps/input-timestamps-unstable-v1.xml \
unstable/xdg-decoration/xdg-decoration-unstable-v1.xml \
+ unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml \
$(NULL)
stable_protocols = \
diff --git a/unstable/linux-explicit-synchronization/README b/unstable/linux-explicit-synchronization/README
new file mode 100644
index 0000000..f13b404
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/README
@@ -0,0 +1,6 @@
+Linux explicit synchronization (dma-fence) protocol
+
diff --git a/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml > b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
Post by Alexandros Frantzis
new file mode 100644
index 0000000..b87663f
--- /dev/null
+++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
@@ -0,0 +1,234 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="zwp_linux_explicit_synchronization_unstable_v1">
+
+ <copyright>
+ Copyright 2016 The Chromium Authors.
+ Copyright 2017 Intel Corporation
+ Copyright 2018 Collabora, Ltd
+
+ Permission is hereby granted, free of charge, to any person obtaining a
+ copy of this software and associated documentation files (the "Software"),
+ to deal in the Software without restriction, including without limitation
+ the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ and/or sell copies of the Software, and to permit persons to whom the
+
+ The above copyright notice and this permission notice (including the next
+ paragraph) shall be included in all copies or substantial portions of the
+ Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ DEALINGS IN THE SOFTWARE.
+ </copyright>
+
+ <interface name="zwp_linux_explicit_synchronization_v1" version="1">
+ <description summary="protocol for providing explicit synchronization">
+ This global is a factory interface, allowing clients to request
+ explicit synchronization for buffers on a per-surface basis.
+
+ See zwp_surface_synchronization_v1 for more information.
+
+ This interface is derived from Chromium's
+ zcr_linux_explicit_synchronization_v1.
+
+ Warning! The protocol described in this file is experimental and
+ backward incompatible changes may be made. Backward compatible changes
+ may be added together with the corresponding interface version bump.
+ Backward incompatible changes are done by bumping the version number in
+ the protocol and interface names and resetting the interface version.
+ Once the protocol is to be declared stable, the 'z' prefix and the
+ version number in the protocol and interface names are removed and the
+ interface version number is reset.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy explicit synchronization factory object">
+ Destroy this explicit synchronization factory object. Other objects,
+ including zwp_surface_synchronization_v1 objects created by this
+ factory, shall not be affected by this request.
+ </description>
+ </request>
+
+ <enum name="error">
+ <entry name="synchronization_exists" value="0"
+ summary="the surface already has a synchronization object associated"/>
+ </enum>
+
+ <request name="get_synchronization">
+ <description summary="extend surface interface for explicit synchronization">
+ Instantiate an interface extension for the given wl_surface to provide
+ explicit synchronization.
+
+ If the given wl_surface already has an explicit synchronization object
+ associated, the synchronization_exists protocol error is raised.
+ </description>
+
+ <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
+ summary="the new synchronization interface id"/>
+ <arg name="surface" type="object" interface="wl_surface"
+ summary="the surface"/>
+ </request>
+ </interface>
+
+ <interface name="zwp_surface_synchronization_v1" version="1">
I missed this before, but: is there a reason why the "linux" prefix has been
dropped here? Maybe it should be renamed to
zwp_linux_surface_synchronization_v1? What about zwp_buffer_release_v1?
That's a good question, because these names are kind of global. Not
really global, but it could cause some name conflicts if the same
program or a compilation unit needed to use two different but
same-named interfaces. They are less global than the same of the global
interface though, which needs to be unique per platform for real.

The stable names would be wp_surface_synchronization and
wp_buffer_release, with the root interface being
wp_linux_explicit_synchronization.

The dmabuf extension relies of the Linux definition of a dmabuf. This
extension relies on the Linux definition of a fence, AFAIU.

So yes, I think repeating "linux" in the interface names would be
appropriate.
Post by Simon Ser
Post by Alexandros Frantzis
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1.get_release request.
Thanks,
pq
Daniel Stone
2018-11-08 14:12:24 UTC
Permalink
Hi,
Post by Pekka Paalanen
Post by Simon Ser
I missed this before, but: is there a reason why the "linux" prefix has been
dropped here? Maybe it should be renamed to
zwp_linux_surface_synchronization_v1? What about zwp_buffer_release_v1?
That's a good question, because these names are kind of global. Not
really global, but it could cause some name conflicts if the same
program or a compilation unit needed to use two different but
same-named interfaces. They are less global than the same of the global
interface though, which needs to be unique per platform for real.
The stable names would be wp_surface_synchronization and
wp_buffer_release, with the root interface being
wp_linux_explicit_synchronization.
The dmabuf extension relies of the Linux definition of a dmabuf. This
extension relies on the Linux definition of a fence, AFAIU.
There's precedence in Vulkan's external-sharing interfaces: one of the
semaphore import/export types is an 'opaque FD' which has
implementation-defined meaning. On the other hand, when this came up
last, NVIDIA said that it was only intended for reusable semaphores
whose lifetime roughly matched swapchain images. Apparently on their
driver stack, doing per-frame import of syncpoint-style fences has
crippling performance implications, so given that this extension's
semantics are so clearly tied to sync_file, it wouldn't be a good
match.

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

Cheers,
Daniel
Alexandros Frantzis
2018-11-08 14:34:52 UTC
Permalink
Post by Pekka Paalanen
On Wed, 07 Nov 2018 20:22:38 +0000
Post by Simon Ser
Thanks! With these fixes, this is now
Below is a small comment, I don't know if it's relevant or not, I just wanted to
point it out.
...
Post by Pekka Paalanen
Post by Simon Ser
Post by Alexandros Frantzis
+ <interface name="zwp_surface_synchronization_v1" version="1">
I missed this before, but: is there a reason why the "linux" prefix has been
dropped here? Maybe it should be renamed to
zwp_linux_surface_synchronization_v1? What about zwp_buffer_release_v1?
That's a good question, because these names are kind of global. Not
really global, but it could cause some name conflicts if the same
program or a compilation unit needed to use two different but
same-named interfaces. They are less global than the same of the global
interface though, which needs to be unique per platform for real.
The stable names would be wp_surface_synchronization and
wp_buffer_release, with the root interface being
wp_linux_explicit_synchronization.
The dmabuf extension relies of the Linux definition of a dmabuf. This
extension relies on the Linux definition of a fence, AFAIU.
So yes, I think repeating "linux" in the interface names would be
appropriate.
Hi Pekka and Simon,

adding the linux_ prefix is reasonable (and means we could be winning at
the longest interface name competition :)).

This got me thinking (also see Daniel's email), though, if there would
be benefit in moving in the other direction: making this protocol
agnostic of the fence type details. So, instead of
"zwp_linux_explicit_synchronization_v1" we will have
"zwp_explicit_synchronization_v1" with the passed fd being an opaque fd
from a protocol perspective.

Of course, this reintroduces the problem of how the client can figure
out what kind of buffer/fence combinations the server can accept, which
we have resolved here by limiting this protocol to dmabuf/dma_fences.
One way would be to allow per-platform protocols that just advertise
supported combinations, e.g., having
"zwp_linux_explicit_synchronization_v1" just means that the
dmabuf/dmafence combo is supported for zwp_explicit_synchronization
operations.

Finally, if we think an opaque fd may be limiting, there are other
options to explore (e.g., a zwp_fence interface with factories in
per-platform extensions).

Thoughts?

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

I'd keep it simple. Leave out other fence types until we have a use
case. It's still an unstable protocol, and we can see if other types
appear by the time people want to declare it stable.

An opaque fd is kind of useless. If the spec does not say what it is,
how could a compositor or a client do anything with it?

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

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

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

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

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

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


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

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

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

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

Even if this is overkill for now, I think it's a viable design to keep
in mind if in the future we need to support multiple fence types.
Post by Pekka Paalanen
For simplicity, I'd go with "linux" in the name and the explicit
connection with linux_dmabuf buffers.
Fine with me.

Thanks,
Alexandros
Pekka Paalanen
2018-11-09 10:18:44 UTC
Permalink
On Thu, 8 Nov 2018 18:37:50 +0200
Post by Alexandros Frantzis
Post by Pekka Paalanen
On Thu, 8 Nov 2018 16:34:52 +0200
Post by Alexandros Frantzis
Post by Pekka Paalanen
On Wed, 07 Nov 2018 20:22:38 +0000
Post by Simon Ser
Thanks! With these fixes, this is now
Below is a small comment, I don't know if it's relevant or not, I just wanted to
point it out.
...
Post by Pekka Paalanen
Post by Simon Ser
Post by Alexandros Frantzis
+ <interface name="zwp_surface_synchronization_v1" version="1">
I missed this before, but: is there a reason why the "linux" prefix has been
dropped here? Maybe it should be renamed to
zwp_linux_surface_synchronization_v1? What about zwp_buffer_release_v1?
That's a good question, because these names are kind of global. Not
really global, but it could cause some name conflicts if the same
program or a compilation unit needed to use two different but
same-named interfaces. They are less global than the same of the global
interface though, which needs to be unique per platform for real.
The stable names would be wp_surface_synchronization and
wp_buffer_release, with the root interface being
wp_linux_explicit_synchronization.
The dmabuf extension relies of the Linux definition of a dmabuf. This
extension relies on the Linux definition of a fence, AFAIU.
So yes, I think repeating "linux" in the interface names would be
appropriate.
Hi Pekka and Simon,
adding the linux_ prefix is reasonable (and means we could be winning at
the longest interface name competition :)).
This got me thinking (also see Daniel's email), though, if there would
be benefit in moving in the other direction: making this protocol
agnostic of the fence type details. So, instead of
"zwp_linux_explicit_synchronization_v1" we will have
"zwp_explicit_synchronization_v1" with the passed fd being an opaque fd
from a protocol perspective.
Of course, this reintroduces the problem of how the client can figure
out what kind of buffer/fence combinations the server can accept, which
we have resolved here by limiting this protocol to dmabuf/dma_fences.
One way would be to allow per-platform protocols that just advertise
supported combinations, e.g., having
"zwp_linux_explicit_synchronization_v1" just means that the
dmabuf/dmafence combo is supported for zwp_explicit_synchronization
operations.
Finally, if we think an opaque fd may be limiting, there are other
options to explore (e.g., a zwp_fence interface with factories in
per-platform extensions).
Hi Alf,
I'd keep it simple. Leave out other fence types until we have a use
case. It's still an unstable protocol, and we can see if other types
appear by the time people want to declare it stable.
An opaque fd is kind of useless. If the spec does not say what it is,
how could a compositor or a client do anything with it?
...
The idea is to have something akin to how the EGL_KHR_image_base and
EGL_KHR_image_* extensions are structured.
So, the base protocol, wp_explicit_synchronization, would just define
the operations but not any acceptable fence types. Then we could have
platform-specific protocols like
wp_{linux,dmafence,syncobj...}_explicit_synchronization, which would
just state what the acceptable fence types are, and possibly which
buffer types these fences can be associated with. Their presence would
let the client know how to create the fence. In order to have a usable
implementation, the server will need to expose at least one of these
platform-specific protocols.
As for the server detecting the fence fd type internally, in case
multiple fence types are supported, we can express this in terms of
checking for fence validity, which we already need in order to support
the INVALID_FENCE error.
Even if this is overkill for now, I think it's a viable design to keep
in mind if in the future we need to support multiple fence types.
Hi Alf,

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

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

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

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

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

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

Until then, let's not get ahead of ourselves.
Post by Alexandros Frantzis
Post by Pekka Paalanen
For simplicity, I'd go with "linux" in the name and the explicit
connection with linux_dmabuf buffers.
Fine with me.
Thanks,
pq
Pekka Paalanen
2018-11-08 15:45:59 UTC
Permalink
On Wed, 7 Nov 2018 16:59:25 +0200
Post by Alexandros Frantzis
---
- Fixed wl_buffer.attach -> wl_surface.attach typos.
- Added NO_BUFFER error and updated request descriptions.
- Consistently used "error is raised" phrasing.
- Added comment about buffer_release object destruction in event
descriptions.
Hi Alf,

looks good!

Now just to finish the bikeshed with the interface names.


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

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 <***@chromium.org>
+Daniel Stone <***@collabora.com>
+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>
--
2.19.1
Simon Ser
2018-11-09 08:14:00 UTC
Permalink
Reviewed-by: Simon Ser <***@emersion.fr>

Thanks!
Pekka Paalanen
2018-11-09 10:48:09 UTC
Permalink
On Fri, 9 Nov 2018 09:46:36 +0200
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 <***@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
Post by Alexandros Frantzis
---
- Added linux_ prefix to interface names.
- 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.
- 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.
- Guaranteed protocol compatibility only with zwp_linux_dmabuf buffers.
- Added the UNSUPPORTED_BUFFER error.
- 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.
- 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
+
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
+
+ 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
2018-11-09 14:24:56 UTC
Permalink
Post by Pekka Paalanen
On Fri, 9 Nov 2018 09:46:36 +0200
Hi Alf,
Hi Pekka,

thanks for the review!
Post by Pekka Paalanen
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).
Post by Pekka Paalanen
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
Alexandros Frantzis
2018-11-12 11:19:36 UTC
Permalink
Post by Alexandros Frantzis
Post by Pekka Paalanen
On Fri, 9 Nov 2018 09:46:36 +0200
Hi Alf,
Hi Pekka,
thanks for the review!
Post by Pekka Paalanen
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.
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
Daniel Stone
2018-11-12 11:15:25 UTC
Permalink
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no other changes
coming.
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 <***@collabora.com>

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

Cheers,
Daniel
Tomek Bury
2018-11-12 12:39:58 UTC
Permalink
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
Post by Daniel Stone
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no other changes
coming.
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 Weston implementation looks pretty good so far, though there's no
full implementation of release yet.
Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Alexandros Frantzis
2018-11-13 09:08:21 UTC
Permalink
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no other changes
coming.
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 Weston implementation looks pretty good so far, though there's no
full implementation of release yet.
Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
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
2018-11-13 09:33:30 UTC
Permalink
Thanks!

On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
Post by Alexandros Frantzis
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no other changes
coming.
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 Weston implementation looks pretty good so far, though there's no
full implementation of release yet.
Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
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
Post by Tomek Bury
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
2018-11-23 13:07:37 UTC
Permalink
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
Post by Simon Ser
Thanks!
On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
Post by Tomek Bury
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no other changes
coming.
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
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
R-b, please?
The Weston implementation looks pretty good so far, though there's no
full implementation of release yet.
Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
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
Post by Tomek Bury
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
2018-11-23 13:47:29 UTC
Permalink
On Fri, 23 Nov 2018 13:07:37 +0000
Post by Simon Ser
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.
Post by Simon Ser
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
Post by Simon Ser
Post by Simon Ser
Thanks!
On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
Post by Tomek Bury
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no other changes
coming.
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
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
R-b, please?
The Weston implementation looks pretty good so far, though there's no
full implementation of release yet.
Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
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
Post by Tomek Bury
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
2018-11-23 16:26:19 UTC
Permalink
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 13:07:37 +0000
Post by Simon Ser
Hi Alexandros,
Sorry for a delay. I've finally got an end-to-end system to test it out.
It
Post by Simon Ser
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
Post by Simon Ser
"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.
Post by Simon Ser
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
Post by Simon Ser
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
Post by Simon Ser
Post by Simon Ser
Thanks!
On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
Post by Tomek Bury
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no other
changes
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
coming.
You have ensured that the C files generated from this revision
build
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
fine in Weston, right?
David, Daniel, since your name is in the maintainers, can I
have
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
your
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
R-b, please?
The Weston implementation looks pretty good so far, though
there's no
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Tomek Bury
Post by Daniel Stone
full implementation of release yet.
Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
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
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Tomek Bury
out with Broadcom driver which doesn't have implicit cross-process
sync. I
Post by Tomek Bury
can add the explicit sync protocol implementation on the driver
side but
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Tomek Bury
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,
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
including some zwp_buffer_release_v1 correctness fixes, in the
following
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
days.
Thanks,
Alexandros
[1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
Pekka Paalanen
2018-11-26 09:21:50 UTC
Permalink
On Fri, 23 Nov 2018 16:26:19 +0000
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
On Fri, 23 Nov 2018 13:07:37 +0000
Post by Simon Ser
Hi Alexandros,
Sorry for a delay. I've finally got an end-to-end system to test it out.
It
Post by Simon Ser
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
Post by Simon Ser
"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.
Post by Simon Ser
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
Post by Simon Ser
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
Post by Simon Ser
Post by Simon Ser
Thanks!
On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
Post by Tomek Bury
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no other
changes
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
coming.
You have ensured that the C files generated from this revision
build
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
fine in Weston, right?
David, Daniel, since your name is in the maintainers, can I
have
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
your
Post by Tomek Bury
Post by Daniel Stone
Post by Pekka Paalanen
R-b, please?
The Weston implementation looks pretty good so far, though
there's no
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Tomek Bury
Post by Daniel Stone
full implementation of release yet.
Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
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
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Tomek Bury
out with Broadcom driver which doesn't have implicit cross-process
sync. I
Post by Tomek Bury
can add the explicit sync protocol implementation on the driver
side but
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Tomek Bury
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,
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
including some zwp_buffer_release_v1 correctness fixes, in the
following
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
days.
Thanks,
Alexandros
[1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
Tomek Bury
2018-11-26 15:14:45 UTC
Permalink
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
Post by Pekka Paalanen
On Fri, 23 Nov 2018 16:26:19 +0000
Post by Alexandros Frantzis
Hi Pekka,
I presume you have a driver stack that relies on the opaque EGL
buffers
Post by Alexandros Frantzis
and not zwp_linux_dmabuf any time soon?
Yes, exactly. I've added a protocol extension for sharing those buffers
and
Post by Alexandros Frantzis
our eglCreateImage() implementation can import such buffers into the
driver
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
backend and/or renderer using whatever hardware or API they have at
hand. A
Post by Alexandros Frantzis
Vulkan renderer could potentially use fences with anything a Vulkan
driver
Post by Alexandros Frantzis
is capable of importing. A renderer that does the CPU wait could be
useful
Post by Alexandros Frantzis
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.
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
Post by Alexandros Frantzis
On Fri, 23 Nov 2018 13:07:37 +0000
Post by Simon Ser
Hi Alexandros,
Sorry for a delay. I've finally got an end-to-end system to test it
out.
Post by Alexandros Frantzis
It
Post by Simon Ser
took some time because Weston backend I wrote a while back needed
serious
Post by Alexandros Frantzis
Post by Simon Ser
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
Post by Alexandros Frantzis
Post by Simon Ser
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
Post by Alexandros Frantzis
Post by Simon Ser
implementation exposes EGL_SYNC_NATIVE_FENCE_ANDROID extension this
should
Post by Simon Ser
"just work" (tm) for the GL renderer. It certainly did for me.
CPU-based
Post by Alexandros Frantzis
Post by Simon Ser
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?
Post by Alexandros Frantzis
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.
Post by Simon Ser
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
Post by Alexandros Frantzis
Post by Simon Ser
else the renderer needs to know how to get pixels and use whatever
means
Post by Alexandros Frantzis
to
Post by Simon Ser
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
Post by Simon Ser
Post by Simon Ser
Thanks!
On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
Post by Tomek Bury
On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <
On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no
other
Post by Alexandros Frantzis
changes
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Pekka Paalanen
coming.
You have ensured that the C files generated from this
revision
Post by Alexandros Frantzis
build
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Pekka Paalanen
fine in Weston, right?
David, Daniel, since your name is in the maintainers, can
I
Post by Alexandros Frantzis
have
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
your
Post by Pekka Paalanen
R-b, please?
The Weston implementation looks pretty good so far, though
there's no
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
full implementation of release yet.
Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
HI Daniel,
Where can I find the work-in-progress implementation? I'd like
to
Post by Alexandros Frantzis
try it
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
out with Broadcom driver which doesn't have implicit
cross-process
Post by Alexandros Frantzis
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
sync. I
can add the explicit sync protocol implementation on the
driver
Post by Alexandros Frantzis
side but
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
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,
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
including some zwp_buffer_release_v1 correctness fixes, in the
following
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
days.
Thanks,
Alexandros
[1]
https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
Pekka Paalanen
2018-11-27 08:53:45 UTC
Permalink
On Mon, 26 Nov 2018 15:14:45 +0000
Post by Alexandros Frantzis
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.
Post by Alexandros Frantzis
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.
Post by Alexandros Frantzis
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.
Post by Alexandros Frantzis
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.
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
Post by Pekka Paalanen
On Fri, 23 Nov 2018 16:26:19 +0000
Post by Alexandros Frantzis
Hi Pekka,
I presume you have a driver stack that relies on the opaque EGL
buffers
Post by Alexandros Frantzis
and not zwp_linux_dmabuf any time soon?
Yes, exactly. I've added a protocol extension for sharing those buffers
and
Post by Alexandros Frantzis
our eglCreateImage() implementation can import such buffers into the
driver
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
backend and/or renderer using whatever hardware or API they have at
hand. A
Post by Alexandros Frantzis
Vulkan renderer could potentially use fences with anything a Vulkan
driver
Post by Alexandros Frantzis
is capable of importing. A renderer that does the CPU wait could be
useful
Post by Alexandros Frantzis
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.
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
Post by Alexandros Frantzis
On Fri, 23 Nov 2018 13:07:37 +0000
Post by Simon Ser
Hi Alexandros,
Sorry for a delay. I've finally got an end-to-end system to test it
out.
Post by Alexandros Frantzis
It
Post by Simon Ser
took some time because Weston backend I wrote a while back needed
serious
Post by Alexandros Frantzis
Post by Simon Ser
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
Post by Alexandros Frantzis
Post by Simon Ser
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
Post by Alexandros Frantzis
Post by Simon Ser
implementation exposes EGL_SYNC_NATIVE_FENCE_ANDROID extension this
should
Post by Simon Ser
"just work" (tm) for the GL renderer. It certainly did for me.
CPU-based
Post by Alexandros Frantzis
Post by Simon Ser
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?
Post by Alexandros Frantzis
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.
Post by Simon Ser
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
Post by Alexandros Frantzis
Post by Simon Ser
else the renderer needs to know how to get pixels and use whatever
means
Post by Alexandros Frantzis
to
Post by Simon Ser
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
Post by Simon Ser
Post by Simon Ser
Thanks!
On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
Post by Tomek Bury
On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <
On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no
other
Post by Alexandros Frantzis
changes
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Pekka Paalanen
coming.
You have ensured that the C files generated from this
revision
Post by Alexandros Frantzis
build
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Pekka Paalanen
fine in Weston, right?
David, Daniel, since your name is in the maintainers, can
I
Post by Alexandros Frantzis
have
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
your
Post by Pekka Paalanen
R-b, please?
The Weston implementation looks pretty good so far, though
there's no
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
full implementation of release yet.
Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
HI Daniel,
Where can I find the work-in-progress implementation? I'd like
to
Post by Alexandros Frantzis
try it
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
out with Broadcom driver which doesn't have implicit
cross-process
Post by Alexandros Frantzis
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
sync. I
can add the explicit sync protocol implementation on the
driver
Post by Alexandros Frantzis
side but
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
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,
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
including some zwp_buffer_release_v1 correctness fixes, in the
following
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
days.
Thanks,
Alexandros
[1]
https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
Tomek Bury
2018-11-27 10:56:39 UTC
Permalink
Hi Pekka,
Post by Pekka Paalanen
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.
Post by Pekka Paalanen
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
Post by Pekka Paalanen
On Mon, 26 Nov 2018 15:14:45 +0000
Post by Alexandros Frantzis
Hi Pekka,
Yes, sorry, I was writing specifically about Weston implementation. In
the
Post by Alexandros Frantzis
merge request from Alexandros the actual compatibility check is in the
main
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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.
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
EGL implementation. Let's say a client-side driver uses dma-buf buffers
for
Post by Alexandros Frantzis
its swapchain images. If the compositor knows how to handle dma-buf, it
can
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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.
Post by Alexandros Frantzis
If the EGL implementation can use the type of the wl_buffer and can
import
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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.
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
startup, and the implementation of that API in the 3D driver adds a
custom
Post by Alexandros Frantzis
Wayland protocol extension for sharing buffers. Now the scene is set.
When
Post by Alexandros Frantzis
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.
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
charge, the app manages the window swap chain buffers and
synchronization,
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
Post by Pekka Paalanen
On Fri, 23 Nov 2018 16:26:19 +0000
Post by Alexandros Frantzis
Hi Pekka,
I presume you have a driver stack that relies on the opaque EGL
buffers
Post by Alexandros Frantzis
and not zwp_linux_dmabuf any time soon?
Yes, exactly. I've added a protocol extension for sharing those
buffers
Post by Alexandros Frantzis
Post by Pekka Paalanen
and
Post by Alexandros Frantzis
our eglCreateImage() implementation can import such buffers into
the
Post by Alexandros Frantzis
Post by Pekka Paalanen
driver
Post by Alexandros Frantzis
on the compositor end. The buffers are carried by an fd to the
compositor
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
that's the only similarity. They're not dma-buf.
Yeah, support for opaque EGL buffers could be added, just need to
think
Post by Alexandros Frantzis
of a good wording, since acquire fences do not make sense for all
buffer
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
types.
Isn't that renderer's and/or backend's decision? The GL renderer
can
Post by Alexandros Frantzis
Post by Pekka Paalanen
accept
Post by Alexandros Frantzis
fence with any buffer it can send to the 3D driver, so, effectively,
anything backed by available EGL image extensions. Someone may add
a
Post by Alexandros Frantzis
Post by Pekka Paalanen
custom
Post by Alexandros Frantzis
backend and/or renderer using whatever hardware or API they have at
hand. A
Post by Alexandros Frantzis
Vulkan renderer could potentially use fences with anything a Vulkan
driver
Post by Alexandros Frantzis
is capable of importing. A renderer that does the CPU wait could be
useful
Post by Alexandros Frantzis
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.
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
Post by Alexandros Frantzis
On Fri, 23 Nov 2018 13:07:37 +0000
Post by Simon Ser
Hi Alexandros,
Sorry for a delay. I've finally got an end-to-end system to test
it
Post by Alexandros Frantzis
Post by Pekka Paalanen
out.
Post by Alexandros Frantzis
It
Post by Simon Ser
took some time because Weston backend I wrote a while back
needed
Post by Alexandros Frantzis
Post by Pekka Paalanen
serious
Post by Alexandros Frantzis
Post by Simon Ser
rework to catch up with latest changes.
There's one thing that didn't work for me. In compositor you
reject
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
Post by Simon Ser
anything that isn't a DMA buffer and then in glrenderer you put
an
Post by Alexandros Frantzis
Post by Pekka Paalanen
extra
Post by Alexandros Frantzis
Post by Simon Ser
assertion. Why? All you do is use an EGL extension in order to
import
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
Post by Simon Ser
external fence_fd. There's no dmabuf dependency there. As long
as
Post by Alexandros Frantzis
Post by Pekka Paalanen
the EGL
Post by Alexandros Frantzis
Post by Simon Ser
implementation exposes EGL_SYNC_NATIVE_FENCE_ANDROID extension
this
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
should
Post by Simon Ser
"just work" (tm) for the GL renderer. It certainly did for me.
CPU-based
Post by Alexandros Frantzis
Post by Simon Ser
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
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
Post by Pekka Paalanen
soon?
Post by Alexandros Frantzis
Would anyone ever use an acquire fence with wl_shm buffers? That
sounds
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
fundamentally wrong to me as one cannot create fences to be
signalled
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
by userspace AFAIK. Therefore buffers whose wait cannot be
pipelined to
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
the GPU or the display device do not make much sense to me.
Post by Simon Ser
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
Post by Alexandros Frantzis
Post by Simon Ser
else the renderer needs to know how to get pixels and use
whatever
Post by Alexandros Frantzis
Post by Pekka Paalanen
means
Post by Alexandros Frantzis
to
Post by Simon Ser
put those pixels on screen.
Yeah, support for opaque EGL buffers could be added, just need to
think
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
fence+buffer combinations it cannot use, which means that clients
must
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
know in advance what they cannot use.
Thanks,
pq
Post by Simon Ser
Post by Simon Ser
Thanks!
On Tue, Nov 13, 2018 at 9:08 AM Alexandros Frantzis <
Post by Tomek Bury
On Mon, Nov 12, 2018 at 11:15 AM Daniel Stone <
On Fri, 9 Nov 2018 at 10:48, Pekka Paalanen <
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no
other
Post by Alexandros Frantzis
changes
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Pekka Paalanen
coming.
Reviewed-by: Pekka Paalanen <
You have ensured that the C files generated from this
revision
Post by Alexandros Frantzis
build
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
Post by Pekka Paalanen
fine in Weston, right?
David, Daniel, since your name is in the maintainers,
can
Post by Alexandros Frantzis
Post by Pekka Paalanen
I
Post by Alexandros Frantzis
have
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
your
Post by Pekka Paalanen
R-b, please?
The Weston implementation looks pretty good so far,
though
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
there's no
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
full implementation of release yet.
Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
HI Daniel,
Where can I find the work-in-progress implementation? I'd
like
Post by Alexandros Frantzis
Post by Pekka Paalanen
to
Post by Alexandros Frantzis
try it
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
out with Broadcom driver which doesn't have implicit
cross-process
Post by Alexandros Frantzis
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
sync. I
can add the explicit sync protocol implementation on the
driver
Post by Alexandros Frantzis
side but
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
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
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
update,
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
including some zwp_buffer_release_v1 correctness fixes, in
the
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
following
Post by Simon Ser
Post by Simon Ser
Post by Tomek Bury
days.
Thanks,
Alexandros
[1]
https://gitlab.freedesktop.org/wayland/weston/merge_requests/32
Pekka Paalanen
2018-11-27 13:09:58 UTC
Permalink
On Tue, 27 Nov 2018 10:56:39 +0000
Post by Alexandros Frantzis
Hi Pekka,
Post by Pekka Paalanen
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
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.
Post by Alexandros Frantzis
Post by Pekka Paalanen
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
2018-11-27 14:57:07 UTC
Permalink
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.
Post by Pekka Paalanen
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.
Post by Pekka Paalanen
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.
Post by Pekka Paalanen
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
Post by Pekka Paalanen
On Tue, 27 Nov 2018 10:56:39 +0000
Post by Alexandros Frantzis
Hi Pekka,
Post by Pekka Paalanen
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
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,
Post by Alexandros Frantzis
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.
Post by Alexandros Frantzis
Post by Pekka Paalanen
Otherwise, a client could use e.g. EGL in a different way to get
dmabuf,
Post by Alexandros Frantzis
Post by Pekka Paalanen
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
Post by Alexandros Frantzis
window directly and the EGL driver is not involved. EGL is only used to
set
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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,
Post by Alexandros Frantzis
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
Post by Alexandros Frantzis
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
Pekka Paalanen
2018-11-28 13:32:25 UTC
Permalink
On Tue, 27 Nov 2018 14:57:07 +0000
Post by Alexandros Frantzis
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.
Post by Alexandros Frantzis
Post by Pekka Paalanen
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.
Post by Pekka Paalanen
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.
Post by Pekka Paalanen
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
Tomek Bury
2018-11-28 14:35:02 UTC
Permalink
Hi Pekka,
Post by Pekka Paalanen
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

Alexandros Frantzis
2018-11-27 15:17:40 UTC
Permalink
Post by Pekka Paalanen
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
2018-11-27 16:25:04 UTC
Permalink
Post by Alexandros Frantzis
Post by Pekka Paalanen
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
2018-11-28 14:09:03 UTC
Permalink
On Tue, 27 Nov 2018 18:25:04 +0200
Post by Alexandros Frantzis
Post by Alexandros Frantzis
Post by Pekka Paalanen
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
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.
Post by Alexandros Frantzis
"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".
Post by Alexandros Frantzis
If we don't want to commit to general opaque EGL buffer support,
"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.
Post by Alexandros Frantzis
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
Pekka Paalanen
2018-11-12 14:40:20 UTC
Permalink
On Mon, 12 Nov 2018 11:15:25 +0000
Post by Daniel Stone
Post by Pekka Paalanen
I can add that while pushing upstream, if there are no other changes
coming.
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 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
2018-10-15 14:16:59 UTC
Permalink
Post by Pekka Paalanen
Hi,
this is a good specification, all my comments are clarifications or
minor adjustments. The fundamental design looks fine to me.
Hi Pekka,

thanks for the review! My answers are inline. I have sent a updated
version (v3) of this proposal based on this discussion.
Post by Pekka Paalanen
On Tue, 9 Oct 2018 18:11:22 +0300
Post by Alexandros Frantzis
---
patch v1 here: https://patchwork.freedesktop.org/patch/177866/
- Add NO_SURFACE error for zwp_surface_synchronization_v1 requests.
- Remove restriction to destroy a zwp_surface_synchronization_v1 object
after the associated wl_surface is destroyed.
- Clarify which buffer the acquire fence is associated with.
- Clarify 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 | 222 ++++++++++++++++++
3 files changed, 229 insertions(+)
create mode 100644 unstable/linux-explicit-synchronization/README
create mode 100644 unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
<snip>
Post by Pekka Paalanen
Post by Alexandros Frantzis
+ <interface name="zwp_surface_synchronization_v1" version="1">
+ <description summary="per-surface explicit synchronization support">
+ This object implements per-surface explicit synchronization.
+
+ Explicit synchronization refers to co-ordination of pipelined
Strike "Explicit"? It's odd because this starts with "explicit
synchronization refers to..." and then ends saying this is "implicit
synchronization.
See below.
Post by Pekka Paalanen
Post by Alexandros Frantzis
+ 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.
Should there be a paragraph break here?
See below.
Post by Pekka Paalanen
Post by Alexandros Frantzis
+ 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. This is referred
+ to as 'implicit synchronization'.
+
+ By contrast, explicit synchronization takes dma_fence objects as a
+ marker of when the asynchronous operations are complete. The fence
+ provided by the client will be waited on before the compositor
+ accesses the buffer. Conversely, in place of wl_buffer.release
+ events, the Wayland server will inform the client when the buffer
+ can be destructively accessed through a zwp_buffer_release_v1
+ object.
So this guarantees that no more wl_buffer.release events can be triggered
by commits that used explicit sync?
See below.
Post by Pekka Paalanen
Post by Alexandros Frantzis
+
+ This interface cannot be instantiated multiple times for a single
+ surface, and as such should only be used by the component which
+ performs the wl_surface.attach request. Whenever control of
+ buffer attachments is released, the releasing component should
+ ensure it destroys the zwp_surface_synchronization_v1 object.
Is the last sentence necessary? It might confuse things as it can
easily be read as if zwp_surface_synchronization_v1 was a one-shot
thing.
Ack on all of the above. In v3 I have reworded a big part of this
section, taking into account the points you brought up.
Post by Pekka Paalanen
Post by Alexandros Frantzis
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy synchronization object">
+ Destroy this explicit synchronization object.
+
+ Any fence set with set_acquire_fence in this commit cycle will
+ be invalidated in the server.
This means that if zwp_surface_synchronization_v1 object is
destroyed before issuing wl_surface.commit, then the pending acquire
fence is discarded by the server, right?
Yes, reworded to make this more clear.
Post by Pekka Paalanen
Post by Alexandros Frantzis
+
+ zwp_buffer_release_v1 objects created by this object are not affected
+ by this request.
And after wl_surface.commit, destruction has no effect on the commit.
I have clarified this in the previous paragraph about set_acquire_fence.
Post by Pekka Paalanen
Post by Alexandros Frantzis
+ </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"/>
+ </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_buffer_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 fence could not be imported, an INVALID_FENCE error is signaled
+ to the client.
"... error is raised." There is no need to be able to recover from a
failed fence import, disconnection is fine, right?
I think disconnection is fine.

This is a client-side error, and the server has no reasonable way to
continue with an invalid fence, since it doesn't know if it safe to use
the committed buffer. One option would be for the server to ignore the
fence, but since the client explicitly requested set_acquire_fence, it's
a good sign that the buffer may not be ready at commit time.

<snip>
Post by Pekka Paalanen
Post by Alexandros Frantzis
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1 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
+ where relevant for the client to synchronize against before reusing
+ the buffer.
+
+ Exactly one event, either a fenced_release or an immediate_release,
+ will be emitted for each wl_surface.commit request.
+
+ This event does not replace wl_buffer.release events; servers are still
+ required to send those events.
Ok. The introduction lead me to believe otherwise, that should probably
be worded differently.
I have remove all mention of wl_buffer.release from the intro.
Post by Pekka Paalanen
Post by Alexandros Frantzis
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy buffer release synchronization object">
+ Destroy this buffer release explicit synchronization object. The object
+ may be destroyed at any time.
+ </description>
Is it inconceivable to ever add other requests in this interface?
I don't expect other requests to be added, this is meant as
an event-only interface.
Post by Pekka Paalanen
Could there ever be a benefit from destroying this object before it has
sent an event?
The main scenario is an early exit from some code using this
object, in which case it will be easier to correctly synchronize proper
destruction of any user data used by the zwp_buffer_release_v1 listener,
when having an explicit destroy request.

This isn't particular to this protocol though, it's a general
(theoretical) concern of mine with the destroy-on-event pattern. But if
this has worked well for wp_presentation_feedback, perhaps it's not a big
deal.
Post by Pekka Paalanen
Could this object ever be used as an argument to a request?
I can't think of any cases where this would be particularly useful. In
a wild scenario we could pass this as a meta-fence object, but that
would probably require a better wayland abstraction anyway.
Post by Pekka Paalanen
If not, this seems like the perfect candidate for destroy-on-event
behaviour, similar to wp_presentation_feedback. But if there is any
suspicion about extending this interface, then it's better to keep the
destroy request.
I agree that it's a good match, with only the 2nd point concerning me a
bit.

Anyway, this change is not in v3, but if previous experience has shown
that my concern is not issue, I am happy to update this in v4.
Post by Pekka Paalanen
Post by Alexandros Frantzis
+ </request>
+
+ <event name="fenced_release">
+ <description summary="release buffer with fence">
+ Sent when the compositor has finalised its usage of the associated
+ buffer, providing a dma_fence which will be signaled when all
+ operations by the compositor on that buffer have finished.
+
+ Clients must not perform any destructive operations (e.g. clearing or
+ rendering) on the buffer until that fence has passed. They may,
+ however, destroy the wl_buffer object.
"...without any visible effects" I would add. Clients may destroy a
wl_buffer at any time, but at a "wrong" time it may lead to visual
glitches. This even here says that no glitches can happen. Unless, of
course, the wl_buffer has been submitted again or elsewhere already.
Or maybe just remove the note about destroying the wl_buffer? Strictly
speaking this event alone is not sufficient, there must not be other
unreleased uses of the wl_buffer.
Removed, and updated this section a bit.
Post by Pekka Paalanen
Post by Alexandros Frantzis
+ </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, and either performed no operations using it, or has a
+ guarantee that all its operations have finished and no more external
+ effects will be observed from these operations.
Right. Should there be explicit wording that this applies to the
specific commit, and not the buffer's usage in general?
After all, this is the extension that makes it well-defined to use a
wl_buffer again (with its contents intact) before it has been released,
providing a solution to
https://gitlab.freedesktop.org/wayland/wayland/issues/46 .
I have tried to make this more explicit, both in this and the previous
section.

Thanks,
Alexandros
Pekka Paalanen
2018-10-31 09:59:34 UTC
Permalink
On Mon, 15 Oct 2018 17:16:59 +0300
Post by Alexandros Frantzis
Post by Pekka Paalanen
Hi,
this is a good specification, all my comments are clarifications or
minor adjustments. The fundamental design looks fine to me.
Hi Pekka,
thanks for the review! My answers are inline. I have sent a updated
version (v3) of this proposal based on this discussion.
Post by Pekka Paalanen
On Tue, 9 Oct 2018 18:11:22 +0300
...
Post by Alexandros Frantzis
Post by Pekka Paalanen
Post by Alexandros Frantzis
+ <interface name="zwp_buffer_release_v1" version="1">
+ <description summary="buffer release explicit synchronization">
+ This object is instantiated in response to a
+ zwp_surface_synchronization_v1 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
+ where relevant for the client to synchronize against before reusing
+ the buffer.
+
+ Exactly one event, either a fenced_release or an immediate_release,
+ will be emitted for each wl_surface.commit request.
+
+ This event does not replace wl_buffer.release events; servers are still
+ required to send those events.
Ok. The introduction lead me to believe otherwise, that should probably
be worded differently.
I have remove all mention of wl_buffer.release from the intro.
Post by Pekka Paalanen
Post by Alexandros Frantzis
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy buffer release synchronization object">
+ Destroy this buffer release explicit synchronization object. The object
+ may be destroyed at any time.
+ </description>
Is it inconceivable to ever add other requests in this interface?
I don't expect other requests to be added, this is meant as
an event-only interface.
Post by Pekka Paalanen
Could there ever be a benefit from destroying this object before it has
sent an event?
The main scenario is an early exit from some code using this
object, in which case it will be easier to correctly synchronize proper
destruction of any user data used by the zwp_buffer_release_v1 listener,
when having an explicit destroy request.
This isn't particular to this protocol though, it's a general
(theoretical) concern of mine with the destroy-on-event pattern. But if
this has worked well for wp_presentation_feedback, perhaps it's not a big
deal.
Hi Alf,

I'm not sure what you mean with that concern.

When an event destroys a protocol object, the compositor will
unconditionally destroy the wl_resource right after sending the event.
That means the server-side request listener cannot receive any messages
anymore, so any user data would be destroyed at the same time anyway.

On client side, regardless of whether there is destroy request or not,
the client will destroy the wl_proxy. The request would only let the
compositor know that the protocol object is no more. Regardless of a
destroy request, the client side will automatically ignore any events
to the destroyed wl_proxy. That ignoring is what makes client initiated
object destruction safe in the first place.

When the client destroys the wl_proxy, it can also free any user data
associated with it, because that will guarantee that the listeners
cannot be called anymore.

If we have a destructor event, and the client destroys the wl_proxy
before that event is sent, then the event will simply be ignored. Once
the compositor sends the event, then both client and compositor again
agree that the protocol object no longer exists.

wl_display has a "secret" event that tells libwayland-client when the
server has destroyed the protocol object, which makes all the above
work.


Thanks,
pq
Alexandros Frantzis
2018-10-31 11:16:42 UTC
Permalink
Post by Pekka Paalanen
On Mon, 15 Oct 2018 17:16:59 +0300
...
Post by Pekka Paalanen
Post by Alexandros Frantzis
The main scenario is an early exit from some code using this
object, in which case it will be easier to correctly synchronize proper
destruction of any user data used by the zwp_buffer_release_v1 listener,
when having an explicit destroy request.
This isn't particular to this protocol though, it's a general
(theoretical) concern of mine with the destroy-on-event pattern. But if
this has worked well for wp_presentation_feedback, perhaps it's not a big
deal.
Hi Alf,
I'm not sure what you mean with that concern.
When an event destroys a protocol object, the compositor will
unconditionally destroy the wl_resource right after sending the event.
That means the server-side request listener cannot receive any messages
anymore, so any user data would be destroyed at the same time anyway.
On client side, regardless of whether there is destroy request or not,
the client will destroy the wl_proxy. The request would only let the
compositor know that the protocol object is no more. Regardless of a
destroy request, the client side will automatically ignore any events
to the destroyed wl_proxy. That ignoring is what makes client initiated
object destruction safe in the first place.
When the client destroys the wl_proxy, it can also free any user data
associated with it, because that will guarantee that the listeners
cannot be called anymore.
If we have a destructor event, and the client destroys the wl_proxy
before that event is sent, then the event will simply be ignored. Once
the compositor sends the event, then both client and compositor again
agree that the protocol object no longer exists.
wl_display has a "secret" event that tells libwayland-client when the
server has destroyed the protocol object, which makes all the above
work.
Thanks,
pq
Hi Pekka,

thanks for the detailed explanation. I was misunderstanding how
destroy-on-event is expected to work.

I'll update the protocol to use destroy-on-event in v5.

Thanks,
Alexandros
Loading...