On 2023-09-28 11:38, Shashank Sharma wrote:
Hello Felix, Mukul,
On 28/09/2023 17:30, Felix Kuehling wrote:
On 2023-09-28 10:30, Joshi, Mukul wrote:
[AMD Official Use Only - General]
-----Original Message-----
From: Yadav, Arvind <Arvind.Yadav@xxxxxxx>
Sent: Thursday, September 28, 2023 5:54 AM
To: Koenig, Christian <Christian.Koenig@xxxxxxx>; Deucher, Alexander
<Alexander.Deucher@xxxxxxx>; Sharma, Shashank
<Shashank.Sharma@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>;
Joshi, Mukul <Mukul.Joshi@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>;
airlied@xxxxxxxxx; daniel@xxxxxxxx
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
linux-
kernel@xxxxxxxxxxxxxxx; Yadav, Arvind <Arvind.Yadav@xxxxxxx>; Koenig,
Christian <Christian.Koenig@xxxxxxx>
Subject: [PATCH v2 1/1] drm/amdkfd: Fix unaligned doorbell absolute
offset
for gfx8
This patch is to adjust the absolute doorbell offset against the
doorbell id
considering the doorbell size of 32/64 bit.
v2:
- Addressed the review comment from Felix.
Cc: Christian Koenig <christian.koenig@xxxxxxx>
Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxx>
Signed-off-by: Arvind Yadav <Arvind.Yadav@xxxxxxx>
---
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 0d3d538b64eb..c54c4392d26e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -407,7 +407,14 @@ static int allocate_doorbell(struct
qcm_process_device *qpd,
q->properties.doorbell_off = amdgpu_doorbell_index_on_bar(dev-
adev,
qpd-
proc_doorbells,
- q-
doorbell_id);
+ 0);
+
It looks like amdgpu_doorbell_index_on_bar() works only for 64-bit
doorbells.
Shouldn't it work for both 32-bit and 64-bit doorbells considering
this is common
doorbell manager code?
Yes, You are right that the calculations to find a particular doorbell
in the doorbell page considers a doorbell width of 64-bit.
I could see this argument going either way. KFD is the only one that
cares about managing doorbells for user mode queues on GFXv8 GPUs.
This is not a use case that amdgpu cares about. So I'm OK with KFD
doing its own address calculations to make sure doorbells continue to
work on GFXv8.
It may not be worth adding complexity to the common doorbell manager
code to support legacy GPUs with 32-bit doorbells.
I was thinking about adding an additional input parameter which will
indicate if the doorbell width is 32-bit vs 64-bit (like
is_doorbell_64_bit), and doorbell manager can alter the multiplier
while calculating the final offset. Please let me know if that will
work for both the cases.
Yes, that would work for KFD because we already have the doorbell size
in our device-info structure. Instead of making it a boolean flag, you
could make it a doorbell_size parameter, in byte or dword units to
simplify the pointer math.
Regards,
Felix
- Shashank
Regards,
Felix
Thanks,
Mukul
+ /* Adjust the absolute doorbell offset against the doorbell id
considering
+ * the doorbell size of 32/64 bit.
+ */
+ q->properties.doorbell_off += q->doorbell_id *
+ dev->kfd->device_info.doorbell_size / 4;
+
return 0;
}
--
2.34.1
