[PATCH 1/4] drm/amdgpu/gfx11: properly reference EOP interrupts for userqs

[Khatri, Sunil]

On 4/14/2025 10:54 PM, Alex Deucher wrote:
> On Mon, Apr 14, 2025 at 1:17 PM Khatri, Sunil<sukhatri at amd.com> wrote:
>>
>> On 4/14/2025 8:59 PM, Alex Deucher wrote:
>>
>> On Mon, Apr 14, 2025 at 5:44 AM Khatri, Sunil<sukhatri at amd.com> wrote:
>>
>> This is how i see the future of this code and we can do based on it now itself.
>> disable_kq = 0, Use kernel queues.
>> disable_kq = 1, Use User queues.
>>
>> disable_kq = 0 means allow kernel queues and user queues.  disable_kq
>> =1 means disable kernel queues.  I think we'd want to allow both at
>> least on current chips.  I think if we want a general knob for kernel
>> and user queues, we should do something like:
>> userq:
>> -1 auto (whatever we want the default to be per IP)
>> 0 disable user queues (kernel queues only where supported)
>> 1 enable user queues (user queues and kernel queues)
>> 2 enable user queues only (disable kernel queues)
>>
>> In case of kernel queues we should not be even calling gfx_v11_0_set_userq_eop_interrupts at all. Instead its better if we add a this check "if (adev->userq_funcs[AMDGPU_HW_IP_GFX])" before calling the gfx_v11_0_set_userq_eop_interrupts. I am assuming there wont be any mixed use of kernel|user queues together. Let me know if you think otherwise. Regards Sunil Khatri
>>
>> We should only be calling it if user queues are enabled.  I think
>> there will definitely be mixed user and kernel queues on current
>> hardware as we ramp up on user queues.
>>
>> Alex, are you saying we could expect some device where Kernel queues and user queues will be working in parallel ? If that is the case i can see we need eop reference for both the cases and this change then makes perfect sense but,  if its either kernel or user then disable_kq feature check looked better in control.
>>
> That's the case right now for gfx11 and gfx12 dGPUs.  Even if we did
> add an option to disable user queues, everything would still work as
> expected with the way things are coded in these patches.  The presence
> of the userq function pointers determines whether user queues are
> enabled for the IP.  If they are NULL, then we skip the extra
> references so the logic works for any combination of user queues and
> kernel queues.
Fair enough,  we do enable the function pointers based on IP.

Reviewed-by: Sunil Khatri <sunil.khatri at amd.com>
> Alex
>
>> Regards
>> Sunil Khatri
>>
>> Alex
>>
>> On 4/13/2025 9:36 PM, Alex Deucher wrote:
>>
>> Regardless of whether we disable kernel queues, we need
>> to take an extra reference to the pipe interrupts for
>> user queues to make sure they stay enabled in case we
>> disable them for kernel queues.
>>
>> Signed-off-by: Alex Deucher<alexander.deucher at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
>> index 7274334ecd6fa..40d3c05326c02 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
>> @@ -4836,10 +4836,10 @@ static int gfx_v11_0_hw_init(struct amdgpu_ip_block *ip_block)
>>   static int gfx_v11_0_set_userq_eop_interrupts(struct amdgpu_device *adev,
>>         bool enable)
>>   {
>> - if (adev->gfx.disable_kq) {
>> - unsigned int irq_type;
>> - int m, p, r;
>> + unsigned int irq_type;
>> + int m, p, r;
>>
>> + if (adev->userq_funcs[AMDGPU_HW_IP_GFX]) {
>>    for (m = 0; m < adev->gfx.me.num_me; m++) {
>>    for (p = 0; p < adev->gfx.me.num_pipe_per_me; p++) {
>>    irq_type = AMDGPU_CP_IRQ_GFX_ME0_PIPE0_EOP + p;
>> @@ -4853,7 +4853,9 @@ static int gfx_v11_0_set_userq_eop_interrupts(struct amdgpu_device *adev,
>>    return r;
>>    }
>>    }
>> + }
>>
>> + if (adev->userq_funcs[AMDGPU_HW_IP_COMPUTE]) {
>>    for (m = 0; m < adev->gfx.mec.num_mec; ++m) {
>>    for (p = 0; p < adev->gfx.mec.num_pipe_per_mec; p++) {
>>    irq_type = AMDGPU_CP_IRQ_COMPUTE_MEC1_PIPE0_EOP
>> @@ -4870,6 +4872,7 @@ static int gfx_v11_0_set_userq_eop_interrupts(struct amdgpu_device *adev,
>>    }
>>    }
>>    }
>> +
>>    return 0;
>>   }
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20250414/9a1473fc/attachment-0001.htm>