Re: [PATCH v2 12/12] drm/amdgpu: cleanup MES process level doorbells

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 12.04.23 um 18:25 schrieb Shashank Sharma:
> MES allocates process level doorbells, but there is no userspace
> client to consume it. It was only being used for the MES ring
> tests (in kernel), and was written by kernel doorbell write.
>
> The previous patch of this series has changed the MES ring test code to
> use kernel level MES doorbells. This patch now cleans up the process level
> doorbell allocation code which is not required.
>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Cc: Christian Koenig <christian.koenig@xxxxxxx>
> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxx>
> Signed-off-by: Arvind Yadav <arvind.yadav@xxxxxxx>

Reviewed-by: Christian König <christian.koenig@xxxxxxx>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 54 +------------------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h | 10 -----
>   2 files changed, 1 insertion(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> index cd3ee851f0a4..2180e8e2c82e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> @@ -36,35 +36,6 @@ int amdgpu_mes_doorbell_process_slice(struct amdgpu_device *adev)
>   		       PAGE_SIZE);
>   }
>   
> -int amdgpu_mes_alloc_process_doorbells(struct amdgpu_device *adev,
> -				      unsigned int *doorbell_index)
> -{
> -	int r = ida_simple_get(&adev->mes.doorbell_ida, 2,
> -			       adev->mes.max_doorbell_slices,
> -			       GFP_KERNEL);
> -	if (r > 0)
> -		*doorbell_index = r;
> -
> -	return r;
> -}
> -
> -void amdgpu_mes_free_process_doorbells(struct amdgpu_device *adev,
> -				      unsigned int doorbell_index)
> -{
> -	if (doorbell_index)
> -		ida_simple_remove(&adev->mes.doorbell_ida, doorbell_index);
> -}
> -
> -unsigned int amdgpu_mes_get_doorbell_dw_offset_in_bar(
> -					struct amdgpu_device *adev,
> -					uint32_t doorbell_index,
> -					unsigned int doorbell_id)
> -{
> -	return ((doorbell_index *
> -		amdgpu_mes_doorbell_process_slice(adev)) / sizeof(u32) +
> -		doorbell_id * 2);
> -}
> -
>   static int amdgpu_mes_kernel_doorbell_get(struct amdgpu_device *adev,
>   					 struct amdgpu_mes_process *process,
>   					 int ip_type, uint64_t *doorbell_index)
> @@ -256,15 +227,6 @@ int amdgpu_mes_create_process(struct amdgpu_device *adev, int pasid,
>   		return -ENOMEM;
>   	}
>   
> -	process->doorbell_bitmap =
> -		kzalloc(DIV_ROUND_UP(AMDGPU_MES_MAX_NUM_OF_QUEUES_PER_PROCESS,
> -				     BITS_PER_BYTE), GFP_KERNEL);
> -	if (!process->doorbell_bitmap) {
> -		DRM_ERROR("failed to allocate doorbell bitmap\n");
> -		kfree(process);
> -		return -ENOMEM;
> -	}
> -
>   	/* allocate the process context bo and map it */
>   	r = amdgpu_bo_create_kernel(adev, AMDGPU_MES_PROC_CTX_SIZE, PAGE_SIZE,
>   				    AMDGPU_GEM_DOMAIN_GTT,
> @@ -291,15 +253,6 @@ int amdgpu_mes_create_process(struct amdgpu_device *adev, int pasid,
>   		goto clean_up_ctx;
>   	}
>   
> -	/* allocate the starting doorbell index of the process */
> -	r = amdgpu_mes_alloc_process_doorbells(adev, &process->doorbell_index);
> -	if (r < 0) {
> -		DRM_ERROR("failed to allocate doorbell for process\n");
> -		goto clean_up_pasid;
> -	}
> -
> -	DRM_DEBUG("process doorbell index = %d\n", process->doorbell_index);
> -
>   	INIT_LIST_HEAD(&process->gang_list);
>   	process->vm = vm;
>   	process->pasid = pasid;
> @@ -309,15 +262,12 @@ int amdgpu_mes_create_process(struct amdgpu_device *adev, int pasid,
>   	amdgpu_mes_unlock(&adev->mes);
>   	return 0;
>   
> -clean_up_pasid:
> -	idr_remove(&adev->mes.pasid_idr, pasid);
> -	amdgpu_mes_unlock(&adev->mes);
>   clean_up_ctx:
> +	amdgpu_mes_unlock(&adev->mes);
>   	amdgpu_bo_free_kernel(&process->proc_ctx_bo,
>   			      &process->proc_ctx_gpu_addr,
>   			      &process->proc_ctx_cpu_ptr);
>   clean_up_memory:
> -	kfree(process->doorbell_bitmap);
>   	kfree(process);
>   	return r;
>   }
> @@ -363,7 +313,6 @@ void amdgpu_mes_destroy_process(struct amdgpu_device *adev, int pasid)
>   		idr_remove(&adev->mes.gang_id_idr, gang->gang_id);
>   	}
>   
> -	amdgpu_mes_free_process_doorbells(adev, process->doorbell_index);
>   	idr_remove(&adev->mes.pasid_idr, pasid);
>   	amdgpu_mes_unlock(&adev->mes);
>   
> @@ -385,7 +334,6 @@ void amdgpu_mes_destroy_process(struct amdgpu_device *adev, int pasid)
>   	amdgpu_bo_free_kernel(&process->proc_ctx_bo,
>   			      &process->proc_ctx_gpu_addr,
>   			      &process->proc_ctx_cpu_ptr);
> -	kfree(process->doorbell_bitmap);
>   	kfree(process);
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> index b04225585757..f96010dbd12a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> @@ -77,7 +77,6 @@ struct amdgpu_mes {
>   	uint32_t			kiq_version;
>   
>   	uint32_t                        total_max_queue;
> -	uint32_t                        doorbell_id_offset;
>   	uint32_t                        max_doorbell_slices;
>   
>   	uint64_t                        default_process_quantum;
> @@ -148,7 +147,6 @@ struct amdgpu_mes_process {
>   	uint64_t 		process_quantum;
>   	struct 			list_head gang_list;
>   	uint32_t 		doorbell_index;
> -	unsigned long 		*doorbell_bitmap;
>   	struct mutex		doorbell_lock;
>   };
>   
> @@ -367,14 +365,6 @@ int amdgpu_mes_ctx_unmap_meta_data(struct amdgpu_device *adev,
>   
>   int amdgpu_mes_self_test(struct amdgpu_device *adev);
>   
> -int amdgpu_mes_alloc_process_doorbells(struct amdgpu_device *adev,
> -					unsigned int *doorbell_index);
> -void amdgpu_mes_free_process_doorbells(struct amdgpu_device *adev,
> -					unsigned int doorbell_index);
> -unsigned int amdgpu_mes_get_doorbell_dw_offset_in_bar(
> -					struct amdgpu_device *adev,
> -					uint32_t doorbell_index,
> -					unsigned int doorbell_id);
>   int amdgpu_mes_doorbell_process_slice(struct amdgpu_device *adev);
>   
>   /*