AMD General Hi Christian,
> -----Original Message----- > From: Koenig, Christian <[email protected]> > Sent: Thursday, July 16, 2026 12:13 PM > To: SHANMUGAM, SRINIVASAN <[email protected]>; > Deucher, Alexander <[email protected]> > Cc: [email protected] > Subject: Re: [PATCH v11 2/5] drm/amdgpu: Add wait-event manager and per-file > lifetime plumbing > > On 7/15/26 16:25, Srinivasan Shanmugam wrote: > > Add the per-file WAIT_EVENT manager and lifetime plumbing used to > > store and retrieve event records for render-node clients. > > > > The manager maintains a list of pending WAIT_EVENT records, supports > > blocking waits from userspace, copies the first matching event record > > to userspace, and enforces single-consumer semantics by removing > > records once they are consumed. > > > > Register the WAIT_EVENT ioctl so render-node clients can access the > > per-file WAIT_EVENT manager. > > > > For queue-scoped events, queue_id is resolved to the corresponding > > usermode queue object at the ioctl boundary. Pending records are > > matched internally using queue pointer equality, while queue_id > > remains a userspace identifier used only at the UAPI boundary and > > returned in event metadata. > > > > WAIT_EVENT now uses an absolute CLOCK_MONOTONIC deadline for timeout > > handling. The remaining timeout is recomputed after each wakeup so > > that interrupted or spurious wakeups continue to honor the original > > userspace deadline. > > > > Pending WAIT_EVENT records hold queue references while queued. Those > > references are released when records are consumed, explicitly removed, > > or destroyed during manager teardown. > > > > Embed the WAIT_EVENT manager in amdgpu_fpriv and tie its lifetime to > > drm_file. During teardown, the manager is marked dead before pending > > records are detached and blocked waiters are awakened, preventing new > > records from being queued while shutdown is in progress. > > > > The USERQ manager is torn down before the embedded WAIT_EVENT manager > > is finalized, so queues are removed from the producer lookup paths > > before the manager can go out of scope. This existing teardown > > ordering provides the lifetime guarantee for WAIT_EVENT producers > > without requiring an additional manager reference. > > > > Changes since v9: > > - Switch WAIT_EVENT timeout handling to an absolute CLOCK_MONOTONIC > > deadline, as suggested by Christian. > > - Recompute the remaining timeout after each wakeup. > > - Simplify queue reference cleanup using unconditional > > amdgpu_userq_put(). > > - Clarify that queue pointers are used internally for event matching, > > while queue_id remains a userspace-only identifier. > > - Improve kerneldoc describing manager state, event matching, and > > teardown ordering. > > - Document the existing teardown ordering that guarantees the embedded > > WAIT_EVENT manager remains valid while USERQ producers are active. > > > > Cc: Alex Deucher <[email protected]> > > Cc: Christian König <[email protected]> > > Signed-off-by: Srinivasan Shanmugam <[email protected]> > > --- > > drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- > > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 +- > > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 + > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 5 +- > > .../gpu/drm/amd/amdgpu/amdgpu_wait_event.c | 427 ++++++++++++++++++ > > .../gpu/drm/amd/amdgpu/amdgpu_wait_event.h | 101 +++++ > > 6 files changed, 538 insertions(+), 3 deletions(-) create mode > > 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_wait_event.c > > create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_wait_event.h > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile > > b/drivers/gpu/drm/amd/amdgpu/Makefile > > index b7897f98436c..c4cee6a6bc64 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/Makefile > > +++ b/drivers/gpu/drm/amd/amdgpu/Makefile > > @@ -72,7 +72,7 @@ amdgpu-y += amdgpu_device.o amdgpu_reg_access.o > amdgpu_doorbell_mgr.o amdgpu_kms > > amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o > amdgpu_lockdep.o \ > > amdgpu_ring_mux.o amdgpu_xcp.o amdgpu_seq64.o > amdgpu_dev_coredump.o \ > > amdgpu_cper.o amdgpu_userq_fence.o amdgpu_eviction_fence.o > amdgpu_ip.o \ > > - amdgpu_wb.o amdgpu_cwsr.o amdgpu_events.o amdgpu_eventfd.o > > + amdgpu_wb.o amdgpu_cwsr.o amdgpu_events.o amdgpu_eventfd.o > > +amdgpu_wait_event.o > > > > amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > index 4f42888e2647..91f1dc737aee 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > @@ -104,6 +104,7 @@ > > #include "amdgpu_fdinfo.h" > > #include "amdgpu_mca.h" > > #include "amdgpu_eventfd.h" > > +#include "amdgpu_wait_event.h" > > #include "amdgpu_ras.h" > > #include "amdgpu_lockdep.h" > > #include "amdgpu_cper.h" > > @@ -427,13 +428,15 @@ struct amdgpu_fpriv { > > uint32_t xcp_id; > > > > struct amdgpu_eventfd_mgr eventfd_mgr; > > + struct amdgpu_wait_event_mgr wait_event_mgr; > > }; > > > > struct drm_device; > > struct drm_file; > > > > int amdgpu_eventfd_ioctl(struct drm_device *dev, void *data, struct > > drm_file *file_priv); > > - > > +int amdgpu_wait_event_drm_ioctl(struct drm_device *dev, void *data, > > + struct drm_file *file_priv); > > int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv > > **fpriv); > > > > /* > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > index e90cf67c1cd8..b738a1bdf9d3 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > @@ -3097,6 +3097,7 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { > > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_LIST_HANDLES, > amdgpu_gem_list_handles_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), > > DRM_IOCTL_DEF_DRV(AMDGPU_PROC_OPTIONS, > amdgpu_proc_options_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), > > DRM_IOCTL_DEF_DRV(AMDGPU_EVENTFD, amdgpu_eventfd_ioctl, > > DRM_RENDER_ALLOW), > > + DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_EVENT, > amdgpu_wait_event_drm_ioctl, > > +DRM_RENDER_ALLOW), > > }; > > > > static const struct drm_driver amdgpu_kms_driver = { diff --git > > a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > index ccc9c3f8aba7..72bea83d7408 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > @@ -1681,6 +1681,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, > struct drm_file *file_priv) > > } > > > > amdgpu_eventfd_mgr_init(&fpriv->eventfd_mgr); > > + amdgpu_wait_event_mgr_init(&fpriv->wait_event_mgr); > > > > pasid = amdgpu_pasid_alloc(16); > > if (pasid < 0) { > > @@ -1754,6 +1755,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, > struct drm_file *file_priv) > > if (pasid) > > amdgpu_pasid_free(pasid); > > > > + amdgpu_wait_event_mgr_fini(&fpriv->wait_event_mgr); > > kfree(fpriv); > > > > out_suspend: > > @@ -1784,8 +1786,9 @@ void amdgpu_driver_postclose_kms(struct > drm_device *dev, > > if (!fpriv) > > return; > > > > - /* Drop all subscriptions before fpriv goes away. */ > > + /* Drop eventfd subscriptions and pending wait-event records. */ > > amdgpu_eventfd_mgr_fini(&fpriv->eventfd_mgr); > > + amdgpu_wait_event_mgr_fini(&fpriv->wait_event_mgr); > > > > pm_runtime_get_sync(dev->dev); > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_wait_event.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_wait_event.c > > new file mode 100644 > > index 000000000000..db612106fb12 > > --- /dev/null > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_wait_event.c > > @@ -0,0 +1,427 @@ > > +// SPDX-License-Identifier: MIT > > +/* > > + * Copyright 2026 Advanced Micro Devices, Inc. > > + * > > + * Permission is hereby granted, free of charge, to any person > > +obtaining a > > + * copy of this software and associated documentation files (the > > +"Software"), > > + * to deal in the Software without restriction, including without > > +limitation > > + * the rights to use, copy, modify, merge, publish, distribute, > > +sublicense, > > + * and/or sell copies of the Software, and to permit persons to whom > > +the > > + * Software is furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice shall be > > +included in > > + * all copies or substantial portions of the Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY > KIND, > > +EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > +MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN > NO EVENT > > +SHALL > > + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY > CLAIM, > > +DAMAGES OR > > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > > +OTHERWISE, > > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE > OR THE USE > > +OR > > + * OTHER DEALINGS IN THE SOFTWARE. > > + * > > + */ > > + > > +#include <linux/jiffies.h> > > +#include <linux/sched/signal.h> > > +#include <linux/slab.h> > > +#include <linux/timekeeping.h> > > +#include <linux/uaccess.h> > > + > > +#include "amdgpu.h" > > +#include "amdgpu_userq_internal.h" > > +#include "amdgpu_wait_event.h" > > + > > +/** > > + * amdgpu_wait_event_deadline_to_jiffies - calculate remaining wait > > +time > > + * @deadline_ns: absolute CLOCK_MONOTONIC deadline in nanoseconds > > + * > > + * Convert an absolute CLOCK_MONOTONIC deadline into the remaining > > +number > > + * of jiffies. A negative deadline requests an indefinite wait. > > + * > > + * The caller recalculates the remaining time after every wakeup so > > +that > > + * spurious wakeups do not extend the userspace deadline. > > + * > > + * Return: > > + * Remaining wait time in jiffies, zero if the deadline has expired, > > +or > > + * MAX_SCHEDULE_TIMEOUT for an indefinite wait. > > + */ > > +static long > > +amdgpu_wait_event_deadline_to_jiffies(__s64 deadline_ns) { > > + u64 remaining_ns; > > + u64 now_ns; > > + u64 timeout; > > + > > + if (deadline_ns < 0) > > + return MAX_SCHEDULE_TIMEOUT; > > Usually absolute timeouts are unsigned values. Good point. I'll change the absolute deadline to an unsigned value in the next revision. > > > + > > + now_ns = ktime_get_ns(); > > + if ((u64)deadline_ns <= now_ns) > > + return 0; > > + > > + remaining_ns = (u64)deadline_ns - now_ns; > > + timeout = nsecs_to_jiffies(remaining_ns); > > + > > + /* > > + * Reserve MAX_SCHEDULE_TIMEOUT for the explicit indefinite-wait > > + * case above. > > + */ > > + if (timeout >= MAX_SCHEDULE_TIMEOUT) > > + return MAX_SCHEDULE_TIMEOUT - 1; > > I think that can be dropped. Agreed. I added the clamp only to support the negative value used for an indefinite wait. Since the deadline will now be unsigned and that special case will be removed, the clamp is no longer needed. I'll remove it in the next revision. > > > + > > + /* > > + * A future deadline shorter than one jiffy must still wait for at > > + * least one scheduler tick. > > + */ > > A timeout of 0 should usually mean check event and return. Is there any reson > for > this? I added it because I wanted a future deadline to wait instead of returning immediately due to jiffy rounding. But I agree it's better to follow the normal kernel timeout behavior. I'll remove the minimum one-jiffy adjustment in the next revision. Best regards, Srini > > Regards, > Christian.
