[RFC PATCH] drm/syncobj: add IOCTL to register an eventfd for a timeline
Introduce a new DRM_IOCTL_SYNCOBJ_TIMELINE_REGISTER_EVENTFD IOCTL which signals an eventfd when a timeline point completes. 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. 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 --- drivers/gpu/drm/drm_internal.h | 3 + drivers/gpu/drm/drm_ioctl.c| 3 + drivers/gpu/drm/drm_syncobj.c | 113 +++-- include/drm/drm_syncobj.h | 6 +- include/uapi/drm/drm.h | 15 + 5 files changed, 133 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 1fbbc19f1ac0..4244e929b7f9 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -242,6 +242,9 @@ 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_timeline_register_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..dcd18dba28b7 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -702,6 +702,9 @@ 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_TIMELINE_REGISTER_EVENTFD, + drm_syncobj_timeline_register_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..401d09b56cf1 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,17 @@ 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_cb fence_cb; + struct eventfd_ctx *ev_fd_ctx; + u64 point; +}; + +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 +286,25 @@ static void drm_syncobj_remove_wait(struct drm_syncobj *syncobj, spin_unlock(&syncobj->lock); } +static void +syncobj_eventfd_entry_free(struct syncobj_eventfd_entry *entry) +{ + eventfd_ctx_put(entry->ev_fd_ctx); + /* This happens inside the syncobj lock */ + list_del(&entry->node); + kfree(entry); +} + +static void +drm_syncobj_add_eventfd(struct drm_syncobj *syncobj, + struct syncobj_eventfd_entry *entry) +{ + spin_lock(&syncobj->lock); + list_add_tail(&entry->node, &syncobj->cb_list); + syncobj_eventfd_entry_func(syncobj, entry); + spin_unlock(&syncobj->lock); +} + /** * drm_syncobj_add_point - add new timeline point to the syncobj * @syncobj: sync object to add timeline point do @@ -288,7 +319,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 +334,10 @@ void drm_syncobj_add_point(struct drm_syncobj *syncobj, dma_fence_chain_init(chain, prev, fence, point); rcu_assign_pointer(syncobj->fence, &chain->base); - list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) -
Re: [RFC PATCH] drm/syncobj: add IOCTL to register an eventfd for a timeline
Am 09.10.22 um 16:40 schrieb Simon Ser: Introduce a new DRM_IOCTL_SYNCOBJ_TIMELINE_REGISTER_EVENTFD IOCTL which signals an eventfd when a timeline point completes. I was entertaining the same though for quite a while, but I would even go a step further and actually always base the wait before signal functionality of the drm_syncobj and the eventfd functionality. That would save us quite a bit of complexity I think. As a general note I think the name DRM_IOCTL_SYNCOBJ_TIMELINE_REGISTER_EVENTFD is just to long, just make that DRM_IOCTL_SYNCOBJ_EVENTFD. Same for the function names as well. Additional to that I think we should also always have a graceful handling for binary syncobjs. So please try to avoid making this special for the timeline case (the timeline case should of course still be supported). Regards, Christian. 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. 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 --- drivers/gpu/drm/drm_internal.h | 3 + drivers/gpu/drm/drm_ioctl.c| 3 + drivers/gpu/drm/drm_syncobj.c | 113 +++-- include/drm/drm_syncobj.h | 6 +- include/uapi/drm/drm.h | 15 + 5 files changed, 133 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 1fbbc19f1ac0..4244e929b7f9 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -242,6 +242,9 @@ 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_timeline_register_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..dcd18dba28b7 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -702,6 +702,9 @@ 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_TIMELINE_REGISTER_EVENTFD, + drm_syncobj_timeline_register_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..401d09b56cf1 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,17 @@ 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_cb fence_cb; + struct eventfd_ctx *ev_fd_ctx; + u64 point; +}; + +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 +286,25 @@ static void drm_syncobj_remove_wait(struct drm_syncobj *syncobj, spin_unlock(&syncobj->lock); } +static void +syncobj_eventfd_entry_free(struct syncobj_eventfd_entry *entry) +{ + eventfd_ctx_put(entry->ev_fd_ctx); + /* This happens inside the syncobj lock */ + list_del(&entry->node); + kfree(entry); +} + +static void +drm_syncobj_add_eventfd(struct drm_syncobj *syncobj, + struct syncobj_eventfd_entry *entry) +{ + spin_lock(&syncobj->lock); + list_add_tail(&entry->node, &syncobj->cb_list); + syncobj_eventfd_entry_func(syncobj, entry); + spin_unlock(&syncobj->lock); +} + /** * drm_syncobj_add_point - add