[PATCH v4 07/11] drm/amdgpu: add user queue vm identifier

[Liang, Prike]

[Public]

Regards,
      Prike

> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig at amd.com>
> Sent: Wednesday, June 25, 2025 3:52 PM
> To: Liang, Prike <Prike.Liang at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>
> Subject: Re: [PATCH v4 07/11] drm/amdgpu: add user queue vm identifier
>
> On 24.06.25 10:45, Prike Liang wrote:
> > Add a user queue vm identifier for each userqueue kms opt accessing.
> >
> > Signed-off-by: Prike Liang <Prike.Liang at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 4 ++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    | 2 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h    | 2 +-
> >  3 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > index 79b263c18eb1..0208c6b8a8e0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > @@ -501,6 +501,8 @@ amdgpu_userq_destroy(struct drm_file *filp, int queue_id)
> >     pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> >     pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> >
> > +   fpriv->vm.is_userq_context = false;
> > +
>
> I really don't like that approach. The VM should not have any special handling
> dependign on if user queues exists or not.
>
> Why do you want that?
This change is using the is_userq_context flag for identifying the user queue task to see whether it has been scheduled, and then
further used for handling the sequence of attaching the eviction fence . The eviction fence is only available when the user queue
process is scheduled, but in the amdgpu_gem_object_open(), it always attaches the eviction fence to the BOs regardless of whether
the eviction fence is valid or not. Based on this flag, we can simplify the eviction fence attaching sequence and only attach a valid
eviction fence to the BO.

>
> Regards,
> Christian.
>
> >     return r;
> >  }
> >
> > @@ -535,6 +537,8 @@ amdgpu_userq_create(struct drm_file *filp, union
> drm_amdgpu_userq *args)
> >             (args->in.flags &
> AMDGPU_USERQ_CREATE_FLAGS_QUEUE_PRIORITY_MASK) >>
> >             AMDGPU_USERQ_CREATE_FLAGS_QUEUE_PRIORITY_SHIFT;
> >
> > +   fpriv->vm.is_userq_context = true;
> > +
> >     r = amdgpu_userq_priority_permit(filp, priority);
> >     if (r)
> >             return r;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > index 3911c78f8282..5e075e8f0ca3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -2560,7 +2560,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev,
> struct amdgpu_vm *vm,
> >     ttm_lru_bulk_move_init(&vm->lru_bulk_move);
> >
> >     vm->is_compute_context = false;
> > -
> > +   vm->is_userq_context = false;
> >     vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
> >                                 AMDGPU_VM_USE_CPU_FOR_GFX);
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > index f3ad687125ad..1e3c0f85c98e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > @@ -422,7 +422,7 @@ struct amdgpu_vm {
> >     struct ttm_lru_bulk_move lru_bulk_move;
> >     /* Flag to indicate if VM is used for compute */
> >     bool                    is_compute_context;
> > -
> > +   bool                    is_userq_context;
> >     /* Memory partition number, -1 means any partition */
> >     int8_t                  mem_id;
> >