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 2:43 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:31, SHANMUGAM, SRINIVASAN wrote:
> > 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?
>
> 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