All of lore.kernel.org
 help / color / mirror / Atom feed
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
> 

  parent reply	other threads:[~2026-09-08  4:53 UTC|newest]

Thread overview: 38+ 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  5:41 ` [PATCH v5 4/4] drm: Add per-signal compare functionality " Srinivasan Shanmugam
2026-08-31  5:51   ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.