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

[Khatri, Sunil]

I feel existing implementation makes more sense. Using the disable_kq 
flag to avoid get/put completely. Get/put for userqueues only here and 
for kernel anyways we are handling in ring_init.

if (adev->userq_funcs[AMDGPU_HW_IP_GFX]), this if condition will always be valid once we enable it based on the fw version. so this will be an additional loop and get/put when kernel queues are being used.

Regards
Sunil Khatri

On 4/14/2025 3:12 PM, Khatri, Sunil 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.
>
> 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
>
> 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/8a1c7077/attachment-0001.htm>