[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8c9c3b5b-ba1e-ba07-3001-242eab9ca1a6@amd.com>
Date: Thu, 28 Sep 2023 17:38:38 +0200
From: Shashank Sharma <shashank.sharma@....com>
To: Felix Kuehling <felix.kuehling@....com>,
"Joshi, Mukul" <Mukul.Joshi@....com>,
"Yadav, Arvind" <Arvind.Yadav@....com>,
"Koenig, Christian" <Christian.Koenig@....com>,
"Deucher, Alexander" <Alexander.Deucher@....com>,
"Pan, Xinhui" <Xinhui.Pan@....com>,
"airlied@...il.com" <airlied@...il.com>,
"daniel@...ll.ch" <daniel@...ll.ch>
Cc: "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"amd-gfx@...ts.freedesktop.org" <amd-gfx@...ts.freedesktop.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset
for gfx8
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@....com>
>>> Sent: Thursday, September 28, 2023 5:54 AM
>>> To: Koenig, Christian <Christian.Koenig@....com>; Deucher, Alexander
>>> <Alexander.Deucher@....com>; Sharma, Shashank
>>> <Shashank.Sharma@....com>; Kuehling, Felix <Felix.Kuehling@....com>;
>>> Joshi, Mukul <Mukul.Joshi@....com>; Pan, Xinhui <Xinhui.Pan@....com>;
>>> airlied@...il.com; daniel@...ll.ch
>>> Cc: amd-gfx@...ts.freedesktop.org; dri-devel@...ts.freedesktop.org;
>>> linux-
>>> kernel@...r.kernel.org; Yadav, Arvind <Arvind.Yadav@....com>; Koenig,
>>> Christian <Christian.Koenig@....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@....com>
>>> Cc: Alex Deucher <alexander.deucher@....com>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@....com>
>>> Signed-off-by: Arvind Yadav <Arvind.Yadav@....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
Powered by blists - more mailing lists