Re: [PATCH 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8
On 27/09/2023 19:27, Felix Kuehling wrote: [+Mukul] On 2023-09-27 12:16, Arvind Yadav wrote: This patch is to adjust the absolute doorbell offset against the doorbell id considering the doorbell size of 32/64 bit. Cc: Christian Koenig Cc: Alex Deucher Signed-off-by: Shashank Sharma Signed-off-by: Arvind Yadav --- drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++- 1 file changed, 10 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..c327f7f604d7 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,16 @@ 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); Looks like we're now always calling amdgpu_doorbell_index_on_bar with the third parameter = 0. So we could remove that parameter. It doesn't do us any good and only causes bugs if we use any non-0 value. Hey Felix, Actually this was happening because in usermode KFD library the doorbell-size is for non-SOC15() hardware is hard coded to 4 bytes. We added this fix just so that the library code doesn't need to be changed, but can still get aligned to kernel code. GFX Usermode queue code uses this doorbell-index parameter, and it gives us the right offset. Regards Shashank + + /* Adjust the absolute doorbell offset against the doorbell id considering +* the doorbell size of 32/64 bit. +*/ + if (!KFD_IS_SOC15(dev)) + q->properties.doorbell_off += q->doorbell_id; + else + q->properties.doorbell_off += q->doorbell_id * 2; This could be simplified and generalized as q->properties.doorbell_off += q->doorbell_id * dev->kfd->device_info.doorbell_size / 4; Agree, we can do this simplification. - Shashank Regards, Felix + return 0; }
Re: [PATCH 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8
[+Mukul] On 2023-09-27 12:16, Arvind Yadav wrote: This patch is to adjust the absolute doorbell offset against the doorbell id considering the doorbell size of 32/64 bit. Cc: Christian Koenig Cc: Alex Deucher Signed-off-by: Shashank Sharma Signed-off-by: Arvind Yadav --- drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++- 1 file changed, 10 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..c327f7f604d7 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,16 @@ 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); Looks like we're now always calling amdgpu_doorbell_index_on_bar with the third parameter = 0. So we could remove that parameter. It doesn't do us any good and only causes bugs if we use any non-0 value. + + /* Adjust the absolute doorbell offset against the doorbell id considering +* the doorbell size of 32/64 bit. +*/ + if (!KFD_IS_SOC15(dev)) + q->properties.doorbell_off += q->doorbell_id; + else + q->properties.doorbell_off += q->doorbell_id * 2; This could be simplified and generalized as q->properties.doorbell_off += q->doorbell_id * dev->kfd->device_info.doorbell_size / 4; Regards, Felix + return 0; }
Re: [PATCH 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8
Adding felix.kuehl...@amd.com for review. Thanks ~Arvind On 9/27/2023 9:46 PM, Arvind Yadav wrote: This patch is to adjust the absolute doorbell offset against the doorbell id considering the doorbell size of 32/64 bit. Cc: Christian Koenig Cc: Alex Deucher Signed-off-by: Shashank Sharma Signed-off-by: Arvind Yadav --- drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++- 1 file changed, 10 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..c327f7f604d7 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,16 @@ 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); + + /* Adjust the absolute doorbell offset against the doorbell id considering +* the doorbell size of 32/64 bit. +*/ + if (!KFD_IS_SOC15(dev)) + q->properties.doorbell_off += q->doorbell_id; + else + q->properties.doorbell_off += q->doorbell_id * 2; + return 0; }
