(mistyped mailing list) ________________________________________ From: Tvrtko Ursulin <[email protected]> Sent: Friday, September 25, 2026 7:46 AM To: Francis, David; [email protected] Subject: Re: [PATCH v4 2/2] drm/amdgpu: Add CHANGE_ID option for USERQ ioctl
[Side note - you mistyped the mailing list address in this round.] On 23/09/2026 20:30, David Francis wrote: > Add a new option to the USERQ ioctl, which is called with > the queue_id of an existing user queue and an unused queue_id, > and changes that queue's id to the new value. > > Calling with an invalid new handle will fail. Calling with new_handle > = handle will succeed if that queue exists but not do anything. > > Performing this operation on a queue with signals or waits > outstanding is fine, as those hold not the queue_id but a > direct reference to the queue object. > > v3: Poison queue and misc fixes > v4: Using xas interface > > Signed-off-by: David Francis <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 49 +++++++++++++++++++++++ > include/uapi/drm/amdgpu_drm.h | 17 ++++++-- > 2 files changed, 63 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c > index 9338f6f4cc5f..d2ac797dcb7f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c > @@ -852,6 +852,11 @@ static int amdgpu_userq_input_args_validate(struct > drm_device *dev, > break; > case AMDGPU_USERQ_OP_LIST: > break; > + case AMDGPU_USERQ_OP_CHANGE_ID: > + if (!args->change_in.new_queue_id || > + args->change_in.new_queue_id > AMDGPU_MAX_USERQ_COUNT) > + return -EINVAL; > + break; > default: > return -EINVAL; > } > @@ -995,6 +1000,47 @@ amdgpu_userq_list(struct drm_file *filp, union > drm_amdgpu_userq *args) > return ret; > } > > +static int amdgpu_userq_change_id(struct drm_file *filp, union > drm_amdgpu_userq *args) > +{ > + struct amdgpu_fpriv *fpriv = filp->driver_priv; > + struct amdgpu_userq_mgr *uq_mgr = &fpriv->userq_mgr; > + XA_STATE(xas, &uq_mgr->userq_xa, args->change_in.new_queue_id); > + struct amdgpu_usermode_queue *queue; > + int ret; > + > + if (args->change_in.new_queue_id == args->change_in.queue_id) { > + if (xa_load(&uq_mgr->userq_xa, args->change_in.queue_id)) If you insist. > + return 0; > + return -ENOENT; > + } > + > + do { > + xas_lock(&xas); > + > + queue = xa_load(&uq_mgr->userq_xa, args->change_in.queue_id); > + if (!queue) { > + ret = -EINVAL; > + goto unlock; > + } > + > + if (xas_load(&xas)) { > + ret = -EBUSY; > + goto unlock; > + } > + > + xas_store(&xas, queue); > + ret = xas_error(&xas); > + if (ret) > + goto unlock; > + > + __xa_erase(&uq_mgr->userq_xa, args->change_in.queue_id); > + unlock: I think labels always go to column zero but please double check with checkpatch. > + xas_unlock(&xas); > + } while (xas_nomem(&xas, GFP_KERNEL)); Yep - this loop should work accordinging to my understanding. > + > + return ret; > +} > + > bool amdgpu_userq_enabled(struct drm_device *dev) > { > struct amdgpu_device *adev = drm_to_adev(dev); > @@ -1041,6 +1087,9 @@ int amdgpu_userq_ioctl(struct drm_device *dev, void > *data, > case AMDGPU_USERQ_OP_LIST: > r = amdgpu_userq_list(filp, args); > break; > + case AMDGPU_USERQ_OP_CHANGE_ID: > + r = amdgpu_userq_change_id(filp, args); > + break; > default: > drm_dbg_driver(dev, "Invalid user queue op specified: %d\n", > args->in.op); > return -EINVAL; > diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h > index 92738d630eea..37d0efd1eda4 100644 > --- a/include/uapi/drm/amdgpu_drm.h > +++ b/include/uapi/drm/amdgpu_drm.h > @@ -330,9 +330,10 @@ union drm_amdgpu_ctx { > }; > > /* user queue IOCTL operations */ > -#define AMDGPU_USERQ_OP_CREATE 1 > -#define AMDGPU_USERQ_OP_FREE 2 > -#define AMDGPU_USERQ_OP_LIST 3 > +#define AMDGPU_USERQ_OP_CREATE 1 > +#define AMDGPU_USERQ_OP_FREE 2 > +#define AMDGPU_USERQ_OP_LIST 3 > +#define AMDGPU_USERQ_OP_CHANGE_ID 4 > > /* queue priority levels */ > /* low < normal low < normal high < high */ > @@ -464,10 +465,20 @@ struct drm_amdgpu_userq_list_in_out { > __u64 entries; > }; > > +struct drm_amdgpu_userq_change_id_in { > + /** AMDGPU_USERQ_OP_CHANGE_ID */ > + __u32 op; > + /** Queue id of some queue */ > + __u32 queue_id; > + /** Queue id to change that queue to */ > + __u32 new_queue_id; > +}; > + > union drm_amdgpu_userq { > struct drm_amdgpu_userq_in in; > struct drm_amdgpu_userq_out out; > struct drm_amdgpu_userq_list_in_out list_in_out; > + struct drm_amdgpu_userq_change_id_in change_in; > }; > > /* GFX V11 IP specific MQD parameters */ Looks good to me, assuming checkpatch is either happy or fixed. Reviewed-by: Tvrtko Ursulin <[email protected]> We cannot merge it though without userspace and an IGT. Would you like me to re-post my IGT series so you can add userq tests to it? It would also be cool to progress the reviews on my render node amdgpu, IGT and CRIU patches. Regards, Tvrtko
