[PATCH 11/16] drm/amdgpu: get absolute offset from doorbell index

[Luben Tuikov]

On 2023-03-29 11:47, Shashank Sharma wrote:
> This patch adds a helper function which converts a doorbell's
> relative index in a BO to an absolute doorbell offset in the
> doorbell BAR.
> 
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: Christian Koenig <christian.koenig at amd.com>
> Signed-off-by: Shashank Sharma <shashank.sharma at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h  | 15 +++++++++++
>  .../gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c  | 26 +++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> index 10a9bb10e974..3481e9d83879 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> @@ -383,6 +383,21 @@ int amdgpu_doorbell_alloc_page(struct amdgpu_device *adev,
>   */
>  int amdgpu_doorbell_create_kernel_doorbells(struct amdgpu_device *adev);
>  
> +/**
> + * amdgpu_doorbell_index_on_bar - Find doorbell's absolute offset in BAR
> + *
> + * @adev: amdgpu_device pointer
> + *
> + * @db_bo: doorbell object's bo
> + *
> + * @db_index: doorbell relative index in this doorbell object
> + *
> + * returns doorbell's absolute index in BAR
> + */
> +uint32_t amdgpu_doorbell_index_on_bar(struct amdgpu_device *adev,
> +				       struct amdgpu_bo *db_bo,
> +				       uint32_t doorbell_index);
> +

Two things:
1. No kernel doc for function declarations--this should go to where
the function is defined. (This also removes redundancy.)

2. No empty lines around function arguments in kernel doc. See this about
the format of function documentation:
https://www.kernel.org/doc/html/v4.12/doc-guide/kernel-doc.html#function-documentation

>  #define RDOORBELL32(index) amdgpu_mm_rdoorbell(adev, (index))
>  #define WDOORBELL32(index, v) amdgpu_mm_wdoorbell(adev, (index), (v))
>  #define RDOORBELL64(index) amdgpu_mm_rdoorbell64(adev, (index))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
> index 81713b2c28e1..c263bae6b0c4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
> @@ -130,6 +130,32 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device *adev, u32 index, u64 v)
>  	}
>  }
>  
> +/**
> + * amdgpu_doorbell_index_on_bar - Find doorbell's absolute offset in BAR
> + *
> + * @adev: amdgpu_device pointer
> + *
> + * @db_bo: doorbell object's bo
> + *
> + * @db_index: doorbell relative index in this doorbell object
> + *
> + * returns doorbell's absolute index in BAR
> + */
> +uint32_t amdgpu_doorbell_index_on_bar(struct amdgpu_device *adev,
> +				       struct amdgpu_bo *db_bo,
> +				       uint32_t doorbell_index)
> +{
> +	int db_bo_offset;
> +
> +	db_bo_offset = amdgpu_bo_gpu_offset_no_check(db_bo);

amdgpu_bo_gpu_offset_no_check() returns u64. Perhaps use u64,
or u32 (which is what this function returns) and cast it down.

> +
> +	/*
> +	 * doorbell index granularity is maintained at 32 bit
> +	 * but doorbell's size is 64-bit, so index * 2
> +	 */
> +	return db_bo_offset / sizeof(u32) + doorbell_index * 2;

Perhaps add this inside the comment:
* (db_bo_offset + doorbell_index * 8) / sizeof(u32),
which seems clearer to me. But leave the return as is.

Regards,
Luben

> +}
> +
>  /**
>   * amdgpu_doorbell_free_page - Free a doorbell page
>   *