From: Matthew Brost <matthew.brost@intel.com>
To: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Cc: dri-devel@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
amd-gfx@lists.freedesktop.org,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH v7 1/4] drm: Add drm_work_fence helper
Date: Mon, 7 Sep 2026 21:53:05 -0700 [thread overview]
Message-ID: <ap+UsYGDGdsGEkM9@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <20260902152049.183685-1-srinivasan.shanmugam@amd.com>
On Wed, Sep 02, 2026 at 08:50:46PM +0530, Srinivasan Shanmugam wrote:
> GPU drivers often need to queue work when a dma-fence signals
> because certain operations (copy_to_user, eventfd_signal, memory
> allocation) cannot run in IRQ context. This pattern is currently
> open-coded in multiple drivers.
>
> Introduce drm_work_fence — an embeddable base structure that handles the
> dma-fence-callback-to-workqueue pattern in one place. Drivers embed this
> in their own structure and implement ops->writeback() for the deferred
> work and ops->destroy() for cleanup.
>
> The helper manages:
> - kref lifetime
> - dma-fence callback registration
> - workqueue dispatch on fence signal
> - safe cancellation before driver teardown
>
> For work that additionally requires borrowing the process MM via
> kthread_use_mm(), see drm_user_fence which builds on top of this.
>
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/drm_work_fence.c | 184 +++++++++++++++++++++++++++++++
> include/drm/drm_work_fence.h | 69 ++++++++++++
> 3 files changed, 254 insertions(+)
> create mode 100644 drivers/gpu/drm/drm_work_fence.c
> create mode 100644 include/drm/drm_work_fence.h
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index e97faabcd783..c5be8e80d0c8 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -72,6 +72,7 @@ drm-y := \
> drm_vblank.o \
> drm_vblank_work.o \
> drm_vma_manager.o \
> + drm_work_fence.o \
> drm_writeback.o
> drm-$(CONFIG_DRM_CLIENT) += \
> drm_client.o \
> diff --git a/drivers/gpu/drm/drm_work_fence.c b/drivers/gpu/drm/drm_work_fence.c
> new file mode 100644
> index 000000000000..482eb3d4475f
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_work_fence.c
> @@ -0,0 +1,184 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 The Linux Foundation
ianal but this copywrite looks wrong for fresh code.
> + *
> + * Common DRM work fence helper.
> + *
> + * When a GPU dma-fence signals, drivers often need to perform work that
> + * cannot run in IRQ context (e.g., memory allocation, copy_to_user,
> + * eventfd_signal). This helper queues a work item when a dma-fence
> + * signals, allowing that work to run safely in a workqueue context.
> + *
> + * NOTE: This helper consumes dma_fences but CANNOT implement
> + * dma_fence_ops. Work items queued here may sleep; dma_fence_ops
> + * callbacks are called under the fence spinlock and must not sleep.
> + *
> + * For work that additionally requires accessing userspace memory via
> + * kthread_use_mm(), see drm_user_fence which builds on top of this.
> + */
> +
> +#include <linux/workqueue.h>
> +
> +#include <drm/drm_work_fence.h>
> +
> +static void drm_work_fence_destroy(struct kref *kref)
> +{
> + struct drm_work_fence *wfence =
> + container_of(kref, struct drm_work_fence, refcount);
> + struct dma_fence *fence = wfence->fence;
> +
> + wfence->ops->destroy(wfence);
> + dma_fence_put(fence); /* NULL-safe */
> +}
> +
> +/**
> + * drm_work_fence_get - Acquire a reference to a work fence
> + * @wfence: work fence
> + */
> +void drm_work_fence_get(struct drm_work_fence *wfence)
> +{
> + kref_get(&wfence->refcount);
> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_get);
EXPORT_SYMBOL I think for MIT licenced code, unless the kernel compile
complains which I don't think will be the case for anything in this
file - I could be wrong though.
> +
> +/**
> + * drm_work_fence_put - Release a reference to a work fence
> + * @wfence: work fence
> + */
> +void drm_work_fence_put(struct drm_work_fence *wfence)
> +{
> + kref_put(&wfence->refcount, drm_work_fence_destroy);
> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_put);
> +
> +static void drm_work_fence_work(struct work_struct *w)
> +{
> + struct drm_work_fence *wfence =
> + container_of(w, struct drm_work_fence, work);
> +
> + wfence->ops->writeback(wfence);
I think you inverted the naming vfuncs for drm_work_fence vs drm_user_fence.
I'd expect drm_work_fence.worker and drm_user_fence.writeback.
Matt
> + drm_work_fence_put(wfence);
> +}
> +
> +static void drm_work_fence_queue(struct drm_work_fence *wfence)
> +{
> + queue_work(wfence->wq, &wfence->work);
> +}
> +
> +static void drm_work_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
> +{
> + struct drm_work_fence *wfence =
> + container_of(cb, struct drm_work_fence, cb);
> +
> + drm_work_fence_queue(wfence);
> + /* Single ref: wfence->fence released in drm_work_fence_destroy(). */
> +}
> +
> +/**
> + * drm_work_fence_init - Initialize a work fence
> + * @wfence: work fence to initialize
> + * @wq: workqueue to run the worker on (must be ordered if sequencing matters)
> + * @ops: driver operations
> + */
> +void drm_work_fence_init(struct drm_work_fence *wfence,
> + struct workqueue_struct *wq,
> + const struct drm_work_fence_ops *ops)
> +{
> + kref_init(&wfence->refcount);
> + wfence->wq = wq;
> + wfence->ops = ops;
> + wfence->fence = NULL;
> + INIT_WORK(&wfence->work, drm_work_fence_work);
> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_init);
> +
> +/**
> + * drm_work_fence_add_callback - Attach a work fence to a dma-fence
> + * @wfence: work fence; caller retains their reference and must release
> + * it via drm_work_fence_put() when no longer needed
> + * @fence: dma-fence to watch; one reference is consumed on any return value
> + *
> + * When @fence signals, a work item is queued that calls ops->writeback().
> + * If @fence has already signaled, the work item is queued immediately.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int drm_work_fence_add_callback(struct drm_work_fence *wfence,
> + struct dma_fence *fence)
> +{
> + int err;
> +
> + drm_work_fence_get(wfence);
> + wfence->fence = fence; /* transfer caller's ref — single ref, no get */
> +
> + err = dma_fence_add_callback(fence, &wfence->cb, drm_work_fence_cb);
> + if (err == -ENOENT) {
> + drm_work_fence_queue(wfence);
> + err = 0;
> + } else if (err) {
> + wfence->fence = NULL;
> + dma_fence_put(fence);
> + drm_work_fence_put(wfence);
> + }
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_add_callback);
> +
> +/**
> + * drm_work_fence_cancel - Cancel a pending work fence callback
> + * @wfence: work fence
> + *
> + * Attempts to remove the pending callback before driver context teardown.
> + * The caller must hold a reference to @wfence across this call.
> + *
> + * If the callback has already fired this returns false and all cleanup
> + * has been handled internally.
> + *
> + * If removal succeeds the callback reference is released internally.
> + * The caller must still release its own reference via drm_work_fence_put().
> + *
> + * This function is safe to call from atomic context as it only acquires
> + * the dma-fence spinlock internally. If the caller also needs to wait
> + * for the worker to finish, use drm_work_fence_cancel_sync() instead,
> + * which may sleep.
> + *
> + * Return: true if callback was removed, false if it had already fired.
> + */
> +bool drm_work_fence_cancel(struct drm_work_fence *wfence)
> +{
> + struct dma_fence *fence = wfence->fence;
> +
> + if (!fence)
> + return false;
> +
> + if (dma_fence_remove_callback(fence, &wfence->cb)) {
> + drm_work_fence_put(wfence); /* drop ref from add_callback */
> + return true;
> + }
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_cancel);
> +
> +/**
> + * drm_work_fence_cancel_sync - Cancel callback and wait for worker to finish
> + * @wfence: work fence
> + *
> + * Calls drm_work_fence_cancel() then cancel_work_sync() to guarantee
> + * the worker has fully completed before returning.
> + *
> + * This function may sleep. Must not be called from atomic or interrupt
> + * context. Use drm_work_fence_cancel() instead when sleeping is not allowed.
> + *
> + * Drivers must call this during teardown before freeing any resources
> + * accessed by ops->writeback().
> + */
> +void drm_work_fence_cancel_sync(struct drm_work_fence *wfence)
> +{
> + if (drm_work_fence_cancel(wfence))
> + return;
> + if (cancel_work_sync(&wfence->work))
> + drm_work_fence_put(wfence);
> +}
> +EXPORT_SYMBOL_GPL(drm_work_fence_cancel_sync);
> diff --git a/include/drm/drm_work_fence.h b/include/drm/drm_work_fence.h
> new file mode 100644
> index 000000000000..c8e3c5b9f0c4
> --- /dev/null
> +++ b/include/drm/drm_work_fence.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2024 The Linux Foundation
> + */
> +
> +#ifndef __DRM_WORK_FENCE_H__
> +#define __DRM_WORK_FENCE_H__
> +
> +#include <linux/dma-fence.h>
> +#include <linux/kref.h>
> +#include <linux/workqueue.h>
> +
> +struct drm_work_fence;
> +
> +/**
> + * struct drm_work_fence_ops - driver callbacks for a DRM work fence
> + */
> +struct drm_work_fence_ops {
> + /**
> + * @writeback: Called from workqueue context when the dma-fence signals.
> + *
> + * Perform the deferred work here (copy_to_user, eventfd_signal, etc.).
> + * May sleep. Must not requeue the fence.
> + */
> + void (*writeback)(struct drm_work_fence *wfence);
> +
> + /**
> + * @destroy: Called when the last reference is dropped.
> + * Free the containing structure here.
> + */
> + void (*destroy)(struct drm_work_fence *wfence);
> +};
> +
> +/**
> + * struct drm_work_fence - DRM dma-fence-to-workqueue helper
> + *
> + * Embeddable base structure that queues a work item when a dma-fence signals.
> + * Drivers embed this in their own structure and implement ops->writeback()
> + * for the deferred work and ops->destroy() for cleanup.
> + *
> + * Call drm_work_fence_init() at creation and drm_work_fence_add_callback()
> + * to arm on a dma-fence. Call drm_work_fence_cancel_sync() before teardown.
> + */
> +struct drm_work_fence {
> + /** @refcount: Reference count. */
> + struct kref refcount;
> + /** @wq: Workqueue on which to run the worker. */
> + struct workqueue_struct *wq;
> + /** @ops: Driver operations. */
> + const struct drm_work_fence_ops *ops;
> + /** @fence: The watched dma-fence; holds a single reference. */
> + struct dma_fence *fence;
> + /** @work: Work item queued when the fence signals. */
> + struct work_struct work;
> + /** @cb: Callback registered on the dma-fence. */
> + struct dma_fence_cb cb;
> +};
> +
> +void drm_work_fence_init(struct drm_work_fence *wfence,
> + struct workqueue_struct *wq,
> + const struct drm_work_fence_ops *ops);
> +void drm_work_fence_get(struct drm_work_fence *wfence);
> +void drm_work_fence_put(struct drm_work_fence *wfence);
> +int drm_work_fence_add_callback(struct drm_work_fence *wfence,
> + struct dma_fence *fence);
> +bool drm_work_fence_cancel(struct drm_work_fence *wfence);
> +void drm_work_fence_cancel_sync(struct drm_work_fence *wfence);
> +
> +#endif /* __DRM_WORK_FENCE_H__ */
> --
> 2.34.1
>
next prev parent reply other threads:[~2026-09-08 4:53 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-27 6:21 [PATCH 0/2] drm: Add Common drm_user_fence helper and Convert XE Srinivasan Shanmugam
2026-08-27 6:21 ` [PATCH 1/2] drm: Add common drm_user_fence helper Srinivasan Shanmugam
2026-08-27 6:32 ` sashiko-bot
2026-08-27 6:21 ` [PATCH 2/2] drm/xe: Convert xe_user_fence to drm_user_fence Srinivasan Shanmugam
2026-08-27 6:37 ` sashiko-bot
2026-08-27 6:28 ` ✗ CI.checkpatch: warning for drm: Add Common drm_user_fence helper and Convert XE Patchwork
2026-08-27 6:29 ` ✗ CI.KUnit: failure " Patchwork
2026-08-31 5:41 ` [PATCH v5 0/4] drm: Add common drm_work_fence/drm_user_fence helpers and convert XE Srinivasan Shanmugam
2026-08-31 10:16 ` Thomas Hellström
2026-08-31 11:13 ` SHANMUGAM, SRINIVASAN
2026-08-31 12:22 ` Thomas Hellström
2026-08-31 12:36 ` SHANMUGAM, SRINIVASAN
2026-08-31 12:40 ` Thomas Hellström
2026-08-31 5:41 ` [PATCH v5 1/4] drm: Add drm_work_fence helper Srinivasan Shanmugam
2026-08-31 5:41 ` [PATCH v5 2/4] drm: Add drm_user_fence helper Srinivasan Shanmugam
2026-08-31 5:41 ` [PATCH v5 3/4] drm/xe: Convert xe_user_fence to drm_user_fence Srinivasan Shanmugam
2026-08-31 5:56 ` sashiko-bot
2026-08-31 13:45 ` [PATCH v6 0/4] drm: Add common drm_work_fence/drm_user_fence helpers and convert XE Srinivasan Shanmugam
2026-08-31 13:45 ` [PATCH v6 1/4] drm: Add drm_work_fence helper Srinivasan Shanmugam
2026-08-31 20:21 ` Matthew Brost
2026-09-01 7:39 ` SHANMUGAM, SRINIVASAN
2026-09-01 10:04 ` Matthew Brost
2026-09-02 15:20 ` [PATCH v7 " Srinivasan Shanmugam
2026-09-02 15:20 ` [PATCH v7 2/4] drm: Add drm_user_fence helper Srinivasan Shanmugam
2026-09-08 4:54 ` Matthew Brost
2026-09-02 15:20 ` [PATCH v7 3/4] drm/xe: Convert xe_user_fence to drm_user_fence Srinivasan Shanmugam
2026-09-02 15:34 ` sashiko-bot
2026-09-08 5:11 ` Matthew Brost
2026-09-02 15:20 ` [PATCH v7 4/4] drm: Add per-signal compare functionality " Srinivasan Shanmugam
2026-09-02 15:29 ` sashiko-bot
2026-09-08 4:53 ` Matthew Brost [this message]
2026-08-31 13:45 ` [PATCH v6 2/4] drm: Add drm_user_fence helper Srinivasan Shanmugam
2026-08-31 20:36 ` Matthew Brost
2026-08-31 13:45 ` [PATCH v6 3/4] drm/xe: Convert xe_user_fence to drm_user_fence Srinivasan Shanmugam
2026-08-31 13:45 ` [PATCH v6 4/4] drm: Add per-signal compare functionality " Srinivasan Shanmugam
2026-08-31 14:25 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ap+UsYGDGdsGEkM9@gsse-cloud1.jf.intel.com \
--to=matthew.brost@intel.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=srinivasan.shanmugam@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox