On 30/03/2023 19:18, Luben Tuikov wrote:
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@xxxxxxx>
Cc: Christian Koenig <christian.koenig@xxxxxxx>
Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxx>
---
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
Noted, agreed.
#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.
Yeah, makes sense, will do it.
- Shashank
+
+ /*
+ * 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
*
