RE: [PATCH v4 1/2] drm: Add common drm_user_fence helper

[SHANMUGAM, SRINIVASAN]

AMD General

> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@xxxxxxx>
> Sent: Friday, August 28, 2026 1:48 PM
> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@xxxxxxx>;
> Matthew Brost <matthew.brost@xxxxxxxxx>
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Maarten Lankhorst
> <maarten.lankhorst@xxxxxxxxxxxxxxx>; Maxime Ripard <mripard@xxxxxxxxxx>;
> Thomas Zimmermann <tzimmermann@xxxxxxx>; David Airlie
> <airlied@xxxxxxxxx>; Simona Vetter <simona@xxxxxxxx>; Sumit Semwal
> <sumit.semwal@xxxxxxxxxx>; Thomas Hellström
> <thomas.hellstrom@xxxxxxxxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx; intel-
> xe@xxxxxxxxxxxxxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx; linaro-mm-
> sig@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v4 1/2] drm: Add common drm_user_fence helper
>
> On 8/28/26 10:06, SHANMUGAM, SRINIVASAN wrote:
> ...
> >>> +/**
> >>> + * struct drm_user_fence - embeddable DRM user fence
> >>> + *
> >>> + * Drivers embed this in their own structure and implement
> >>> + * &drm_user_fence_ops. Call drm_user_fence_init() at creation and
> >>> + * drm_user_fence_add_callback() to arm on a dma-fence.
> >>> + * Call drm_user_fence_cancel_sync() before driver teardown.
> >>> + */
> >>> +struct drm_user_fence {
> >>
> >> Should this common layer be split into two distinct concepts?
> >>
> >> - drm_work_fence: 90% of what is here, minus the kthread_use_mm() and
> >>   mm-related code.
> >> - drm_user_fence: a subclass of drm_work_fence that adds the
> >>   kthread_use_mm() and mm-related code.
> >>
> >> I suggest this because I was thinking about it the other day (I
> >> forget the exact
> >> context) and reconsidered a pattern where a fence signals and then I
> >> need a worker because some work must be done outside of IRQ context.
> >> A user fence is one example, since copy_to_user() can fault, which is
> >> not allowed in IRQ context. At various times in Xe we've had multiple
> >> patterns like this, although at the moment user fences are probably
> >> the only case that requires it. If we looked across DRM as a whole, I suspect
> we'd find this pattern open-coded in a number of places.
> >>
> >> Yes, drm_user_fence would be a very thin layer on top of
> >> drm_work_fence, but I still see value in the split.
> >
> > Hi Matt,
> >
> > Thanks for the review and for being supportive of the idea.
> >
> > The split into drm_work_fence (general fence-to-workqueue pattern) and
> > drm_user_fence (subclass adding kthread_use_mm) makes sense. I'll
> > restructure v5 as follows:
> >
> >   drm_work_fence: kref, work_struct, dma_fence_cb, stored fence ref,
> >                   wq, ops — add_callback, cancel, cancel_sync
>
> Yeah, this pattern came up so often that I already considered adding it to the core
> dma_fence framework.

Hi Christian,

Thanks for the feedback.

On dma_fence_work: would you prefer I place the generic fence-to-work
helper directly in the core dma_fence framework (drivers/dma-buf/),
or is starting with drm_work_fence in DRM and promoting it later also
acceptable?

I'll add the value comparison logic and will add a clear note that this cannot be
used to implement dma_fence_ops.

Thanks,
Srini