<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 4/14/2025 10:54 PM, Alex Deucher
wrote:<br>
</div>
<blockquote type="cite" cite="mid:CADnq5_P7KKGW5etkuhB0Gy4rB9rHEj=+pTxZ8_-6+zUeVKgayg@mail.gmail.com">
<pre wrap="" class="moz-quote-pre">On Mon, Apr 14, 2025 at 1:17 PM Khatri, Sunil <a class="moz-txt-link-rfc2396E" href="mailto:sukhatri@amd.com"><sukhatri@amd.com></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">
On 4/14/2025 8:59 PM, Alex Deucher wrote:
On Mon, Apr 14, 2025 at 5:44 AM Khatri, Sunil <a class="moz-txt-link-rfc2396E" href="mailto:sukhatri@amd.com"><sukhatri@amd.com></a> 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.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
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.
</pre>
</blockquote>
Fair enough, we do enable the function pointers based on IP.<br>
<br>
Reviewed-by:
Sunil Khatri <a class="moz-txt-link-rfc2396E" href="mailto:sunil.khatri@amd.com"><sunil.khatri@amd.com></a><span style="white-space: pre-wrap"> </span>
<blockquote type="cite" cite="mid:CADnq5_P7KKGW5etkuhB0Gy4rB9rHEj=+pTxZ8_-6+zUeVKgayg@mail.gmail.com">
<pre wrap="" class="moz-quote-pre">Alex
</pre>
</blockquote>
<blockquote type="cite" cite="mid:CADnq5_P7KKGW5etkuhB0Gy4rB9rHEj=+pTxZ8_-6+zUeVKgayg@mail.gmail.com">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">
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 <a class="moz-txt-link-rfc2396E" href="mailto:alexander.deucher@amd.com"><alexander.deucher@amd.com></a>
---
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;
}
</pre>
</blockquote>
</blockquote>
</body>
</html>