[PATCH v6 03/11] drm/amdgpu: rework the userq doorbell object destroy

[Liang, Prike]

[Public]

Regards,
      Prike

> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig at amd.com>
> Sent: Friday, July 11, 2025 8:01 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 v6 03/11] drm/amdgpu: rework the userq doorbell object
> destroy
>
> On 11.07.25 11:39, Prike Liang wrote:
> > This patch aims to unify and destroy the userq doorbell objects at
> > mes_userq_mqd_destroy(), and this change will also help with unpinning
> > and destroying the userq doorbell objects for amdgpu_userq_mgr_fini()
> > during releasing the drm files.
> >
> > Signed-off-by: Prike Liang <Prike.Liang at amd.com>
> > Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c  | 6 ------
> > drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 7 +++++++
> >  2 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > index 3d2a7f8946cf..15e833b1b3e3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > @@ -312,12 +312,6 @@ amdgpu_userq_destroy(struct drm_file *filp, int
> queue_id)
> >             return -EINVAL;
> >     }
> >     amdgpu_userq_wait_for_last_fence(uq_mgr, queue);
> > -   r = amdgpu_bo_reserve(queue->db_obj.obj, true);
> > -   if (!r) {
> > -           amdgpu_bo_unpin(queue->db_obj.obj);
> > -           amdgpu_bo_unreserve(queue->db_obj.obj);
> > -   }
> > -   amdgpu_bo_unref(&queue->db_obj.obj);
> >     r = amdgpu_userq_unmap_helper(uq_mgr, queue);
> >     /*TODO: It requires a reset for userq hw unmap error*/
> >     if (unlikely(r != AMDGPU_USERQ_STATE_UNMAPPED)) { diff --git
> > a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > index 1457fb49a794..15aa1ca67a11 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > @@ -336,6 +336,13 @@ mes_userq_mqd_destroy(struct amdgpu_userq_mgr
> *uq_mgr,
> >                   struct amdgpu_usermode_queue *queue)  {
> >     amdgpu_userq_destroy_object(uq_mgr, &queue->fw_obj);
> > +
> > +   if (!amdgpu_bo_reserve(queue->db_obj.obj, true)) {
> > +           amdgpu_bo_unpin(queue->db_obj.obj);
> > +           amdgpu_bo_unreserve(queue->db_obj.obj);
> > +           amdgpu_userq_destroy_object(uq_mgr, &queue->db_obj);
> > +   }
> > +
>
> That makes no sense to do here. The pinning isn't done in mes_userq_mqd_create()
> either.
Yes, but the doorbell BO is pinned by amdgpu_userq_get_doorbell_index(), which is still
Invoked during userq BOs creation phase. This patch wants to free the doorbell object like some
other userq objects at the unified place of mes_userq_mqd_destroy().

> In general we should avoid pinning the MQD in the first place, that buffer needs to be
> fences instead.
If here not pin the userq doorbell BO, then will the doorbell index be changed when the doorbell
BO is moved?

> Regards,
> Christian.
>
> >     kfree(queue->userq_prop);
> >     amdgpu_userq_destroy_object(uq_mgr, &queue->mqd);  }