Re: [PATCH v2] drm/syncobj: add IOCTL to register an eventfd
On Wed, 12 Oct 2022 12:32:53 +
Simon Ser wrote:
> Introduce a new DRM_IOCTL_SYNCOBJ_EVENTFD IOCTL which signals an
> eventfd from a syncobj.
>
> This is useful for Wayland compositors to handle wait-before-submit.
> Wayland clients can send a timeline point to the compositor
> before the point has materialized yet, then compositors can wait
> for the point to materialize via this new IOCTL.
>
> The existing DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT IOCTL is not suitable
> because it blocks. Compositors want to integrate the wait with
> their poll(2)-based event loop.
>
> v2:
> - Wait for fence when flags is zero
> - Improve documentation (Pekka)
> - Rename IOCTL (Christian)
> - Fix typo in drm_syncobj_add_eventfd() (Christian)
>
> Signed-off-by: Simon Ser
> Cc: Jason Ekstrand
> Cc: Daniel Vetter
> Cc: Christian König
> Cc: Bas Nieuwenhuizen
> Cc: Daniel Stone
> Cc: Pekka Paalanen
> Cc: James Jones
> ---
>
> Toy user-space available at:
> https://paste.sr.ht/~emersion/674bd4fc614570f262b5bb2213be5c77d68944ac
>
> drivers/gpu/drm/drm_internal.h | 2 +
> drivers/gpu/drm/drm_ioctl.c| 2 +
> drivers/gpu/drm/drm_syncobj.c | 143 +++--
> include/drm/drm_syncobj.h | 6 +-
> include/uapi/drm/drm.h | 22 +
> 5 files changed, 168 insertions(+), 7 deletions(-)
>
...
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 642808520d92..5ac0a48b0169 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -909,6 +909,27 @@ struct drm_syncobj_timeline_wait {
> __u32 pad;
> };
>
> +/**
> + * struct drm_syncobj_eventfd
> + * @handle: syncobj handle.
> + * @flags: Zero to wait for the point to be signalled, or
> + * _SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE to wait for a fence to be
> + * available for the point.
> + * @point: syncobj timeline point (set to zero for binary syncobjs).
> + * @fd: Existing eventfd to sent events to.
> + * @pad: Must be zero.
> + *
> + * Register an eventfd to be signalled by a syncobj. The eventfd counter will
> + * be incremented by one.
> + */
> +struct drm_syncobj_eventfd {
> + __u32 handle;
> + __u32 flags;
> + __u64 point;
> + __s32 fd;
> + __u32 pad;
> +};
> +
>
> struct drm_syncobj_array {
> __u64 handles;
> @@ -1095,6 +1116,7 @@ extern "C" {
> #define DRM_IOCTL_SYNCOBJ_QUERY DRM_IOWR(0xCB, struct
> drm_syncobj_timeline_array)
> #define DRM_IOCTL_SYNCOBJ_TRANSFER DRM_IOWR(0xCC, struct
> drm_syncobj_transfer)
> #define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNALDRM_IOWR(0xCD, struct
> drm_syncobj_timeline_array)
> +#define DRM_IOCTL_SYNCOBJ_EVENTFDDRM_IOWR(0xCE, struct
> drm_syncobj_eventfd)
>
> /**
> * DRM_IOCTL_MODE_GETFB2 - Get framebuffer metadata.
Hi,
UAPI
Acked-by: Pekka Paalanen
with the disclaimer that I know nothing about syncobjs. The doc sounds
good though, and I read through eventfd man page to see that the
increment makes sense.
I think there is also a way initialize eventfd with 0x - N
where N is the number of syncobjs you want to wait, and then poll for
readable. Once the Nth syncobj fires, the eventfd overflows and polls
readable (and produces POLLERR). Maybe that's abuse, and maybe you
usually have no need to poll for all of a set of syncobjs, so fine.
I suspect the Wayland compositor use case wants to know about each
syncobj individually, so either you have one eventfd per syncobj or
poll for "any" in an eventfd with multiple syncobjs and check them all
each time it polls readable.
Seems fine to me.
Thanks,
pq
pgp05kHCKXQ4j.pgp
Description: OpenPGP digital signature
Re: [PATCH v2] drm/syncobj: add IOCTL to register an eventfd
Am 12.10.22 um 14:32 schrieb Simon Ser:
Introduce a new DRM_IOCTL_SYNCOBJ_EVENTFD IOCTL which signals an
eventfd from a syncobj.
This is useful for Wayland compositors to handle wait-before-submit.
Wayland clients can send a timeline point to the compositor
before the point has materialized yet, then compositors can wait
for the point to materialize via this new IOCTL.
The existing DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT IOCTL is not suitable
because it blocks. Compositors want to integrate the wait with
their poll(2)-based event loop.
v2:
- Wait for fence when flags is zero
- Improve documentation (Pekka)
- Rename IOCTL (Christian)
- Fix typo in drm_syncobj_add_eventfd() (Christian)
Signed-off-by: Simon Ser
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Christian König
Cc: Bas Nieuwenhuizen
Cc: Daniel Stone
Cc: Pekka Paalanen
Cc: James Jones
Reviewed-by: Christian König
---
Toy user-space available at:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpaste.sr.ht%2F~emersion%2F674bd4fc614570f262b5bb2213be5c77d68944acdata=05%7C01%7Cchristian.koenig%40amd.com%7C5cc939311a00477ef8eb08daac4de7ea%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638011747896643399%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=COiWGlyLQiqvVJlMaz2BcGLXqVGktLuJTl7CC7jpfDU%3Dreserved=0
drivers/gpu/drm/drm_internal.h | 2 +
drivers/gpu/drm/drm_ioctl.c| 2 +
drivers/gpu/drm/drm_syncobj.c | 143 +++--
include/drm/drm_syncobj.h | 6 +-
include/uapi/drm/drm.h | 22 +
5 files changed, 168 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 1fbbc19f1ac0..ca320e155b69 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -242,6 +242,8 @@ int drm_syncobj_wait_ioctl(struct drm_device *dev, void
*data,
struct drm_file *file_private);
int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_eventfd_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index ca2a6e6101dc..95ec9a02f8a7 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -702,6 +702,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
drm_syncobj_timeline_wait_ioctl,
DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_EVENTFD, drm_syncobj_eventfd_ioctl,
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 0c2be8360525..659577ad8c07 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -185,6 +185,7 @@
#include
#include
+#include
#include
#include
#include
@@ -212,6 +213,20 @@ struct syncobj_wait_entry {
static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj,
struct syncobj_wait_entry *wait);
+struct syncobj_eventfd_entry {
+ struct list_head node;
+ struct dma_fence *fence;
+ struct dma_fence_cb fence_cb;
+ struct drm_syncobj *syncobj;
+ struct eventfd_ctx *ev_fd_ctx;
+ u64 point;
+ u32 flags;
+};
+
+static void
+syncobj_eventfd_entry_func(struct drm_syncobj *syncobj,
+ struct syncobj_eventfd_entry *entry);
+
/**
* drm_syncobj_find - lookup and reference a sync object.
* @file_private: drm file private pointer
@@ -274,6 +289,27 @@ static void drm_syncobj_remove_wait(struct drm_syncobj
*syncobj,
spin_unlock(>lock);
}
+static void
+syncobj_eventfd_entry_free(struct syncobj_eventfd_entry *entry)
+{
+ eventfd_ctx_put(entry->ev_fd_ctx);
+ dma_fence_put(entry->fence);
+ /* This happens either inside the syncobj lock, or after the node has
+* already been removed from the list */
+ list_del(>node);
+ kfree(entry);
+}
+
+static void
+drm_syncobj_add_eventfd(struct drm_syncobj *syncobj,
+ struct syncobj_eventfd_entry *entry)
+{
+ spin_lock(>lock);
+ list_add_tail(>node, >ev_fd_list);
+ syncobj_eventfd_entry_func(syncobj, entry);
+ spin_unlock(>lock);
+}
+
/**
* drm_syncobj_add_point - add new timeline point to the syncobj
*
[PATCH v2] drm/syncobj: add IOCTL to register an eventfd
Introduce a new DRM_IOCTL_SYNCOBJ_EVENTFD IOCTL which signals an
eventfd from a syncobj.
This is useful for Wayland compositors to handle wait-before-submit.
Wayland clients can send a timeline point to the compositor
before the point has materialized yet, then compositors can wait
for the point to materialize via this new IOCTL.
The existing DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT IOCTL is not suitable
because it blocks. Compositors want to integrate the wait with
their poll(2)-based event loop.
v2:
- Wait for fence when flags is zero
- Improve documentation (Pekka)
- Rename IOCTL (Christian)
- Fix typo in drm_syncobj_add_eventfd() (Christian)
Signed-off-by: Simon Ser
Cc: Jason Ekstrand
Cc: Daniel Vetter
Cc: Christian König
Cc: Bas Nieuwenhuizen
Cc: Daniel Stone
Cc: Pekka Paalanen
Cc: James Jones
---
Toy user-space available at:
https://paste.sr.ht/~emersion/674bd4fc614570f262b5bb2213be5c77d68944ac
drivers/gpu/drm/drm_internal.h | 2 +
drivers/gpu/drm/drm_ioctl.c| 2 +
drivers/gpu/drm/drm_syncobj.c | 143 +++--
include/drm/drm_syncobj.h | 6 +-
include/uapi/drm/drm.h | 22 +
5 files changed, 168 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 1fbbc19f1ac0..ca320e155b69 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -242,6 +242,8 @@ int drm_syncobj_wait_ioctl(struct drm_device *dev, void
*data,
struct drm_file *file_private);
int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_eventfd_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index ca2a6e6101dc..95ec9a02f8a7 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -702,6 +702,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
drm_syncobj_timeline_wait_ioctl,
DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_EVENTFD, drm_syncobj_eventfd_ioctl,
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 0c2be8360525..659577ad8c07 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -185,6 +185,7 @@
#include
#include
+#include
#include
#include
#include
@@ -212,6 +213,20 @@ struct syncobj_wait_entry {
static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj,
struct syncobj_wait_entry *wait);
+struct syncobj_eventfd_entry {
+ struct list_head node;
+ struct dma_fence *fence;
+ struct dma_fence_cb fence_cb;
+ struct drm_syncobj *syncobj;
+ struct eventfd_ctx *ev_fd_ctx;
+ u64 point;
+ u32 flags;
+};
+
+static void
+syncobj_eventfd_entry_func(struct drm_syncobj *syncobj,
+ struct syncobj_eventfd_entry *entry);
+
/**
* drm_syncobj_find - lookup and reference a sync object.
* @file_private: drm file private pointer
@@ -274,6 +289,27 @@ static void drm_syncobj_remove_wait(struct drm_syncobj
*syncobj,
spin_unlock(>lock);
}
+static void
+syncobj_eventfd_entry_free(struct syncobj_eventfd_entry *entry)
+{
+ eventfd_ctx_put(entry->ev_fd_ctx);
+ dma_fence_put(entry->fence);
+ /* This happens either inside the syncobj lock, or after the node has
+* already been removed from the list */
+ list_del(>node);
+ kfree(entry);
+}
+
+static void
+drm_syncobj_add_eventfd(struct drm_syncobj *syncobj,
+ struct syncobj_eventfd_entry *entry)
+{
+ spin_lock(>lock);
+ list_add_tail(>node, >ev_fd_list);
+ syncobj_eventfd_entry_func(syncobj, entry);
+ spin_unlock(>lock);
+}
+
/**
* drm_syncobj_add_point - add new timeline point to the syncobj
* @syncobj: sync object to add timeline point do
@@ -288,7 +324,8 @@ void drm_syncobj_add_point(struct drm_syncobj *syncobj,
struct dma_fence *fence,
uint64_t point)
{
- struct syncobj_wait_entry *cur, *tmp;
+ struct syncobj_wait_entry *wait_cur, *wait_tmp;
+ struct syncobj_eventfd_entry *ev_fd_cur, *ev_fd_tmp;
struct dma_fence *prev;
dma_fence_get(fence);
@@ -302,8 +339,10 @@ void
