gpu/drm/amd/amdkfd/kdf_queue.c: removal of kfd_queue_buffer_put

[PATCH] gpu/drm/amd/amdkfd/kdf_queue.c: removal of kfd_queue_buffer_put

Posted by Alex Tran 1 month, 1 week ago

Removed kfd_queue_buffer_put to call amdgpu_bo_unref directly.

Signed-off-by: Alex Tran <alex.t.tran@gmail.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h            |  1 -
 .../drm/amd/amdkfd/kfd_process_queue_manager.c   |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_queue.c           | 16 +++++-----------
 3 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 67694bcd9464..2bc0365b2ce9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1312,7 +1312,6 @@ void print_queue_properties(struct queue_properties *q);
 void print_queue(struct queue *q);
 int kfd_queue_buffer_get(struct amdgpu_vm *vm, void __user *addr, struct amdgpu_bo **pbo,
 			 u64 expected_size);
-void kfd_queue_buffer_put(struct amdgpu_bo **bo);
 int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct queue_properties *properties);
 int kfd_queue_release_buffers(struct kfd_process_device *pdd, struct queue_properties *properties);
 void kfd_queue_unref_bo_va(struct amdgpu_vm *vm, struct amdgpu_bo **bo);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 7fbb5c274ccc..b13817e4a829 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -610,7 +610,7 @@ int pqm_update_queue_properties(struct process_queue_manager *pqm,
 		}
 
 		kfd_queue_unref_bo_va(vm, &pqn->q->properties.ring_bo);
-		kfd_queue_buffer_put(&pqn->q->properties.ring_bo);
+		amdgpu_bo_unref(&pqn->q->properties.ring_bo);
 		amdgpu_bo_unreserve(vm->root.bo);
 
 		pqn->q->properties.ring_bo = p->ring_bo;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
index a65c67cf56ff..dd529e37c0e6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
@@ -224,12 +224,6 @@ int kfd_queue_buffer_get(struct amdgpu_vm *vm, void __user *addr, struct amdgpu_
 	return -EINVAL;
 }
 
-/* FIXME: remove this function, just call amdgpu_bo_unref directly */
-void kfd_queue_buffer_put(struct amdgpu_bo **bo)
-{
-	amdgpu_bo_unref(bo);
-}
-
 int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct queue_properties *properties)
 {
 	struct kfd_topology_device *topo_dev;
@@ -343,11 +337,11 @@ int kfd_queue_release_buffers(struct kfd_process_device *pdd, struct queue_prope
 	struct kfd_topology_device *topo_dev;
 	u32 total_cwsr_size;
 
-	kfd_queue_buffer_put(&properties->wptr_bo);
-	kfd_queue_buffer_put(&properties->rptr_bo);
-	kfd_queue_buffer_put(&properties->ring_bo);
-	kfd_queue_buffer_put(&properties->eop_buf_bo);
-	kfd_queue_buffer_put(&properties->cwsr_bo);
+	amdgpu_bo_unref(&properties->wptr_bo);
+	amdgpu_bo_unref(&properties->rptr_bo);
+	amdgpu_bo_unref(&properties->ring_bo);
+	amdgpu_bo_unref(&properties->eop_buf_bo);
+	amdgpu_bo_unref(&properties->cwsr_bo);
 
 	topo_dev = kfd_topology_device_by_id(pdd->dev->id);
 	if (!topo_dev)
-- 
2.50.1

Re: [PATCH] gpu/drm/amd/amdkfd/kdf_queue.c: removal of kfd_queue_buffer_put

Posted by Alex Deucher 1 month, 1 week ago

On Sat, Aug 23, 2025 at 6:11 AM Alex Tran <alex.t.tran@gmail.com> wrote:
>
> Removed kfd_queue_buffer_put to call amdgpu_bo_unref directly.
>
> Signed-off-by: Alex Tran <alex.t.tran@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h            |  1 -
>  .../drm/amd/amdkfd/kfd_process_queue_manager.c   |  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_queue.c           | 16 +++++-----------
>  3 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 67694bcd9464..2bc0365b2ce9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -1312,7 +1312,6 @@ void print_queue_properties(struct queue_properties *q);
>  void print_queue(struct queue *q);
>  int kfd_queue_buffer_get(struct amdgpu_vm *vm, void __user *addr, struct amdgpu_bo **pbo,
>                          u64 expected_size);
> -void kfd_queue_buffer_put(struct amdgpu_bo **bo);
>  int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct queue_properties *properties);
>  int kfd_queue_release_buffers(struct kfd_process_device *pdd, struct queue_properties *properties);
>  void kfd_queue_unref_bo_va(struct amdgpu_vm *vm, struct amdgpu_bo **bo);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index 7fbb5c274ccc..b13817e4a829 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -610,7 +610,7 @@ int pqm_update_queue_properties(struct process_queue_manager *pqm,
>                 }
>
>                 kfd_queue_unref_bo_va(vm, &pqn->q->properties.ring_bo);
> -               kfd_queue_buffer_put(&pqn->q->properties.ring_bo);
> +               amdgpu_bo_unref(&pqn->q->properties.ring_bo);
>                 amdgpu_bo_unreserve(vm->root.bo);
>
>                 pqn->q->properties.ring_bo = p->ring_bo;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> index a65c67cf56ff..dd529e37c0e6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> @@ -224,12 +224,6 @@ int kfd_queue_buffer_get(struct amdgpu_vm *vm, void __user *addr, struct amdgpu_
>         return -EINVAL;
>  }
>
> -/* FIXME: remove this function, just call amdgpu_bo_unref directly */

Despite what this comment says, I think it makes sense to keep this
around so that we have a matching set of put and get functions so it's
clear why we are putting the reference.  That said if Felix and Harish
are ok with it, that's fine with me.

Alex

> -void kfd_queue_buffer_put(struct amdgpu_bo **bo)
> -{
> -       amdgpu_bo_unref(bo);
> -}
> -
>  int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct queue_properties *properties)
>  {
>         struct kfd_topology_device *topo_dev;
> @@ -343,11 +337,11 @@ int kfd_queue_release_buffers(struct kfd_process_device *pdd, struct queue_prope
>         struct kfd_topology_device *topo_dev;
>         u32 total_cwsr_size;
>
> -       kfd_queue_buffer_put(&properties->wptr_bo);
> -       kfd_queue_buffer_put(&properties->rptr_bo);
> -       kfd_queue_buffer_put(&properties->ring_bo);
> -       kfd_queue_buffer_put(&properties->eop_buf_bo);
> -       kfd_queue_buffer_put(&properties->cwsr_bo);
> +       amdgpu_bo_unref(&properties->wptr_bo);
> +       amdgpu_bo_unref(&properties->rptr_bo);
> +       amdgpu_bo_unref(&properties->ring_bo);
> +       amdgpu_bo_unref(&properties->eop_buf_bo);
> +       amdgpu_bo_unref(&properties->cwsr_bo);
>
>         topo_dev = kfd_topology_device_by_id(pdd->dev->id);
>         if (!topo_dev)
> --
> 2.50.1
>