AMD General
> -----Original Message-----
> From: Koenig, Christian <[email protected]>
> Sent: Friday, August 28, 2026 2:43 PM
> To: SHANMUGAM, SRINIVASAN <[email protected]>;
> Matthew Brost <[email protected]>
> Cc: Deucher, Alexander <[email protected]>; Maarten Lankhorst
> <[email protected]>; Maxime Ripard <[email protected]>;
> Thomas Zimmermann <[email protected]>; David Airlie
> <[email protected]>; Simona Vetter <[email protected]>; Sumit Semwal
> <[email protected]>; Thomas Hellström
> <[email protected]>; [email protected]; intel-
> [email protected]; [email protected]; linaro-mm-
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v4 1/2] drm: Add common drm_user_fence helper
>
> On 8/28/26 10:31, SHANMUGAM, SRINIVASAN wrote:
> > AMD General
> >
> >> -----Original Message-----
> >> From: Koenig, Christian <[email protected]>
> >> Sent: Friday, August 28, 2026 1:48 PM
> >> To: SHANMUGAM, SRINIVASAN <[email protected]>;
> Matthew
> >> Brost <[email protected]>
> >> Cc: Deucher, Alexander <[email protected]>; Maarten Lankhorst
> >> <[email protected]>; Maxime Ripard
> >> <[email protected]>; Thomas Zimmermann <[email protected]>; David
> >> Airlie <[email protected]>; Simona Vetter <[email protected]>; Sumit
> >> Semwal <[email protected]>; Thomas Hellström
> >> <[email protected]>; [email protected];
> >> intel- [email protected]; [email protected];
> >> linaro-mm- [email protected]; [email protected];
> >> [email protected]
> >> 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?
>
> Maybe ask AI to search for use cases. If you find something outside of
> drivers/gpu/drm then please place it under drivers/dma-buf.
>
> If you don't find any existing use case drivers/gpu/drm should do as well.
Hi Christian,
Searched the tree for files combining dma_fence_cb + work_struct
outside of drivers/gpu/drm/:
$ grep -rl "dma_fence_cb" . --include="*.c" | \
grep -v "drivers/gpu/drm" | \
xargs grep -l "work_struct" 2>/dev/null
(no output)
No matches found outside drivers/gpu/drm/. Files using
dma_fence_add_callback outside DRM are the core framework itself
(dma-fence.c, dma-fence-chain.c, dma-buf.c) — not consumers of the
fence-to-work pattern.
Based on your suggestions, I'll keep the helper as drm_work_fence in
drivers/gpu/drm/. I'll add the value comparison logic and the
dma_fence_ops warning in the next version.
Thanks,
Srini
