Messages in this thread | | | Date | Wed, 27 Sep 2023 19:59:34 +0200 | Subject | Re: [PATCH 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8 | From | Shashank Sharma <> |
| |
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<christian.koenig@amd.com> >> Cc: Alex Deucher<alexander.deucher@amd.com> >> Signed-off-by: Shashank Sharma<shashank.sharma@amd.com> >> Signed-off-by: Arvind Yadav<Arvind.Yadav@amd.com> >> --- >> 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; >> } >>
| |