Messages in this thread | | | Date | Fri, 29 Sep 2023 10:39:28 +0200 | Subject | Re: [PATCH v2 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8 | From | Shashank Sharma <> |
| |
On 28/09/2023 20:53, Felix Kuehling wrote: > 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@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. > > 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.
Sounds good, will do that and send an update.
- Shashank
> > 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
| |