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? 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
