Messages in this thread | | | Date | Thu, 28 Sep 2023 17:38:38 +0200 | Subject | Re: [PATCH v2 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8 | From | Shashank Sharma <> |
| |
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@amd.com> >>> Sent: Thursday, September 28, 2023 5:54 AM >>> To: Koenig, Christian <Christian.Koenig@amd.com>; Deucher, Alexander >>> <Alexander.Deucher@amd.com>; Sharma, Shashank >>> <Shashank.Sharma@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>; >>> Joshi, Mukul <Mukul.Joshi@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; >>> airlied@gmail.com; daniel@ffwll.ch >>> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; >>> linux- >>> kernel@vger.kernel.org; Yadav, Arvind <Arvind.Yadav@amd.com>; Koenig, >>> Christian <Christian.Koenig@amd.com> >>> 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@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 | 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.
- 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
| |