* [PATCH 01/11] drm/amdgpu: add parameter to disable kernel queues
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
@ 2025-03-13 14:41 ` Alex Deucher
2025-03-13 14:41 ` [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions Alex Deucher
` (10 subsequent siblings)
11 siblings, 0 replies; 41+ messages in thread
From: Alex Deucher @ 2025-03-13 14:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Prike Liang, Sunil Khatri
On chips that support user queues, setting this option
will disable kernel queues to be used to validate
user queues without kernel queues.
Reviewed-by: Prike Liang <Prike.Liang@amd.com>
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 87062c1adcdf7..45437a8f29d3b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -270,6 +270,7 @@ extern int amdgpu_user_partt_mode;
extern int amdgpu_agp;
extern int amdgpu_wbrf;
+extern int amdgpu_disable_kq;
#define AMDGPU_VM_MAX_NUM_CTX 4096
#define AMDGPU_SG_THRESHOLD (256*1024*1024)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index ba8111169fa99..f50a25fb60376 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -238,6 +238,7 @@ int amdgpu_agp = -1; /* auto */
int amdgpu_wbrf = -1;
int amdgpu_damage_clips = -1; /* auto */
int amdgpu_umsch_mm_fwlog;
+int amdgpu_disable_kq = -1;
DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_CORE",
@@ -1084,6 +1085,14 @@ MODULE_PARM_DESC(wbrf,
"Enable Wifi RFI interference mitigation (0 = disabled, 1 = enabled, -1 = auto(default)");
module_param_named(wbrf, amdgpu_wbrf, int, 0444);
+/**
+ * DOC: disable_kq (int)
+ * Disable kernel queues on systems that support user queues.
+ * (0 = kernel queues enabled, 1 = kernel queues disabled, -1 = auto (default setting))
+ */
+MODULE_PARM_DESC(disable_kq, "Disable kernel queues (-1 = auto (default), 0 = enable KQ, 1 = disable KQ)");
+module_param_named(disable_kq, amdgpu_disable_kq, int, 0444);
+
/* These devices are not supported by amdgpu.
* They are supported by the mach64, r128, radeon drivers
*/
--
2.48.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
2025-03-13 14:41 ` [PATCH 01/11] drm/amdgpu: add parameter to disable kernel queues Alex Deucher
@ 2025-03-13 14:41 ` Alex Deucher
2025-03-13 21:52 ` Rodrigo Siqueira
2025-03-17 7:08 ` Liang, Prike
2025-03-13 14:41 ` [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq Alex Deucher
` (9 subsequent siblings)
11 siblings, 2 replies; 41+ messages in thread
From: Alex Deucher @ 2025-03-13 14:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Sunil Khatri
This would be set by IPs which only accept submissions
from the kernel, not userspace, such as when kernel
queues are disabled. Don't expose the rings to userspace
and reject any submissions in the CS IOCTL.
Reviewed-by: Sunil Khatri<sunil.khatri@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ++++
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 30 ++++++++++++++++--------
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
3 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 5df21529b3b13..5cc18034b75df 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -349,6 +349,10 @@ static int amdgpu_cs_p2_ib(struct amdgpu_cs_parser *p,
ring = amdgpu_job_ring(job);
ib = &job->ibs[job->num_ibs++];
+ /* submissions to kernel queus are disabled */
+ if (ring->no_user_submission)
+ return -EINVAL;
+
/* MM engine doesn't support user fences */
if (p->uf_bo && ring->funcs->no_user_fence)
return -EINVAL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index cd6eb7a3bc58a..3b7dfd56ccd0e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -408,7 +408,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
case AMDGPU_HW_IP_GFX:
type = AMD_IP_BLOCK_TYPE_GFX;
for (i = 0; i < adev->gfx.num_gfx_rings; i++)
- if (adev->gfx.gfx_ring[i].sched.ready)
+ if (adev->gfx.gfx_ring[i].sched.ready &&
+ !adev->gfx.gfx_ring[i].no_user_submission)
++num_rings;
ib_start_alignment = 32;
ib_size_alignment = 32;
@@ -416,7 +417,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
case AMDGPU_HW_IP_COMPUTE:
type = AMD_IP_BLOCK_TYPE_GFX;
for (i = 0; i < adev->gfx.num_compute_rings; i++)
- if (adev->gfx.compute_ring[i].sched.ready)
+ if (adev->gfx.compute_ring[i].sched.ready &&
+ !adev->gfx.compute_ring[i].no_user_submission)
++num_rings;
ib_start_alignment = 32;
ib_size_alignment = 32;
@@ -424,7 +426,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
case AMDGPU_HW_IP_DMA:
type = AMD_IP_BLOCK_TYPE_SDMA;
for (i = 0; i < adev->sdma.num_instances; i++)
- if (adev->sdma.instance[i].ring.sched.ready)
+ if (adev->sdma.instance[i].ring.sched.ready &&
+ !adev->gfx.gfx_ring[i].no_user_submission)
++num_rings;
ib_start_alignment = 256;
ib_size_alignment = 4;
@@ -435,7 +438,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
if (adev->uvd.harvest_config & (1 << i))
continue;
- if (adev->uvd.inst[i].ring.sched.ready)
+ if (adev->uvd.inst[i].ring.sched.ready &&
+ !adev->uvd.inst[i].ring.no_user_submission)
++num_rings;
}
ib_start_alignment = 256;
@@ -444,7 +448,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
case AMDGPU_HW_IP_VCE:
type = AMD_IP_BLOCK_TYPE_VCE;
for (i = 0; i < adev->vce.num_rings; i++)
- if (adev->vce.ring[i].sched.ready)
+ if (adev->vce.ring[i].sched.ready &&
+ !adev->vce.ring[i].no_user_submission)
++num_rings;
ib_start_alignment = 256;
ib_size_alignment = 4;
@@ -456,7 +461,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
continue;
for (j = 0; j < adev->uvd.num_enc_rings; j++)
- if (adev->uvd.inst[i].ring_enc[j].sched.ready)
+ if (adev->uvd.inst[i].ring_enc[j].sched.ready &&
+ !adev->uvd.inst[i].ring_enc[j].no_user_submission)
++num_rings;
}
ib_start_alignment = 256;
@@ -468,7 +474,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
if (adev->vcn.harvest_config & (1 << i))
continue;
- if (adev->vcn.inst[i].ring_dec.sched.ready)
+ if (adev->vcn.inst[i].ring_dec.sched.ready &&
+ !adev->vcn.inst[i].ring_dec.no_user_submission)
++num_rings;
}
ib_start_alignment = 256;
@@ -481,7 +488,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
continue;
for (j = 0; j < adev->vcn.inst[i].num_enc_rings; j++)
- if (adev->vcn.inst[i].ring_enc[j].sched.ready)
+ if (adev->vcn.inst[i].ring_enc[j].sched.ready &&
+ !adev->vcn.inst[i].ring_enc[j].no_user_submission)
++num_rings;
}
ib_start_alignment = 256;
@@ -496,7 +504,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
continue;
for (j = 0; j < adev->jpeg.num_jpeg_rings; j++)
- if (adev->jpeg.inst[i].ring_dec[j].sched.ready)
+ if (adev->jpeg.inst[i].ring_dec[j].sched.ready &&
+ !adev->jpeg.inst[i].ring_dec[j].no_user_submission)
++num_rings;
}
ib_start_alignment = 256;
@@ -504,7 +513,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
break;
case AMDGPU_HW_IP_VPE:
type = AMD_IP_BLOCK_TYPE_VPE;
- if (adev->vpe.ring.sched.ready)
+ if (adev->vpe.ring.sched.ready &&
+ !adev->vpe.ring.no_user_submission)
++num_rings;
ib_start_alignment = 256;
ib_size_alignment = 4;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index b4fd1e17205e9..4a97afcb38b78 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -297,6 +297,7 @@ struct amdgpu_ring {
struct dma_fence *vmid_wait;
bool has_compute_vm_bug;
bool no_scheduler;
+ bool no_user_submission;
int hw_prio;
unsigned num_hw_submission;
atomic_t *sched_score;
@@ -310,7 +311,6 @@ struct amdgpu_ring {
unsigned int entry_index;
/* store the cached rptr to restore after reset */
uint64_t cached_rptr;
-
};
#define amdgpu_ring_parse_cs(r, p, job, ib) ((r)->funcs->parse_cs((p), (job), (ib)))
--
2.48.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions
2025-03-13 14:41 ` [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions Alex Deucher
@ 2025-03-13 21:52 ` Rodrigo Siqueira
2025-03-14 1:44 ` Alex Deucher
2025-03-17 7:08 ` Liang, Prike
1 sibling, 1 reply; 41+ messages in thread
From: Rodrigo Siqueira @ 2025-03-13 21:52 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx, Sunil Khatri
On 03/13, Alex Deucher wrote:
> This would be set by IPs which only accept submissions
> from the kernel, not userspace, such as when kernel
> queues are disabled. Don't expose the rings to userspace
> and reject any submissions in the CS IOCTL.
Just out of curiosity, is CS == Command Submission?
>
> Reviewed-by: Sunil Khatri<sunil.khatri@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 30 ++++++++++++++++--------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
> 3 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 5df21529b3b13..5cc18034b75df 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -349,6 +349,10 @@ static int amdgpu_cs_p2_ib(struct amdgpu_cs_parser *p,
> ring = amdgpu_job_ring(job);
> ib = &job->ibs[job->num_ibs++];
>
> + /* submissions to kernel queus are disabled */
/queus/queues/
> + if (ring->no_user_submission)
> + return -EINVAL;
Since this will be set for ASICs that don't accept userspace
submissions, maybe -ENOTSUPP would be more accurate?
Thanks
> +
> /* MM engine doesn't support user fences */
> if (p->uf_bo && ring->funcs->no_user_fence)
> return -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index cd6eb7a3bc58a..3b7dfd56ccd0e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -408,7 +408,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> case AMDGPU_HW_IP_GFX:
> type = AMD_IP_BLOCK_TYPE_GFX;
> for (i = 0; i < adev->gfx.num_gfx_rings; i++)
> - if (adev->gfx.gfx_ring[i].sched.ready)
> + if (adev->gfx.gfx_ring[i].sched.ready &&
> + !adev->gfx.gfx_ring[i].no_user_submission)
> ++num_rings;
> ib_start_alignment = 32;
> ib_size_alignment = 32;
> @@ -416,7 +417,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> case AMDGPU_HW_IP_COMPUTE:
> type = AMD_IP_BLOCK_TYPE_GFX;
> for (i = 0; i < adev->gfx.num_compute_rings; i++)
> - if (adev->gfx.compute_ring[i].sched.ready)
> + if (adev->gfx.compute_ring[i].sched.ready &&
> + !adev->gfx.compute_ring[i].no_user_submission)
> ++num_rings;
> ib_start_alignment = 32;
> ib_size_alignment = 32;
> @@ -424,7 +426,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> case AMDGPU_HW_IP_DMA:
> type = AMD_IP_BLOCK_TYPE_SDMA;
> for (i = 0; i < adev->sdma.num_instances; i++)
> - if (adev->sdma.instance[i].ring.sched.ready)
> + if (adev->sdma.instance[i].ring.sched.ready &&
> + !adev->gfx.gfx_ring[i].no_user_submission)
> ++num_rings;
> ib_start_alignment = 256;
> ib_size_alignment = 4;
> @@ -435,7 +438,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> if (adev->uvd.harvest_config & (1 << i))
> continue;
>
> - if (adev->uvd.inst[i].ring.sched.ready)
> + if (adev->uvd.inst[i].ring.sched.ready &&
> + !adev->uvd.inst[i].ring.no_user_submission)
> ++num_rings;
> }
> ib_start_alignment = 256;
> @@ -444,7 +448,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> case AMDGPU_HW_IP_VCE:
> type = AMD_IP_BLOCK_TYPE_VCE;
> for (i = 0; i < adev->vce.num_rings; i++)
> - if (adev->vce.ring[i].sched.ready)
> + if (adev->vce.ring[i].sched.ready &&
> + !adev->vce.ring[i].no_user_submission)
> ++num_rings;
> ib_start_alignment = 256;
> ib_size_alignment = 4;
> @@ -456,7 +461,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> continue;
>
> for (j = 0; j < adev->uvd.num_enc_rings; j++)
> - if (adev->uvd.inst[i].ring_enc[j].sched.ready)
> + if (adev->uvd.inst[i].ring_enc[j].sched.ready &&
> + !adev->uvd.inst[i].ring_enc[j].no_user_submission)
> ++num_rings;
> }
> ib_start_alignment = 256;
> @@ -468,7 +474,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> if (adev->vcn.harvest_config & (1 << i))
> continue;
>
> - if (adev->vcn.inst[i].ring_dec.sched.ready)
> + if (adev->vcn.inst[i].ring_dec.sched.ready &&
> + !adev->vcn.inst[i].ring_dec.no_user_submission)
> ++num_rings;
> }
> ib_start_alignment = 256;
> @@ -481,7 +488,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> continue;
>
> for (j = 0; j < adev->vcn.inst[i].num_enc_rings; j++)
> - if (adev->vcn.inst[i].ring_enc[j].sched.ready)
> + if (adev->vcn.inst[i].ring_enc[j].sched.ready &&
> + !adev->vcn.inst[i].ring_enc[j].no_user_submission)
> ++num_rings;
> }
> ib_start_alignment = 256;
> @@ -496,7 +504,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> continue;
>
> for (j = 0; j < adev->jpeg.num_jpeg_rings; j++)
> - if (adev->jpeg.inst[i].ring_dec[j].sched.ready)
> + if (adev->jpeg.inst[i].ring_dec[j].sched.ready &&
> + !adev->jpeg.inst[i].ring_dec[j].no_user_submission)
> ++num_rings;
> }
> ib_start_alignment = 256;
> @@ -504,7 +513,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> break;
> case AMDGPU_HW_IP_VPE:
> type = AMD_IP_BLOCK_TYPE_VPE;
> - if (adev->vpe.ring.sched.ready)
> + if (adev->vpe.ring.sched.ready &&
> + !adev->vpe.ring.no_user_submission)
> ++num_rings;
> ib_start_alignment = 256;
> ib_size_alignment = 4;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index b4fd1e17205e9..4a97afcb38b78 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -297,6 +297,7 @@ struct amdgpu_ring {
> struct dma_fence *vmid_wait;
> bool has_compute_vm_bug;
> bool no_scheduler;
> + bool no_user_submission;
> int hw_prio;
> unsigned num_hw_submission;
> atomic_t *sched_score;
> @@ -310,7 +311,6 @@ struct amdgpu_ring {
> unsigned int entry_index;
> /* store the cached rptr to restore after reset */
> uint64_t cached_rptr;
> -
> };
>
> #define amdgpu_ring_parse_cs(r, p, job, ib) ((r)->funcs->parse_cs((p), (job), (ib)))
> --
> 2.48.1
>
--
Rodrigo Siqueira
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions
2025-03-13 21:52 ` Rodrigo Siqueira
@ 2025-03-14 1:44 ` Alex Deucher
0 siblings, 0 replies; 41+ messages in thread
From: Alex Deucher @ 2025-03-14 1:44 UTC (permalink / raw)
To: Rodrigo Siqueira; +Cc: Alex Deucher, amd-gfx, Sunil Khatri
On Thu, Mar 13, 2025 at 5:53 PM Rodrigo Siqueira <siqueira@igalia.com> wrote:
>
> On 03/13, Alex Deucher wrote:
> > This would be set by IPs which only accept submissions
> > from the kernel, not userspace, such as when kernel
> > queues are disabled. Don't expose the rings to userspace
> > and reject any submissions in the CS IOCTL.
>
> Just out of curiosity, is CS == Command Submission?
Yes.
>
> >
> > Reviewed-by: Sunil Khatri<sunil.khatri@amd.com>
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ++++
> > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 30 ++++++++++++++++--------
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
> > 3 files changed, 25 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > index 5df21529b3b13..5cc18034b75df 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > @@ -349,6 +349,10 @@ static int amdgpu_cs_p2_ib(struct amdgpu_cs_parser *p,
> > ring = amdgpu_job_ring(job);
> > ib = &job->ibs[job->num_ibs++];
> >
> > + /* submissions to kernel queus are disabled */
>
> /queus/queues/
will fix.
>
> > + if (ring->no_user_submission)
> > + return -EINVAL;
>
> Since this will be set for ASICs that don't accept userspace
> submissions, maybe -ENOTSUPP would be more accurate?
I could go either way on that. When kernel queues are disabled, CS
submissions to a kernel queue are not supported, but it's also an
invalid parameter to the IOCTL. I don't have a strong opinion. Maybe
-ENOTSUPP would differentiate it from just general invalid parameters.
Alex
>
> Thanks
>
> > +
> > /* MM engine doesn't support user fences */
> > if (p->uf_bo && ring->funcs->no_user_fence)
> > return -EINVAL;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > index cd6eb7a3bc58a..3b7dfd56ccd0e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > @@ -408,7 +408,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> > case AMDGPU_HW_IP_GFX:
> > type = AMD_IP_BLOCK_TYPE_GFX;
> > for (i = 0; i < adev->gfx.num_gfx_rings; i++)
> > - if (adev->gfx.gfx_ring[i].sched.ready)
> > + if (adev->gfx.gfx_ring[i].sched.ready &&
> > + !adev->gfx.gfx_ring[i].no_user_submission)
> > ++num_rings;
> > ib_start_alignment = 32;
> > ib_size_alignment = 32;
> > @@ -416,7 +417,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> > case AMDGPU_HW_IP_COMPUTE:
> > type = AMD_IP_BLOCK_TYPE_GFX;
> > for (i = 0; i < adev->gfx.num_compute_rings; i++)
> > - if (adev->gfx.compute_ring[i].sched.ready)
> > + if (adev->gfx.compute_ring[i].sched.ready &&
> > + !adev->gfx.compute_ring[i].no_user_submission)
> > ++num_rings;
> > ib_start_alignment = 32;
> > ib_size_alignment = 32;
> > @@ -424,7 +426,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> > case AMDGPU_HW_IP_DMA:
> > type = AMD_IP_BLOCK_TYPE_SDMA;
> > for (i = 0; i < adev->sdma.num_instances; i++)
> > - if (adev->sdma.instance[i].ring.sched.ready)
> > + if (adev->sdma.instance[i].ring.sched.ready &&
> > + !adev->gfx.gfx_ring[i].no_user_submission)
> > ++num_rings;
> > ib_start_alignment = 256;
> > ib_size_alignment = 4;
> > @@ -435,7 +438,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> > if (adev->uvd.harvest_config & (1 << i))
> > continue;
> >
> > - if (adev->uvd.inst[i].ring.sched.ready)
> > + if (adev->uvd.inst[i].ring.sched.ready &&
> > + !adev->uvd.inst[i].ring.no_user_submission)
> > ++num_rings;
> > }
> > ib_start_alignment = 256;
> > @@ -444,7 +448,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> > case AMDGPU_HW_IP_VCE:
> > type = AMD_IP_BLOCK_TYPE_VCE;
> > for (i = 0; i < adev->vce.num_rings; i++)
> > - if (adev->vce.ring[i].sched.ready)
> > + if (adev->vce.ring[i].sched.ready &&
> > + !adev->vce.ring[i].no_user_submission)
> > ++num_rings;
> > ib_start_alignment = 256;
> > ib_size_alignment = 4;
> > @@ -456,7 +461,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> > continue;
> >
> > for (j = 0; j < adev->uvd.num_enc_rings; j++)
> > - if (adev->uvd.inst[i].ring_enc[j].sched.ready)
> > + if (adev->uvd.inst[i].ring_enc[j].sched.ready &&
> > + !adev->uvd.inst[i].ring_enc[j].no_user_submission)
> > ++num_rings;
> > }
> > ib_start_alignment = 256;
> > @@ -468,7 +474,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> > if (adev->vcn.harvest_config & (1 << i))
> > continue;
> >
> > - if (adev->vcn.inst[i].ring_dec.sched.ready)
> > + if (adev->vcn.inst[i].ring_dec.sched.ready &&
> > + !adev->vcn.inst[i].ring_dec.no_user_submission)
> > ++num_rings;
> > }
> > ib_start_alignment = 256;
> > @@ -481,7 +488,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> > continue;
> >
> > for (j = 0; j < adev->vcn.inst[i].num_enc_rings; j++)
> > - if (adev->vcn.inst[i].ring_enc[j].sched.ready)
> > + if (adev->vcn.inst[i].ring_enc[j].sched.ready &&
> > + !adev->vcn.inst[i].ring_enc[j].no_user_submission)
> > ++num_rings;
> > }
> > ib_start_alignment = 256;
> > @@ -496,7 +504,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> > continue;
> >
> > for (j = 0; j < adev->jpeg.num_jpeg_rings; j++)
> > - if (adev->jpeg.inst[i].ring_dec[j].sched.ready)
> > + if (adev->jpeg.inst[i].ring_dec[j].sched.ready &&
> > + !adev->jpeg.inst[i].ring_dec[j].no_user_submission)
> > ++num_rings;
> > }
> > ib_start_alignment = 256;
> > @@ -504,7 +513,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device *adev,
> > break;
> > case AMDGPU_HW_IP_VPE:
> > type = AMD_IP_BLOCK_TYPE_VPE;
> > - if (adev->vpe.ring.sched.ready)
> > + if (adev->vpe.ring.sched.ready &&
> > + !adev->vpe.ring.no_user_submission)
> > ++num_rings;
> > ib_start_alignment = 256;
> > ib_size_alignment = 4;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > index b4fd1e17205e9..4a97afcb38b78 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > @@ -297,6 +297,7 @@ struct amdgpu_ring {
> > struct dma_fence *vmid_wait;
> > bool has_compute_vm_bug;
> > bool no_scheduler;
> > + bool no_user_submission;
> > int hw_prio;
> > unsigned num_hw_submission;
> > atomic_t *sched_score;
> > @@ -310,7 +311,6 @@ struct amdgpu_ring {
> > unsigned int entry_index;
> > /* store the cached rptr to restore after reset */
> > uint64_t cached_rptr;
> > -
> > };
> >
> > #define amdgpu_ring_parse_cs(r, p, job, ib) ((r)->funcs->parse_cs((p), (job), (ib)))
> > --
> > 2.48.1
> >
>
> --
> Rodrigo Siqueira
^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions
2025-03-13 14:41 ` [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions Alex Deucher
2025-03-13 21:52 ` Rodrigo Siqueira
@ 2025-03-17 7:08 ` Liang, Prike
2025-03-17 17:15 ` Marek Olšák
1 sibling, 1 reply; 41+ messages in thread
From: Liang, Prike @ 2025-03-17 7:08 UTC (permalink / raw)
To: Deucher, Alexander, amd-gfx@lists.freedesktop.org, Olsak, Marek
Cc: Deucher, Alexander, Khatri, Sunil
[-- Attachment #1: Type: text/plain, Size: 8842 bytes --]
[Public]
We might still need to export each ring's number correctly; otherwise, the Mesa driver will consider there's no available ring supported from the driver and then further assert before submitting the user queue.
If we want to keep the ring number being zero, the Mesa driver may need an attachment change to allow the command submitted to the zero-ring number if the user queue is enabled.
Hi @Olsak, Marek Do you think it's fine to have the attachment patch for the userq support? Except for such changes, maybe we also need to clean up the IB-related part.
Regards,
Prike
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> Deucher
> Sent: Thursday, March 13, 2025 10:41 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Khatri, Sunil
> <Sunil.Khatri@amd.com>
> Subject: [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions
>
> This would be set by IPs which only accept submissions from the kernel, not
> userspace, such as when kernel queues are disabled. Don't expose the rings to
> userspace and reject any submissions in the CS IOCTL.
>
> Reviewed-by: Sunil Khatri<sunil.khatri@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 30 ++++++++++++++++--------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
> 3 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 5df21529b3b13..5cc18034b75df 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -349,6 +349,10 @@ static int amdgpu_cs_p2_ib(struct amdgpu_cs_parser
> *p,
> ring = amdgpu_job_ring(job);
> ib = &job->ibs[job->num_ibs++];
>
> + /* submissions to kernel queus are disabled */
> + if (ring->no_user_submission)
> + return -EINVAL;
> +
> /* MM engine doesn't support user fences */
> if (p->uf_bo && ring->funcs->no_user_fence)
> return -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index cd6eb7a3bc58a..3b7dfd56ccd0e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -408,7 +408,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> *adev,
> case AMDGPU_HW_IP_GFX:
> type = AMD_IP_BLOCK_TYPE_GFX;
> for (i = 0; i < adev->gfx.num_gfx_rings; i++)
> - if (adev->gfx.gfx_ring[i].sched.ready)
> + if (adev->gfx.gfx_ring[i].sched.ready &&
> + !adev->gfx.gfx_ring[i].no_user_submission)
> ++num_rings;
> ib_start_alignment = 32;
> ib_size_alignment = 32;
> @@ -416,7 +417,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> *adev,
> case AMDGPU_HW_IP_COMPUTE:
> type = AMD_IP_BLOCK_TYPE_GFX;
> for (i = 0; i < adev->gfx.num_compute_rings; i++)
> - if (adev->gfx.compute_ring[i].sched.ready)
> + if (adev->gfx.compute_ring[i].sched.ready &&
> + !adev->gfx.compute_ring[i].no_user_submission)
> ++num_rings;
> ib_start_alignment = 32;
> ib_size_alignment = 32;
> @@ -424,7 +426,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> *adev,
> case AMDGPU_HW_IP_DMA:
> type = AMD_IP_BLOCK_TYPE_SDMA;
> for (i = 0; i < adev->sdma.num_instances; i++)
> - if (adev->sdma.instance[i].ring.sched.ready)
> + if (adev->sdma.instance[i].ring.sched.ready &&
> + !adev->gfx.gfx_ring[i].no_user_submission)
> ++num_rings;
> ib_start_alignment = 256;
> ib_size_alignment = 4;
> @@ -435,7 +438,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> *adev,
> if (adev->uvd.harvest_config & (1 << i))
> continue;
>
> - if (adev->uvd.inst[i].ring.sched.ready)
> + if (adev->uvd.inst[i].ring.sched.ready &&
> + !adev->uvd.inst[i].ring.no_user_submission)
> ++num_rings;
> }
> ib_start_alignment = 256;
> @@ -444,7 +448,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> *adev,
> case AMDGPU_HW_IP_VCE:
> type = AMD_IP_BLOCK_TYPE_VCE;
> for (i = 0; i < adev->vce.num_rings; i++)
> - if (adev->vce.ring[i].sched.ready)
> + if (adev->vce.ring[i].sched.ready &&
> + !adev->vce.ring[i].no_user_submission)
> ++num_rings;
> ib_start_alignment = 256;
> ib_size_alignment = 4;
> @@ -456,7 +461,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> *adev,
> continue;
>
> for (j = 0; j < adev->uvd.num_enc_rings; j++)
> - if (adev->uvd.inst[i].ring_enc[j].sched.ready)
> + if (adev->uvd.inst[i].ring_enc[j].sched.ready &&
> + !adev->uvd.inst[i].ring_enc[j].no_user_submission)
> ++num_rings;
> }
> ib_start_alignment = 256;
> @@ -468,7 +474,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> *adev,
> if (adev->vcn.harvest_config & (1 << i))
> continue;
>
> - if (adev->vcn.inst[i].ring_dec.sched.ready)
> + if (adev->vcn.inst[i].ring_dec.sched.ready &&
> + !adev->vcn.inst[i].ring_dec.no_user_submission)
> ++num_rings;
> }
> ib_start_alignment = 256;
> @@ -481,7 +488,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> *adev,
> continue;
>
> for (j = 0; j < adev->vcn.inst[i].num_enc_rings; j++)
> - if (adev->vcn.inst[i].ring_enc[j].sched.ready)
> + if (adev->vcn.inst[i].ring_enc[j].sched.ready &&
> + !adev->vcn.inst[i].ring_enc[j].no_user_submission)
> ++num_rings;
> }
> ib_start_alignment = 256;
> @@ -496,7 +504,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> *adev,
> continue;
>
> for (j = 0; j < adev->jpeg.num_jpeg_rings; j++)
> - if (adev->jpeg.inst[i].ring_dec[j].sched.ready)
> + if (adev->jpeg.inst[i].ring_dec[j].sched.ready &&
> + !adev->jpeg.inst[i].ring_dec[j].no_user_submission)
> ++num_rings;
> }
> ib_start_alignment = 256;
> @@ -504,7 +513,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> *adev,
> break;
> case AMDGPU_HW_IP_VPE:
> type = AMD_IP_BLOCK_TYPE_VPE;
> - if (adev->vpe.ring.sched.ready)
> + if (adev->vpe.ring.sched.ready &&
> + !adev->vpe.ring.no_user_submission)
> ++num_rings;
> ib_start_alignment = 256;
> ib_size_alignment = 4;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index b4fd1e17205e9..4a97afcb38b78 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -297,6 +297,7 @@ struct amdgpu_ring {
> struct dma_fence *vmid_wait;
> bool has_compute_vm_bug;
> bool no_scheduler;
> + bool no_user_submission;
> int hw_prio;
> unsigned num_hw_submission;
> atomic_t *sched_score;
> @@ -310,7 +311,6 @@ struct amdgpu_ring {
> unsigned int entry_index;
> /* store the cached rptr to restore after reset */
> uint64_t cached_rptr;
> -
> };
>
> #define amdgpu_ring_parse_cs(r, p, job, ib) ((r)->funcs->parse_cs((p), (job), (ib)))
> --
> 2.48.1
[-- Attachment #2: 0001-winsys-amdgpu-allow-to-submit-userq-at-invalidate-ri.patch --]
[-- Type: application/octet-stream, Size: 1286 bytes --]
From ddf6ea248a0dfb8e5fd28bf7784b416ad7545fc3 Mon Sep 17 00:00:00 2001
From: Prike Liang <Prike.Liang@amd.com>
Date: Mon, 17 Mar 2025 14:32:37 +0800
Subject: [PATCH] winsys/amdgpu: allow to submit userq at invalidate ring_num
In the kernel driver there will disable the gfx kernel ring throughly
when user queue enabled. So the mesa driver should continue to fill
the user queue for the kernel queue disabled case.
Change-Id: I73584225af87d987182470f56cc4dbbe4c451230
Signed-off-by: Prike Liang <Prike.Liang@amd.com>
---
src/gallium/winsys/amdgpu/drm/amdgpu_cs.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_cs.cpp b/src/gallium/winsys/amdgpu/drm/amdgpu_cs.cpp
index beb16ca63beb..75d113428b9c 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_cs.cpp
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_cs.cpp
@@ -941,7 +941,8 @@ amdgpu_cs_create(struct radeon_cmdbuf *rcs,
/* Compute the queue index by counting the IPs that have queues. */
assert(ip_type < ARRAY_SIZE(ctx->aws->info.ip));
- assert(ctx->aws->info.ip[ip_type].num_queues);
+ if (!acs->aws->info.use_userq)
+ assert(ctx->aws->info.ip[ip_type].num_queues);
if (ip_uses_alt_fence(ip_type)) {
acs->queue_index = INT_MAX;
--
2.34.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions
2025-03-17 7:08 ` Liang, Prike
@ 2025-03-17 17:15 ` Marek Olšák
2025-03-17 18:27 ` Alex Deucher
0 siblings, 1 reply; 41+ messages in thread
From: Marek Olšák @ 2025-03-17 17:15 UTC (permalink / raw)
To: Liang, Prike
Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org, Olsak, Marek,
Khatri, Sunil
[-- Attachment #1: Type: text/plain, Size: 9569 bytes --]
Userspace needs a query that a queue IP type is supported.
"available_rings" is used for that right now, but if that's 0, something
else must indicate IP support.
amd_ip_info::num_queues should be non-zero even when user queues are
supported. The exact number doesn't matter with user queues.
Marek
On Mon, Mar 17, 2025 at 3:09 AM Liang, Prike <Prike.Liang@amd.com> wrote:
> [Public]
>
> We might still need to export each ring's number correctly; otherwise, the
> Mesa driver will consider there's no available ring supported from the
> driver and then further assert before submitting the user queue.
>
> If we want to keep the ring number being zero, the Mesa driver may need an
> attachment change to allow the command submitted to the zero-ring number if
> the user queue is enabled.
>
> Hi @Olsak, Marek Do you think it's fine to have the attachment patch for
> the userq support? Except for such changes, maybe we also need to clean up
> the IB-related part.
>
> Regards,
> Prike
>
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> > Deucher
> > Sent: Thursday, March 13, 2025 10:41 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Khatri, Sunil
> > <Sunil.Khatri@amd.com>
> > Subject: [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions
> >
> > This would be set by IPs which only accept submissions from the kernel,
> not
> > userspace, such as when kernel queues are disabled. Don't expose the
> rings to
> > userspace and reject any submissions in the CS IOCTL.
> >
> > Reviewed-by: Sunil Khatri<sunil.khatri@amd.com>
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ++++
> > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 30 ++++++++++++++++--------
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
> > 3 files changed, 25 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > index 5df21529b3b13..5cc18034b75df 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > @@ -349,6 +349,10 @@ static int amdgpu_cs_p2_ib(struct amdgpu_cs_parser
> > *p,
> > ring = amdgpu_job_ring(job);
> > ib = &job->ibs[job->num_ibs++];
> >
> > + /* submissions to kernel queus are disabled */
> > + if (ring->no_user_submission)
> > + return -EINVAL;
> > +
> > /* MM engine doesn't support user fences */
> > if (p->uf_bo && ring->funcs->no_user_fence)
> > return -EINVAL;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > index cd6eb7a3bc58a..3b7dfd56ccd0e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > @@ -408,7 +408,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> > *adev,
> > case AMDGPU_HW_IP_GFX:
> > type = AMD_IP_BLOCK_TYPE_GFX;
> > for (i = 0; i < adev->gfx.num_gfx_rings; i++)
> > - if (adev->gfx.gfx_ring[i].sched.ready)
> > + if (adev->gfx.gfx_ring[i].sched.ready &&
> > + !adev->gfx.gfx_ring[i].no_user_submission)
> > ++num_rings;
> > ib_start_alignment = 32;
> > ib_size_alignment = 32;
> > @@ -416,7 +417,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> > *adev,
> > case AMDGPU_HW_IP_COMPUTE:
> > type = AMD_IP_BLOCK_TYPE_GFX;
> > for (i = 0; i < adev->gfx.num_compute_rings; i++)
> > - if (adev->gfx.compute_ring[i].sched.ready)
> > + if (adev->gfx.compute_ring[i].sched.ready &&
> > + !adev->gfx.compute_ring[i].no_user_submission)
> > ++num_rings;
> > ib_start_alignment = 32;
> > ib_size_alignment = 32;
> > @@ -424,7 +426,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> > *adev,
> > case AMDGPU_HW_IP_DMA:
> > type = AMD_IP_BLOCK_TYPE_SDMA;
> > for (i = 0; i < adev->sdma.num_instances; i++)
> > - if (adev->sdma.instance[i].ring.sched.ready)
> > + if (adev->sdma.instance[i].ring.sched.ready &&
> > + !adev->gfx.gfx_ring[i].no_user_submission)
> > ++num_rings;
> > ib_start_alignment = 256;
> > ib_size_alignment = 4;
> > @@ -435,7 +438,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> > *adev,
> > if (adev->uvd.harvest_config & (1 << i))
> > continue;
> >
> > - if (adev->uvd.inst[i].ring.sched.ready)
> > + if (adev->uvd.inst[i].ring.sched.ready &&
> > + !adev->uvd.inst[i].ring.no_user_submission)
> > ++num_rings;
> > }
> > ib_start_alignment = 256;
> > @@ -444,7 +448,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> > *adev,
> > case AMDGPU_HW_IP_VCE:
> > type = AMD_IP_BLOCK_TYPE_VCE;
> > for (i = 0; i < adev->vce.num_rings; i++)
> > - if (adev->vce.ring[i].sched.ready)
> > + if (adev->vce.ring[i].sched.ready &&
> > + !adev->vce.ring[i].no_user_submission)
> > ++num_rings;
> > ib_start_alignment = 256;
> > ib_size_alignment = 4;
> > @@ -456,7 +461,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> > *adev,
> > continue;
> >
> > for (j = 0; j < adev->uvd.num_enc_rings; j++)
> > - if
> (adev->uvd.inst[i].ring_enc[j].sched.ready)
> > + if
> (adev->uvd.inst[i].ring_enc[j].sched.ready &&
> > +
> !adev->uvd.inst[i].ring_enc[j].no_user_submission)
> > ++num_rings;
> > }
> > ib_start_alignment = 256;
> > @@ -468,7 +474,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> > *adev,
> > if (adev->vcn.harvest_config & (1 << i))
> > continue;
> >
> > - if (adev->vcn.inst[i].ring_dec.sched.ready)
> > + if (adev->vcn.inst[i].ring_dec.sched.ready &&
> > + !adev->vcn.inst[i].ring_dec.no_user_submission)
> > ++num_rings;
> > }
> > ib_start_alignment = 256;
> > @@ -481,7 +488,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> > *adev,
> > continue;
> >
> > for (j = 0; j < adev->vcn.inst[i].num_enc_rings;
> j++)
> > - if
> (adev->vcn.inst[i].ring_enc[j].sched.ready)
> > + if
> (adev->vcn.inst[i].ring_enc[j].sched.ready &&
> > +
> !adev->vcn.inst[i].ring_enc[j].no_user_submission)
> > ++num_rings;
> > }
> > ib_start_alignment = 256;
> > @@ -496,7 +504,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> > *adev,
> > continue;
> >
> > for (j = 0; j < adev->jpeg.num_jpeg_rings; j++)
> > - if
> (adev->jpeg.inst[i].ring_dec[j].sched.ready)
> > + if
> (adev->jpeg.inst[i].ring_dec[j].sched.ready &&
> > +
> !adev->jpeg.inst[i].ring_dec[j].no_user_submission)
> > ++num_rings;
> > }
> > ib_start_alignment = 256;
> > @@ -504,7 +513,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
> > *adev,
> > break;
> > case AMDGPU_HW_IP_VPE:
> > type = AMD_IP_BLOCK_TYPE_VPE;
> > - if (adev->vpe.ring.sched.ready)
> > + if (adev->vpe.ring.sched.ready &&
> > + !adev->vpe.ring.no_user_submission)
> > ++num_rings;
> > ib_start_alignment = 256;
> > ib_size_alignment = 4;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > index b4fd1e17205e9..4a97afcb38b78 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > @@ -297,6 +297,7 @@ struct amdgpu_ring {
> > struct dma_fence *vmid_wait;
> > bool has_compute_vm_bug;
> > bool no_scheduler;
> > + bool no_user_submission;
> > int hw_prio;
> > unsigned num_hw_submission;
> > atomic_t *sched_score;
> > @@ -310,7 +311,6 @@ struct amdgpu_ring {
> > unsigned int entry_index;
> > /* store the cached rptr to restore after reset */
> > uint64_t cached_rptr;
> > -
> > };
> >
> > #define amdgpu_ring_parse_cs(r, p, job, ib) ((r)->funcs->parse_cs((p),
> (job), (ib)))
> > --
> > 2.48.1
>
>
[-- Attachment #2: Type: text/html, Size: 12681 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions
2025-03-17 17:15 ` Marek Olšák
@ 2025-03-17 18:27 ` Alex Deucher
2025-03-17 20:40 ` Marek Olšák
0 siblings, 1 reply; 41+ messages in thread
From: Alex Deucher @ 2025-03-17 18:27 UTC (permalink / raw)
To: Marek Olšák
Cc: Liang, Prike, Deucher, Alexander, amd-gfx@lists.freedesktop.org,
Olsak, Marek, Khatri, Sunil
On Mon, Mar 17, 2025 at 1:23 PM Marek Olšák <maraeo@gmail.com> wrote:
>
> Userspace needs a query that a queue IP type is supported. "available_rings" is used for that right now, but if that's 0, something else must indicate IP support.
>
> amd_ip_info::num_queues should be non-zero even when user queues are supported. The exact number doesn't matter with user queues.
How will mesa determine whether kernel queues are supported? Can mesa
look at amd_ip_info::num_queues and if it's 0, check some new INFO
query to determine if user queues are available? If
amd_ip_info::num_queues is always non-0, then it would be assumed that
the kernel supports kernel queues, which it may not.
Alex
>
> Marek
>
> On Mon, Mar 17, 2025 at 3:09 AM Liang, Prike <Prike.Liang@amd.com> wrote:
>>
>> [Public]
>>
>> We might still need to export each ring's number correctly; otherwise, the Mesa driver will consider there's no available ring supported from the driver and then further assert before submitting the user queue.
>>
>> If we want to keep the ring number being zero, the Mesa driver may need an attachment change to allow the command submitted to the zero-ring number if the user queue is enabled.
>>
>> Hi @Olsak, Marek Do you think it's fine to have the attachment patch for the userq support? Except for such changes, maybe we also need to clean up the IB-related part.
>>
>> Regards,
>> Prike
>>
>> > -----Original Message-----
>> > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
>> > Deucher
>> > Sent: Thursday, March 13, 2025 10:41 PM
>> > To: amd-gfx@lists.freedesktop.org
>> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Khatri, Sunil
>> > <Sunil.Khatri@amd.com>
>> > Subject: [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions
>> >
>> > This would be set by IPs which only accept submissions from the kernel, not
>> > userspace, such as when kernel queues are disabled. Don't expose the rings to
>> > userspace and reject any submissions in the CS IOCTL.
>> >
>> > Reviewed-by: Sunil Khatri<sunil.khatri@amd.com>
>> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> > ---
>> > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ++++
>> > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 30 ++++++++++++++++--------
>> > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
>> > 3 files changed, 25 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> > index 5df21529b3b13..5cc18034b75df 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> > @@ -349,6 +349,10 @@ static int amdgpu_cs_p2_ib(struct amdgpu_cs_parser
>> > *p,
>> > ring = amdgpu_job_ring(job);
>> > ib = &job->ibs[job->num_ibs++];
>> >
>> > + /* submissions to kernel queus are disabled */
>> > + if (ring->no_user_submission)
>> > + return -EINVAL;
>> > +
>> > /* MM engine doesn't support user fences */
>> > if (p->uf_bo && ring->funcs->no_user_fence)
>> > return -EINVAL;
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> > index cd6eb7a3bc58a..3b7dfd56ccd0e 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> > @@ -408,7 +408,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
>> > *adev,
>> > case AMDGPU_HW_IP_GFX:
>> > type = AMD_IP_BLOCK_TYPE_GFX;
>> > for (i = 0; i < adev->gfx.num_gfx_rings; i++)
>> > - if (adev->gfx.gfx_ring[i].sched.ready)
>> > + if (adev->gfx.gfx_ring[i].sched.ready &&
>> > + !adev->gfx.gfx_ring[i].no_user_submission)
>> > ++num_rings;
>> > ib_start_alignment = 32;
>> > ib_size_alignment = 32;
>> > @@ -416,7 +417,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
>> > *adev,
>> > case AMDGPU_HW_IP_COMPUTE:
>> > type = AMD_IP_BLOCK_TYPE_GFX;
>> > for (i = 0; i < adev->gfx.num_compute_rings; i++)
>> > - if (adev->gfx.compute_ring[i].sched.ready)
>> > + if (adev->gfx.compute_ring[i].sched.ready &&
>> > + !adev->gfx.compute_ring[i].no_user_submission)
>> > ++num_rings;
>> > ib_start_alignment = 32;
>> > ib_size_alignment = 32;
>> > @@ -424,7 +426,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
>> > *adev,
>> > case AMDGPU_HW_IP_DMA:
>> > type = AMD_IP_BLOCK_TYPE_SDMA;
>> > for (i = 0; i < adev->sdma.num_instances; i++)
>> > - if (adev->sdma.instance[i].ring.sched.ready)
>> > + if (adev->sdma.instance[i].ring.sched.ready &&
>> > + !adev->gfx.gfx_ring[i].no_user_submission)
>> > ++num_rings;
>> > ib_start_alignment = 256;
>> > ib_size_alignment = 4;
>> > @@ -435,7 +438,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
>> > *adev,
>> > if (adev->uvd.harvest_config & (1 << i))
>> > continue;
>> >
>> > - if (adev->uvd.inst[i].ring.sched.ready)
>> > + if (adev->uvd.inst[i].ring.sched.ready &&
>> > + !adev->uvd.inst[i].ring.no_user_submission)
>> > ++num_rings;
>> > }
>> > ib_start_alignment = 256;
>> > @@ -444,7 +448,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
>> > *adev,
>> > case AMDGPU_HW_IP_VCE:
>> > type = AMD_IP_BLOCK_TYPE_VCE;
>> > for (i = 0; i < adev->vce.num_rings; i++)
>> > - if (adev->vce.ring[i].sched.ready)
>> > + if (adev->vce.ring[i].sched.ready &&
>> > + !adev->vce.ring[i].no_user_submission)
>> > ++num_rings;
>> > ib_start_alignment = 256;
>> > ib_size_alignment = 4;
>> > @@ -456,7 +461,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
>> > *adev,
>> > continue;
>> >
>> > for (j = 0; j < adev->uvd.num_enc_rings; j++)
>> > - if (adev->uvd.inst[i].ring_enc[j].sched.ready)
>> > + if (adev->uvd.inst[i].ring_enc[j].sched.ready &&
>> > + !adev->uvd.inst[i].ring_enc[j].no_user_submission)
>> > ++num_rings;
>> > }
>> > ib_start_alignment = 256;
>> > @@ -468,7 +474,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
>> > *adev,
>> > if (adev->vcn.harvest_config & (1 << i))
>> > continue;
>> >
>> > - if (adev->vcn.inst[i].ring_dec.sched.ready)
>> > + if (adev->vcn.inst[i].ring_dec.sched.ready &&
>> > + !adev->vcn.inst[i].ring_dec.no_user_submission)
>> > ++num_rings;
>> > }
>> > ib_start_alignment = 256;
>> > @@ -481,7 +488,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
>> > *adev,
>> > continue;
>> >
>> > for (j = 0; j < adev->vcn.inst[i].num_enc_rings; j++)
>> > - if (adev->vcn.inst[i].ring_enc[j].sched.ready)
>> > + if (adev->vcn.inst[i].ring_enc[j].sched.ready &&
>> > + !adev->vcn.inst[i].ring_enc[j].no_user_submission)
>> > ++num_rings;
>> > }
>> > ib_start_alignment = 256;
>> > @@ -496,7 +504,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
>> > *adev,
>> > continue;
>> >
>> > for (j = 0; j < adev->jpeg.num_jpeg_rings; j++)
>> > - if (adev->jpeg.inst[i].ring_dec[j].sched.ready)
>> > + if (adev->jpeg.inst[i].ring_dec[j].sched.ready &&
>> > + !adev->jpeg.inst[i].ring_dec[j].no_user_submission)
>> > ++num_rings;
>> > }
>> > ib_start_alignment = 256;
>> > @@ -504,7 +513,8 @@ static int amdgpu_hw_ip_info(struct amdgpu_device
>> > *adev,
>> > break;
>> > case AMDGPU_HW_IP_VPE:
>> > type = AMD_IP_BLOCK_TYPE_VPE;
>> > - if (adev->vpe.ring.sched.ready)
>> > + if (adev->vpe.ring.sched.ready &&
>> > + !adev->vpe.ring.no_user_submission)
>> > ++num_rings;
>> > ib_start_alignment = 256;
>> > ib_size_alignment = 4;
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> > index b4fd1e17205e9..4a97afcb38b78 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> > @@ -297,6 +297,7 @@ struct amdgpu_ring {
>> > struct dma_fence *vmid_wait;
>> > bool has_compute_vm_bug;
>> > bool no_scheduler;
>> > + bool no_user_submission;
>> > int hw_prio;
>> > unsigned num_hw_submission;
>> > atomic_t *sched_score;
>> > @@ -310,7 +311,6 @@ struct amdgpu_ring {
>> > unsigned int entry_index;
>> > /* store the cached rptr to restore after reset */
>> > uint64_t cached_rptr;
>> > -
>> > };
>> >
>> > #define amdgpu_ring_parse_cs(r, p, job, ib) ((r)->funcs->parse_cs((p), (job), (ib)))
>> > --
>> > 2.48.1
>>
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions
2025-03-17 18:27 ` Alex Deucher
@ 2025-03-17 20:40 ` Marek Olšák
0 siblings, 0 replies; 41+ messages in thread
From: Marek Olšák @ 2025-03-17 20:40 UTC (permalink / raw)
To: Alex Deucher
Cc: Liang, Prike, Deucher, Alexander, amd-gfx@lists.freedesktop.org,
Olsak, Marek, Khatri, Sunil
[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]
On Mon, Mar 17, 2025 at 2:27 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> On Mon, Mar 17, 2025 at 1:23 PM Marek Olšák <maraeo@gmail.com> wrote:
> >
> > Userspace needs a query that a queue IP type is supported.
> "available_rings" is used for that right now, but if that's 0, something
> else must indicate IP support.
> >
> > amd_ip_info::num_queues should be non-zero even when user queues are
> supported. The exact number doesn't matter with user queues.
>
> How will mesa determine whether kernel queues are supported? Can mesa
> look at amd_ip_info::num_queues and if it's 0, check some new INFO
> query to determine if user queues are available? If
> amd_ip_info::num_queues is always non-0, then it would be assumed that
> the kernel supports kernel queues, which it may not.
>
That is to be determined. However, num_queues is also used to indicate
whether the IP is supported, so either the meaning of num_queues has to
change, or the way num_queues is set has to change.
Marek
[-- Attachment #2: Type: text/html, Size: 1471 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
2025-03-13 14:41 ` [PATCH 01/11] drm/amdgpu: add parameter to disable kernel queues Alex Deucher
2025-03-13 14:41 ` [PATCH 02/11] drm/amdgpu: add ring flag for no user submissions Alex Deucher
@ 2025-03-13 14:41 ` Alex Deucher
2025-03-14 11:20 ` Khatri, Sunil
2025-03-17 9:06 ` Liang, Prike
2025-03-13 14:41 ` [PATCH 04/11] drm/amdgpu/mes: centralize gfx_hqd mask management Alex Deucher
` (8 subsequent siblings)
11 siblings, 2 replies; 41+ messages in thread
From: Alex Deucher @ 2025-03-13 14:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher
Add proper checks for disable_kq functionality in
gfx helper functions. Add special logic for families
that require the clear state setup.
v2: use ring count as per Felix suggestion
v3: fix num_gfx_rings handling in amdgpu_gfx_graphics_queue_acquire()
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 8 ++++++--
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 ++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 984e6ff6e4632..a08243dd0798e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -258,8 +258,9 @@ void amdgpu_gfx_graphics_queue_acquire(struct amdgpu_device *adev)
}
/* update the number of active graphics rings */
- adev->gfx.num_gfx_rings =
- bitmap_weight(adev->gfx.me.queue_bitmap, AMDGPU_MAX_GFX_QUEUES);
+ if (adev->gfx.num_gfx_rings)
+ adev->gfx.num_gfx_rings =
+ bitmap_weight(adev->gfx.me.queue_bitmap, AMDGPU_MAX_GFX_QUEUES);
}
static int amdgpu_gfx_kiq_acquire(struct amdgpu_device *adev,
@@ -1544,6 +1545,9 @@ static ssize_t amdgpu_gfx_set_run_cleaner_shader(struct device *dev,
if (adev->in_suspend && !adev->in_runpm)
return -EPERM;
+ if (adev->gfx.disable_kq)
+ return -ENOTSUPP;
+
ret = kstrtol(buf, 0, &value);
if (ret)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index ddf4533614bac..8fa68a4ac34f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -483,6 +483,8 @@ struct amdgpu_gfx {
atomic_t total_submission_cnt;
struct delayed_work idle_work;
+
+ bool disable_kq;
};
struct amdgpu_gfx_ras_reg_entry {
--
2.48.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq
2025-03-13 14:41 ` [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq Alex Deucher
@ 2025-03-14 11:20 ` Khatri, Sunil
2025-03-17 9:06 ` Liang, Prike
1 sibling, 0 replies; 41+ messages in thread
From: Khatri, Sunil @ 2025-03-14 11:20 UTC (permalink / raw)
To: Alex Deucher, amd-gfx
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
On 3/13/2025 8:11 PM, Alex Deucher wrote:
> Add proper checks for disable_kq functionality in
> gfx helper functions. Add special logic for families
> that require the clear state setup.
>
> v2: use ring count as per Felix suggestion
> v3: fix num_gfx_rings handling in amdgpu_gfx_graphics_queue_acquire()
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 8 ++++++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 ++
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 984e6ff6e4632..a08243dd0798e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -258,8 +258,9 @@ void amdgpu_gfx_graphics_queue_acquire(struct amdgpu_device *adev)
> }
>
> /* update the number of active graphics rings */
> - adev->gfx.num_gfx_rings =
> - bitmap_weight(adev->gfx.me.queue_bitmap, AMDGPU_MAX_GFX_QUEUES);
> + if (adev->gfx.num_gfx_rings)
> + adev->gfx.num_gfx_rings =
> + bitmap_weight(adev->gfx.me.queue_bitmap, AMDGPU_MAX_GFX_QUEUES);
> }
>
> static int amdgpu_gfx_kiq_acquire(struct amdgpu_device *adev,
> @@ -1544,6 +1545,9 @@ static ssize_t amdgpu_gfx_set_run_cleaner_shader(struct device *dev,
> if (adev->in_suspend && !adev->in_runpm)
> return -EPERM;
>
> + if (adev->gfx.disable_kq)
> + return -ENOTSUPP;
> +
> ret = kstrtol(buf, 0, &value);
>
> if (ret)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index ddf4533614bac..8fa68a4ac34f1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -483,6 +483,8 @@ struct amdgpu_gfx {
>
> atomic_t total_submission_cnt;
> struct delayed_work idle_work;
> +
> + bool disable_kq;
> };
>
> struct amdgpu_gfx_ras_reg_entry {
^ permalink raw reply [flat|nested] 41+ messages in thread* RE: [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq
2025-03-13 14:41 ` [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq Alex Deucher
2025-03-14 11:20 ` Khatri, Sunil
@ 2025-03-17 9:06 ` Liang, Prike
2025-03-17 13:32 ` Alex Deucher
1 sibling, 1 reply; 41+ messages in thread
From: Liang, Prike @ 2025-03-17 9:06 UTC (permalink / raw)
To: Deucher, Alexander, amd-gfx@lists.freedesktop.org; +Cc: Deucher, Alexander
[Public]
When disabling the gfx kernel queue, then the related ring function callback should be unassigned, and the clean shader callback should also not be further invoked. To avoid the clean shader resource allocated, we may need to drop the shader clean initialized at *_ sw_init() when disable gfx kernel queue.
Regards,
Prike
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> Deucher
> Sent: Thursday, March 13, 2025 10:41 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq
>
> Add proper checks for disable_kq functionality in gfx helper functions. Add special
> logic for families that require the clear state setup.
>
> v2: use ring count as per Felix suggestion
> v3: fix num_gfx_rings handling in amdgpu_gfx_graphics_queue_acquire()
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 8 ++++++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 ++
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 984e6ff6e4632..a08243dd0798e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -258,8 +258,9 @@ void amdgpu_gfx_graphics_queue_acquire(struct
> amdgpu_device *adev)
> }
>
> /* update the number of active graphics rings */
> - adev->gfx.num_gfx_rings =
> - bitmap_weight(adev->gfx.me.queue_bitmap,
> AMDGPU_MAX_GFX_QUEUES);
> + if (adev->gfx.num_gfx_rings)
> + adev->gfx.num_gfx_rings =
> + bitmap_weight(adev->gfx.me.queue_bitmap,
> AMDGPU_MAX_GFX_QUEUES);
> }
>
> static int amdgpu_gfx_kiq_acquire(struct amdgpu_device *adev, @@ -1544,6
> +1545,9 @@ static ssize_t amdgpu_gfx_set_run_cleaner_shader(struct device
> *dev,
> if (adev->in_suspend && !adev->in_runpm)
> return -EPERM;
>
> + if (adev->gfx.disable_kq)
> + return -ENOTSUPP;
> +
> ret = kstrtol(buf, 0, &value);
>
> if (ret)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index ddf4533614bac..8fa68a4ac34f1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -483,6 +483,8 @@ struct amdgpu_gfx {
>
> atomic_t total_submission_cnt;
> struct delayed_work idle_work;
> +
> + bool disable_kq;
> };
>
> struct amdgpu_gfx_ras_reg_entry {
> --
> 2.48.1
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq
2025-03-17 9:06 ` Liang, Prike
@ 2025-03-17 13:32 ` Alex Deucher
2025-03-18 12:23 ` Liang, Prike
0 siblings, 1 reply; 41+ messages in thread
From: Alex Deucher @ 2025-03-17 13:32 UTC (permalink / raw)
To: Liang, Prike; +Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org
On Mon, Mar 17, 2025 at 5:07 AM Liang, Prike <Prike.Liang@amd.com> wrote:
>
> [Public]
>
> When disabling the gfx kernel queue, then the related ring function callback should be unassigned, and the clean shader callback should also not be further invoked. To avoid the clean shader resource allocated, we may need to drop the shader clean initialized at *_ sw_init() when disable gfx kernel queue.
This patch prevents the cleaner shader from being run via sysfs
already. We still need to allocate the cleaner shader resources
though because they are used by the MES and CP firmware for user
queues.
Alex
>
> Regards,
> Prike
>
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> > Deucher
> > Sent: Thursday, March 13, 2025 10:41 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> > Subject: [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq
> >
> > Add proper checks for disable_kq functionality in gfx helper functions. Add special
> > logic for families that require the clear state setup.
> >
> > v2: use ring count as per Felix suggestion
> > v3: fix num_gfx_rings handling in amdgpu_gfx_graphics_queue_acquire()
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 8 ++++++--
> > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 ++
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > index 984e6ff6e4632..a08243dd0798e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > @@ -258,8 +258,9 @@ void amdgpu_gfx_graphics_queue_acquire(struct
> > amdgpu_device *adev)
> > }
> >
> > /* update the number of active graphics rings */
> > - adev->gfx.num_gfx_rings =
> > - bitmap_weight(adev->gfx.me.queue_bitmap,
> > AMDGPU_MAX_GFX_QUEUES);
> > + if (adev->gfx.num_gfx_rings)
> > + adev->gfx.num_gfx_rings =
> > + bitmap_weight(adev->gfx.me.queue_bitmap,
> > AMDGPU_MAX_GFX_QUEUES);
> > }
> >
> > static int amdgpu_gfx_kiq_acquire(struct amdgpu_device *adev, @@ -1544,6
> > +1545,9 @@ static ssize_t amdgpu_gfx_set_run_cleaner_shader(struct device
> > *dev,
> > if (adev->in_suspend && !adev->in_runpm)
> > return -EPERM;
> >
> > + if (adev->gfx.disable_kq)
> > + return -ENOTSUPP;
> > +
> > ret = kstrtol(buf, 0, &value);
> >
> > if (ret)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > index ddf4533614bac..8fa68a4ac34f1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > @@ -483,6 +483,8 @@ struct amdgpu_gfx {
> >
> > atomic_t total_submission_cnt;
> > struct delayed_work idle_work;
> > +
> > + bool disable_kq;
> > };
> >
> > struct amdgpu_gfx_ras_reg_entry {
> > --
> > 2.48.1
>
^ permalink raw reply [flat|nested] 41+ messages in thread* RE: [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq
2025-03-17 13:32 ` Alex Deucher
@ 2025-03-18 12:23 ` Liang, Prike
0 siblings, 0 replies; 41+ messages in thread
From: Liang, Prike @ 2025-03-18 12:23 UTC (permalink / raw)
To: Alex Deucher; +Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org
[Public]
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Monday, March 17, 2025 9:33 PM
> To: Liang, Prike <Prike.Liang@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
> gfx@lists.freedesktop.org
> Subject: Re: [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq
>
> On Mon, Mar 17, 2025 at 5:07 AM Liang, Prike <Prike.Liang@amd.com> wrote:
> >
> > [Public]
> >
> > When disabling the gfx kernel queue, then the related ring function callback
> should be unassigned, and the clean shader callback should also not be further
> invoked. To avoid the clean shader resource allocated, we may need to drop the
> shader clean initialized at *_ sw_init() when disable gfx kernel queue.
>
> This patch prevents the cleaner shader from being run via sysfs already. We still
> need to allocate the cleaner shader resources though because they are used by
> the MES and CP firmware for user queues.
>
> Alex
Yeah, I revisit the enable_cleaner_shader flag usage logic, and the driver requires it to isolate between the GFX and compute processes.
Reviewed-by: Prike Liang <Prike.Liang@amd.com>
> >
> > Regards,
> > Prike
> >
> > > -----Original Message-----
> > > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> > > Alex Deucher
> > > Sent: Thursday, March 13, 2025 10:41 PM
> > > To: amd-gfx@lists.freedesktop.org
> > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> > > Subject: [PATCH 03/11] drm/amdgpu/gfx: add generic handling for
> > > disable_kq
> > >
> > > Add proper checks for disable_kq functionality in gfx helper
> > > functions. Add special logic for families that require the clear state setup.
> > >
> > > v2: use ring count as per Felix suggestion
> > > v3: fix num_gfx_rings handling in
> > > amdgpu_gfx_graphics_queue_acquire()
> > >
> > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 8 ++++++--
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 ++
> > > 2 files changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > > index 984e6ff6e4632..a08243dd0798e 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > > @@ -258,8 +258,9 @@ void amdgpu_gfx_graphics_queue_acquire(struct
> > > amdgpu_device *adev)
> > > }
> > >
> > > /* update the number of active graphics rings */
> > > - adev->gfx.num_gfx_rings =
> > > - bitmap_weight(adev->gfx.me.queue_bitmap,
> > > AMDGPU_MAX_GFX_QUEUES);
> > > + if (adev->gfx.num_gfx_rings)
> > > + adev->gfx.num_gfx_rings =
> > > + bitmap_weight(adev->gfx.me.queue_bitmap,
> > > AMDGPU_MAX_GFX_QUEUES);
> > > }
> > >
> > > static int amdgpu_gfx_kiq_acquire(struct amdgpu_device *adev, @@
> > > -1544,6
> > > +1545,9 @@ static ssize_t amdgpu_gfx_set_run_cleaner_shader(struct
> > > +device
> > > *dev,
> > > if (adev->in_suspend && !adev->in_runpm)
> > > return -EPERM;
> > >
> > > + if (adev->gfx.disable_kq)
> > > + return -ENOTSUPP;
> > > +
> > > ret = kstrtol(buf, 0, &value);
> > >
> > > if (ret)
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > > index ddf4533614bac..8fa68a4ac34f1 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > > @@ -483,6 +483,8 @@ struct amdgpu_gfx {
> > >
> > > atomic_t total_submission_cnt;
> > > struct delayed_work idle_work;
> > > +
> > > + bool disable_kq;
> > > };
> > >
> > > struct amdgpu_gfx_ras_reg_entry {
> > > --
> > > 2.48.1
> >
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 04/11] drm/amdgpu/mes: centralize gfx_hqd mask management
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
` (2 preceding siblings ...)
2025-03-13 14:41 ` [PATCH 03/11] drm/amdgpu/gfx: add generic handling for disable_kq Alex Deucher
@ 2025-03-13 14:41 ` Alex Deucher
2025-03-19 6:12 ` Liang, Prike
2025-03-13 14:41 ` [PATCH 05/11] drm/amdgpu/mes: update hqd masks when disable_kq is set Alex Deucher
` (7 subsequent siblings)
11 siblings, 1 reply; 41+ messages in thread
From: Alex Deucher @ 2025-03-13 14:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Sunil Khatri
Move it to amdgpu_mes to align with the compute and
sdma hqd masks. No functional change.
v2: rebase on new changes
Reviewed-by: Sunil Khatri<sunil.khatri@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 22 ++++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 16 +++-------------
drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 15 +++------------
3 files changed, 28 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index 6f5e272d7ded3..5abc1ca0fee98 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -108,6 +108,28 @@ int amdgpu_mes_init(struct amdgpu_device *adev)
adev->mes.vmid_mask_mmhub = 0xffffff00;
adev->mes.vmid_mask_gfxhub = 0xffffff00;
+ for (i = 0; i < AMDGPU_MES_MAX_GFX_PIPES; i++) {
+ /* use only 1st ME pipe */
+ if (i >= adev->gfx.me.num_pipe_per_me)
+ continue;
+ if (amdgpu_ip_version(adev, GC_HWIP, 0) >=
+ IP_VERSION(12, 0, 0))
+ /*
+ * GFX V12 has only one GFX pipe, but 8 queues in it.
+ * GFX pipe 0 queue 0 is being used by Kernel queue.
+ * Set GFX pipe 0 queue 1-7 for MES scheduling
+ * mask = 1111 1110b
+ */
+ adev->mes.gfx_hqd_mask[i] = 0xFE;
+ else
+ /*
+ * GFX pipe 0 queue 0 is being used by Kernel queue.
+ * Set GFX pipe 0 queue 1 for MES scheduling
+ * mask = 10b
+ */
+ adev->mes.gfx_hqd_mask[i] = 0x2;
+ }
+
for (i = 0; i < AMDGPU_MES_MAX_COMPUTE_PIPES; i++) {
/* use only 1st MEC pipes */
if (i >= adev->gfx.mec.num_pipe_per_mec)
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
index a569d09a1a748..39b45d8b5f049 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -669,18 +669,6 @@ static int mes_v11_0_misc_op(struct amdgpu_mes *mes,
offsetof(union MESAPI__MISC, api_status));
}
-static void mes_v11_0_set_gfx_hqd_mask(union MESAPI_SET_HW_RESOURCES *pkt)
-{
- /*
- * GFX pipe 0 queue 0 is being used by Kernel queue.
- * Set GFX pipe 0 queue 1 for MES scheduling
- * mask = 10b
- * GFX pipe 1 can't be used for MES due to HW limitation.
- */
- pkt->gfx_hqd_mask[0] = 0x2;
- pkt->gfx_hqd_mask[1] = 0;
-}
-
static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes)
{
int i;
@@ -705,7 +693,9 @@ static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes)
mes_set_hw_res_pkt.compute_hqd_mask[i] =
mes->compute_hqd_mask[i];
- mes_v11_0_set_gfx_hqd_mask(&mes_set_hw_res_pkt);
+ for (i = 0; i < MAX_GFX_PIPES; i++)
+ mes_set_hw_res_pkt.gfx_hqd_mask[i] =
+ mes->gfx_hqd_mask[i];
for (i = 0; i < MAX_SDMA_PIPES; i++)
mes_set_hw_res_pkt.sdma_hqd_mask[i] = mes->sdma_hqd_mask[i];
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
index 96336652d14c5..519f054bec60d 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
@@ -694,17 +694,6 @@ static int mes_v12_0_set_hw_resources_1(struct amdgpu_mes *mes, int pipe)
offsetof(union MESAPI_SET_HW_RESOURCES_1, api_status));
}
-static void mes_v12_0_set_gfx_hqd_mask(union MESAPI_SET_HW_RESOURCES *pkt)
-{
- /*
- * GFX V12 has only one GFX pipe, but 8 queues in it.
- * GFX pipe 0 queue 0 is being used by Kernel queue.
- * Set GFX pipe 0 queue 1-7 for MES scheduling
- * mask = 1111 1110b
- */
- pkt->gfx_hqd_mask[0] = 0xFE;
-}
-
static int mes_v12_0_set_hw_resources(struct amdgpu_mes *mes, int pipe)
{
int i;
@@ -727,7 +716,9 @@ static int mes_v12_0_set_hw_resources(struct amdgpu_mes *mes, int pipe)
mes_set_hw_res_pkt.compute_hqd_mask[i] =
mes->compute_hqd_mask[i];
- mes_v12_0_set_gfx_hqd_mask(&mes_set_hw_res_pkt);
+ for (i = 0; i < MAX_GFX_PIPES; i++)
+ mes_set_hw_res_pkt.gfx_hqd_mask[i] =
+ mes->gfx_hqd_mask[i];
for (i = 0; i < MAX_SDMA_PIPES; i++)
mes_set_hw_res_pkt.sdma_hqd_mask[i] =
--
2.48.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* RE: [PATCH 04/11] drm/amdgpu/mes: centralize gfx_hqd mask management
2025-03-13 14:41 ` [PATCH 04/11] drm/amdgpu/mes: centralize gfx_hqd mask management Alex Deucher
@ 2025-03-19 6:12 ` Liang, Prike
2025-03-19 13:46 ` Alex Deucher
0 siblings, 1 reply; 41+ messages in thread
From: Liang, Prike @ 2025-03-19 6:12 UTC (permalink / raw)
To: Deucher, Alexander, amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander, Khatri, Sunil
[Public]
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> Deucher
> Sent: Thursday, March 13, 2025 10:41 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Khatri, Sunil
> <Sunil.Khatri@amd.com>
> Subject: [PATCH 04/11] drm/amdgpu/mes: centralize gfx_hqd mask management
>
> Move it to amdgpu_mes to align with the compute and sdma hqd masks. No
> functional change.
>
> v2: rebase on new changes
>
> Reviewed-by: Sunil Khatri<sunil.khatri@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 22 ++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 16 +++-------------
> drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 15 +++------------
> 3 files changed, 28 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> index 6f5e272d7ded3..5abc1ca0fee98 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> @@ -108,6 +108,28 @@ int amdgpu_mes_init(struct amdgpu_device *adev)
> adev->mes.vmid_mask_mmhub = 0xffffff00;
> adev->mes.vmid_mask_gfxhub = 0xffffff00;
>
> + for (i = 0; i < AMDGPU_MES_MAX_GFX_PIPES; i++) {
> + /* use only 1st ME pipe */
> + if (i >= adev->gfx.me.num_pipe_per_me)
> + continue;
If only requires setting the 1st pipe MES mask here, it seems unnecessary to continue the rest setting loop.
Other than that, the patch is Reviewed-by: Prike Liang <Prike.Liang@amd.com>.
Thanks,
Prike
> + if (amdgpu_ip_version(adev, GC_HWIP, 0) >=
> + IP_VERSION(12, 0, 0))
> + /*
> + * GFX V12 has only one GFX pipe, but 8 queues in it.
> + * GFX pipe 0 queue 0 is being used by Kernel queue.
> + * Set GFX pipe 0 queue 1-7 for MES scheduling
> + * mask = 1111 1110b
> + */
> + adev->mes.gfx_hqd_mask[i] = 0xFE;
> + else
> + /*
> + * GFX pipe 0 queue 0 is being used by Kernel queue.
> + * Set GFX pipe 0 queue 1 for MES scheduling
> + * mask = 10b
> + */
> + adev->mes.gfx_hqd_mask[i] = 0x2;
> + }
> +
> for (i = 0; i < AMDGPU_MES_MAX_COMPUTE_PIPES; i++) {
> /* use only 1st MEC pipes */
> if (i >= adev->gfx.mec.num_pipe_per_mec) diff --git
> a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> index a569d09a1a748..39b45d8b5f049 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> @@ -669,18 +669,6 @@ static int mes_v11_0_misc_op(struct amdgpu_mes
> *mes,
> offsetof(union MESAPI__MISC, api_status)); }
>
> -static void mes_v11_0_set_gfx_hqd_mask(union
> MESAPI_SET_HW_RESOURCES *pkt) -{
> - /*
> - * GFX pipe 0 queue 0 is being used by Kernel queue.
> - * Set GFX pipe 0 queue 1 for MES scheduling
> - * mask = 10b
> - * GFX pipe 1 can't be used for MES due to HW limitation.
> - */
> - pkt->gfx_hqd_mask[0] = 0x2;
> - pkt->gfx_hqd_mask[1] = 0;
> -}
> -
> static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes) {
> int i;
> @@ -705,7 +693,9 @@ static int mes_v11_0_set_hw_resources(struct
> amdgpu_mes *mes)
> mes_set_hw_res_pkt.compute_hqd_mask[i] =
> mes->compute_hqd_mask[i];
>
> - mes_v11_0_set_gfx_hqd_mask(&mes_set_hw_res_pkt);
> + for (i = 0; i < MAX_GFX_PIPES; i++)
> + mes_set_hw_res_pkt.gfx_hqd_mask[i] =
> + mes->gfx_hqd_mask[i];
>
> for (i = 0; i < MAX_SDMA_PIPES; i++)
> mes_set_hw_res_pkt.sdma_hqd_mask[i] = mes-
> >sdma_hqd_mask[i]; diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> index 96336652d14c5..519f054bec60d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> @@ -694,17 +694,6 @@ static int mes_v12_0_set_hw_resources_1(struct
> amdgpu_mes *mes, int pipe)
> offsetof(union MESAPI_SET_HW_RESOURCES_1,
> api_status)); }
>
> -static void mes_v12_0_set_gfx_hqd_mask(union
> MESAPI_SET_HW_RESOURCES *pkt) -{
> - /*
> - * GFX V12 has only one GFX pipe, but 8 queues in it.
> - * GFX pipe 0 queue 0 is being used by Kernel queue.
> - * Set GFX pipe 0 queue 1-7 for MES scheduling
> - * mask = 1111 1110b
> - */
> - pkt->gfx_hqd_mask[0] = 0xFE;
> -}
> -
> static int mes_v12_0_set_hw_resources(struct amdgpu_mes *mes, int pipe) {
> int i;
> @@ -727,7 +716,9 @@ static int mes_v12_0_set_hw_resources(struct
> amdgpu_mes *mes, int pipe)
> mes_set_hw_res_pkt.compute_hqd_mask[i] =
> mes->compute_hqd_mask[i];
>
> - mes_v12_0_set_gfx_hqd_mask(&mes_set_hw_res_pkt);
> + for (i = 0; i < MAX_GFX_PIPES; i++)
> + mes_set_hw_res_pkt.gfx_hqd_mask[i] =
> + mes->gfx_hqd_mask[i];
>
> for (i = 0; i < MAX_SDMA_PIPES; i++)
> mes_set_hw_res_pkt.sdma_hqd_mask[i] =
> --
> 2.48.1
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 04/11] drm/amdgpu/mes: centralize gfx_hqd mask management
2025-03-19 6:12 ` Liang, Prike
@ 2025-03-19 13:46 ` Alex Deucher
0 siblings, 0 replies; 41+ messages in thread
From: Alex Deucher @ 2025-03-19 13:46 UTC (permalink / raw)
To: Liang, Prike
Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org, Khatri, Sunil
On Wed, Mar 19, 2025 at 2:12 AM Liang, Prike <Prike.Liang@amd.com> wrote:
>
> [Public]
>
> > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> > Deucher
> > Sent: Thursday, March 13, 2025 10:41 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Khatri, Sunil
> > <Sunil.Khatri@amd.com>
> > Subject: [PATCH 04/11] drm/amdgpu/mes: centralize gfx_hqd mask management
> >
> > Move it to amdgpu_mes to align with the compute and sdma hqd masks. No
> > functional change.
> >
> > v2: rebase on new changes
> >
> > Reviewed-by: Sunil Khatri<sunil.khatri@amd.com>
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 22 ++++++++++++++++++++++
> > drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 16 +++-------------
> > drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 15 +++------------
> > 3 files changed, 28 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> > index 6f5e272d7ded3..5abc1ca0fee98 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> > @@ -108,6 +108,28 @@ int amdgpu_mes_init(struct amdgpu_device *adev)
> > adev->mes.vmid_mask_mmhub = 0xffffff00;
> > adev->mes.vmid_mask_gfxhub = 0xffffff00;
> >
> > + for (i = 0; i < AMDGPU_MES_MAX_GFX_PIPES; i++) {
> > + /* use only 1st ME pipe */
> > + if (i >= adev->gfx.me.num_pipe_per_me)
> > + continue;
> If only requires setting the 1st pipe MES mask here, it seems unnecessary to continue the rest setting loop.
oh, yeah, this should be break. Same for compute.
Alex
> Other than that, the patch is Reviewed-by: Prike Liang <Prike.Liang@amd.com>.
>
> Thanks,
> Prike
> > + if (amdgpu_ip_version(adev, GC_HWIP, 0) >=
> > + IP_VERSION(12, 0, 0))
> > + /*
> > + * GFX V12 has only one GFX pipe, but 8 queues in it.
> > + * GFX pipe 0 queue 0 is being used by Kernel queue.
> > + * Set GFX pipe 0 queue 1-7 for MES scheduling
> > + * mask = 1111 1110b
> > + */
> > + adev->mes.gfx_hqd_mask[i] = 0xFE;
> > + else
> > + /*
> > + * GFX pipe 0 queue 0 is being used by Kernel queue.
> > + * Set GFX pipe 0 queue 1 for MES scheduling
> > + * mask = 10b
> > + */
> > + adev->mes.gfx_hqd_mask[i] = 0x2;
> > + }
> > +
> > for (i = 0; i < AMDGPU_MES_MAX_COMPUTE_PIPES; i++) {
> > /* use only 1st MEC pipes */
> > if (i >= adev->gfx.mec.num_pipe_per_mec) diff --git
> > a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> > b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> > index a569d09a1a748..39b45d8b5f049 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> > @@ -669,18 +669,6 @@ static int mes_v11_0_misc_op(struct amdgpu_mes
> > *mes,
> > offsetof(union MESAPI__MISC, api_status)); }
> >
> > -static void mes_v11_0_set_gfx_hqd_mask(union
> > MESAPI_SET_HW_RESOURCES *pkt) -{
> > - /*
> > - * GFX pipe 0 queue 0 is being used by Kernel queue.
> > - * Set GFX pipe 0 queue 1 for MES scheduling
> > - * mask = 10b
> > - * GFX pipe 1 can't be used for MES due to HW limitation.
> > - */
> > - pkt->gfx_hqd_mask[0] = 0x2;
> > - pkt->gfx_hqd_mask[1] = 0;
> > -}
> > -
> > static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes) {
> > int i;
> > @@ -705,7 +693,9 @@ static int mes_v11_0_set_hw_resources(struct
> > amdgpu_mes *mes)
> > mes_set_hw_res_pkt.compute_hqd_mask[i] =
> > mes->compute_hqd_mask[i];
> >
> > - mes_v11_0_set_gfx_hqd_mask(&mes_set_hw_res_pkt);
> > + for (i = 0; i < MAX_GFX_PIPES; i++)
> > + mes_set_hw_res_pkt.gfx_hqd_mask[i] =
> > + mes->gfx_hqd_mask[i];
> >
> > for (i = 0; i < MAX_SDMA_PIPES; i++)
> > mes_set_hw_res_pkt.sdma_hqd_mask[i] = mes-
> > >sdma_hqd_mask[i]; diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> > b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> > index 96336652d14c5..519f054bec60d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> > @@ -694,17 +694,6 @@ static int mes_v12_0_set_hw_resources_1(struct
> > amdgpu_mes *mes, int pipe)
> > offsetof(union MESAPI_SET_HW_RESOURCES_1,
> > api_status)); }
> >
> > -static void mes_v12_0_set_gfx_hqd_mask(union
> > MESAPI_SET_HW_RESOURCES *pkt) -{
> > - /*
> > - * GFX V12 has only one GFX pipe, but 8 queues in it.
> > - * GFX pipe 0 queue 0 is being used by Kernel queue.
> > - * Set GFX pipe 0 queue 1-7 for MES scheduling
> > - * mask = 1111 1110b
> > - */
> > - pkt->gfx_hqd_mask[0] = 0xFE;
> > -}
> > -
> > static int mes_v12_0_set_hw_resources(struct amdgpu_mes *mes, int pipe) {
> > int i;
> > @@ -727,7 +716,9 @@ static int mes_v12_0_set_hw_resources(struct
> > amdgpu_mes *mes, int pipe)
> > mes_set_hw_res_pkt.compute_hqd_mask[i] =
> > mes->compute_hqd_mask[i];
> >
> > - mes_v12_0_set_gfx_hqd_mask(&mes_set_hw_res_pkt);
> > + for (i = 0; i < MAX_GFX_PIPES; i++)
> > + mes_set_hw_res_pkt.gfx_hqd_mask[i] =
> > + mes->gfx_hqd_mask[i];
> >
> > for (i = 0; i < MAX_SDMA_PIPES; i++)
> > mes_set_hw_res_pkt.sdma_hqd_mask[i] =
> > --
> > 2.48.1
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 05/11] drm/amdgpu/mes: update hqd masks when disable_kq is set
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
` (3 preceding siblings ...)
2025-03-13 14:41 ` [PATCH 04/11] drm/amdgpu/mes: centralize gfx_hqd mask management Alex Deucher
@ 2025-03-13 14:41 ` Alex Deucher
2025-03-13 14:41 ` [PATCH 06/11] drm/amdgpu/mes: make more vmids available when disable_kq=1 Alex Deucher
` (6 subsequent siblings)
11 siblings, 0 replies; 41+ messages in thread
From: Alex Deucher @ 2025-03-13 14:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Sunil Khatri
Make all resources available to user queues.
Suggested-by: Sunil Khatri <sunil.khatri@amd.com>
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index 5abc1ca0fee98..971bf01fe46a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -120,21 +120,21 @@ int amdgpu_mes_init(struct amdgpu_device *adev)
* Set GFX pipe 0 queue 1-7 for MES scheduling
* mask = 1111 1110b
*/
- adev->mes.gfx_hqd_mask[i] = 0xFE;
+ adev->mes.gfx_hqd_mask[i] = adev->gfx.disable_kq ? 0xFF : 0xFE;
else
/*
* GFX pipe 0 queue 0 is being used by Kernel queue.
* Set GFX pipe 0 queue 1 for MES scheduling
* mask = 10b
*/
- adev->mes.gfx_hqd_mask[i] = 0x2;
+ adev->mes.gfx_hqd_mask[i] = adev->gfx.disable_kq ? 0x3 : 0x2;
}
for (i = 0; i < AMDGPU_MES_MAX_COMPUTE_PIPES; i++) {
/* use only 1st MEC pipes */
if (i >= adev->gfx.mec.num_pipe_per_mec)
continue;
- adev->mes.compute_hqd_mask[i] = 0xc;
+ adev->mes.compute_hqd_mask[i] = adev->gfx.disable_kq ? 0xF : 0xC;
}
for (i = 0; i < AMDGPU_MES_MAX_SDMA_PIPES; i++) {
--
2.48.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 06/11] drm/amdgpu/mes: make more vmids available when disable_kq=1
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
` (4 preceding siblings ...)
2025-03-13 14:41 ` [PATCH 05/11] drm/amdgpu/mes: update hqd masks when disable_kq is set Alex Deucher
@ 2025-03-13 14:41 ` Alex Deucher
2025-03-19 9:02 ` Liang, Prike
2025-03-13 14:41 ` [PATCH 07/11] drm/amdgpu/gfx11: add support for disable_kq Alex Deucher
` (5 subsequent siblings)
11 siblings, 1 reply; 41+ messages in thread
From: Alex Deucher @ 2025-03-13 14:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Sunil Khatri
If we don't have kernel queues, the vmids can be used by
the MES for user queues.
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 2 +-
drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index 971bf01fe46a9..a536a78342a09 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -106,7 +106,7 @@ int amdgpu_mes_init(struct amdgpu_device *adev)
adev->mes.total_max_queue = AMDGPU_FENCE_MES_QUEUE_ID_MASK;
adev->mes.vmid_mask_mmhub = 0xffffff00;
- adev->mes.vmid_mask_gfxhub = 0xffffff00;
+ adev->mes.vmid_mask_gfxhub = adev->gfx.disable_kq ? 0xfffffffe : 0xffffff00;
for (i = 0; i < AMDGPU_MES_MAX_GFX_PIPES; i++) {
/* use only 1st ME pipe */
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 95d894a231fcf..19a5f196829f3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -900,7 +900,7 @@ static int gmc_v10_0_sw_init(struct amdgpu_ip_block *ip_block)
* amdgpu graphics/compute will use VMIDs 1-7
* amdkfd will use VMIDs 8-15
*/
- adev->vm_manager.first_kfd_vmid = 8;
+ adev->vm_manager.first_kfd_vmid = adev->gfx.disable_kq ? 1 : 8;
amdgpu_vm_manager_init(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
index bf8d01da88154..a2f6c9f4ebf2f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
@@ -838,7 +838,7 @@ static int gmc_v12_0_sw_init(struct amdgpu_ip_block *ip_block)
* amdgpu graphics/compute will use VMIDs 1-7
* amdkfd will use VMIDs 8-15
*/
- adev->vm_manager.first_kfd_vmid = 8;
+ adev->vm_manager.first_kfd_vmid = adev->gfx.disable_kq ? 1 : 8;
amdgpu_vm_manager_init(adev);
--
2.48.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* RE: [PATCH 06/11] drm/amdgpu/mes: make more vmids available when disable_kq=1
2025-03-13 14:41 ` [PATCH 06/11] drm/amdgpu/mes: make more vmids available when disable_kq=1 Alex Deucher
@ 2025-03-19 9:02 ` Liang, Prike
2025-03-19 13:09 ` Alex Deucher
0 siblings, 1 reply; 41+ messages in thread
From: Liang, Prike @ 2025-03-19 9:02 UTC (permalink / raw)
To: Deucher, Alexander, amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander, Khatri, Sunil
[Public]
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> Deucher
> Sent: Thursday, March 13, 2025 10:42 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Khatri, Sunil
> <Sunil.Khatri@amd.com>
> Subject: [PATCH 06/11] drm/amdgpu/mes: make more vmids available when
> disable_kq=1
>
> If we don't have kernel queues, the vmids can be used by the MES for user
> queues.
>
> Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> index 971bf01fe46a9..a536a78342a09 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> @@ -106,7 +106,7 @@ int amdgpu_mes_init(struct amdgpu_device *adev)
>
> adev->mes.total_max_queue =
> AMDGPU_FENCE_MES_QUEUE_ID_MASK;
> adev->mes.vmid_mask_mmhub = 0xffffff00;
> - adev->mes.vmid_mask_gfxhub = 0xffffff00;
> + adev->mes.vmid_mask_gfxhub = adev->gfx.disable_kq ? 0xfffffffe :
> +0xffffff00;
>
Question: Why here only set some of the GCHUB VMID to validate? Is that VMID reserved for other specific usage? While it seems vmid_mask_mmhub is not further used in the MES FW?
Anway this patch is: Acked-by: Prike Liang <Prike.Liang@amd.com>
> for (i = 0; i < AMDGPU_MES_MAX_GFX_PIPES; i++) {
> /* use only 1st ME pipe */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 95d894a231fcf..19a5f196829f3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -900,7 +900,7 @@ static int gmc_v10_0_sw_init(struct amdgpu_ip_block
> *ip_block)
> * amdgpu graphics/compute will use VMIDs 1-7
> * amdkfd will use VMIDs 8-15
> */
> - adev->vm_manager.first_kfd_vmid = 8;
> + adev->vm_manager.first_kfd_vmid = adev->gfx.disable_kq ? 1 : 8;
>
> amdgpu_vm_manager_init(adev);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> index bf8d01da88154..a2f6c9f4ebf2f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> @@ -838,7 +838,7 @@ static int gmc_v12_0_sw_init(struct amdgpu_ip_block
> *ip_block)
> * amdgpu graphics/compute will use VMIDs 1-7
> * amdkfd will use VMIDs 8-15
> */
> - adev->vm_manager.first_kfd_vmid = 8;
> + adev->vm_manager.first_kfd_vmid = adev->gfx.disable_kq ? 1 : 8;
>
> amdgpu_vm_manager_init(adev);
>
> --
> 2.48.1
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 06/11] drm/amdgpu/mes: make more vmids available when disable_kq=1
2025-03-19 9:02 ` Liang, Prike
@ 2025-03-19 13:09 ` Alex Deucher
0 siblings, 0 replies; 41+ messages in thread
From: Alex Deucher @ 2025-03-19 13:09 UTC (permalink / raw)
To: Liang, Prike
Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org, Khatri, Sunil
On Wed, Mar 19, 2025 at 5:03 AM Liang, Prike <Prike.Liang@amd.com> wrote:
>
> [Public]
>
> > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> > Deucher
> > Sent: Thursday, March 13, 2025 10:42 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Khatri, Sunil
> > <Sunil.Khatri@amd.com>
> > Subject: [PATCH 06/11] drm/amdgpu/mes: make more vmids available when
> > disable_kq=1
> >
> > If we don't have kernel queues, the vmids can be used by the MES for user
> > queues.
> >
> > Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c | 2 +-
> > 3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> > index 971bf01fe46a9..a536a78342a09 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> > @@ -106,7 +106,7 @@ int amdgpu_mes_init(struct amdgpu_device *adev)
> >
> > adev->mes.total_max_queue =
> > AMDGPU_FENCE_MES_QUEUE_ID_MASK;
> > adev->mes.vmid_mask_mmhub = 0xffffff00;
> > - adev->mes.vmid_mask_gfxhub = 0xffffff00;
> > + adev->mes.vmid_mask_gfxhub = adev->gfx.disable_kq ? 0xfffffffe :
> > +0xffffff00;
> >
> Question: Why here only set some of the GCHUB VMID to validate? Is that VMID reserved for other specific usage? While it seems vmid_mask_mmhub is not further used in the MES FW?
None of the engines managed by MES use mmhub. GFX, compute, and SDMA
are all gchub clients. I suppose we could actually set the mmhub mask
to 0. Driver manages mmhub for VCN, VPE, etc. When we eventually
enable the umsched for VCN, VPE user queues, those will all be mmhub
clients.
Alex
>
> Anway this patch is: Acked-by: Prike Liang <Prike.Liang@amd.com>
>
>
> > for (i = 0; i < AMDGPU_MES_MAX_GFX_PIPES; i++) {
> > /* use only 1st ME pipe */
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> > index 95d894a231fcf..19a5f196829f3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> > @@ -900,7 +900,7 @@ static int gmc_v10_0_sw_init(struct amdgpu_ip_block
> > *ip_block)
> > * amdgpu graphics/compute will use VMIDs 1-7
> > * amdkfd will use VMIDs 8-15
> > */
> > - adev->vm_manager.first_kfd_vmid = 8;
> > + adev->vm_manager.first_kfd_vmid = adev->gfx.disable_kq ? 1 : 8;
> >
> > amdgpu_vm_manager_init(adev);
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> > index bf8d01da88154..a2f6c9f4ebf2f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> > @@ -838,7 +838,7 @@ static int gmc_v12_0_sw_init(struct amdgpu_ip_block
> > *ip_block)
> > * amdgpu graphics/compute will use VMIDs 1-7
> > * amdkfd will use VMIDs 8-15
> > */
> > - adev->vm_manager.first_kfd_vmid = 8;
> > + adev->vm_manager.first_kfd_vmid = adev->gfx.disable_kq ? 1 : 8;
> >
> > amdgpu_vm_manager_init(adev);
> >
> > --
> > 2.48.1
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 07/11] drm/amdgpu/gfx11: add support for disable_kq
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
` (5 preceding siblings ...)
2025-03-13 14:41 ` [PATCH 06/11] drm/amdgpu/mes: make more vmids available when disable_kq=1 Alex Deucher
@ 2025-03-13 14:41 ` Alex Deucher
2025-03-13 22:08 ` Rodrigo Siqueira
2025-03-14 11:39 ` Khatri, Sunil
2025-03-13 14:41 ` [PATCH 08/11] drm/amdgpu/gfx12: " Alex Deucher
` (4 subsequent siblings)
11 siblings, 2 replies; 41+ messages in thread
From: Alex Deucher @ 2025-03-13 14:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher
Plumb in support for disabling kernel queues in
GFX11. We have to bring up a GFX queue briefly in
order to initialize the clear state. After that
we can disable it.
v2: use ring counts per Felix' suggestion
v3: fix stream fault handler, enable EOP interrupts
v4: fix MEC interrupt offset (Sunil)
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 191 ++++++++++++++++++-------
1 file changed, 136 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index 95eefd9a40d28..fde8464cbd3b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -1145,6 +1145,10 @@ static int gfx_v11_0_gfx_ring_init(struct amdgpu_device *adev, int ring_id,
ring->ring_obj = NULL;
ring->use_doorbell = true;
+ if (adev->gfx.disable_kq) {
+ ring->no_scheduler = true;
+ ring->no_user_submission = true;
+ }
if (!ring_id)
ring->doorbell_index = adev->doorbell_index.gfx_ring0 << 1;
@@ -1577,7 +1581,7 @@ static void gfx_v11_0_alloc_ip_dump(struct amdgpu_device *adev)
static int gfx_v11_0_sw_init(struct amdgpu_ip_block *ip_block)
{
- int i, j, k, r, ring_id = 0;
+ int i, j, k, r, ring_id;
int xcc_id = 0;
struct amdgpu_device *adev = ip_block->adev;
@@ -1710,37 +1714,42 @@ static int gfx_v11_0_sw_init(struct amdgpu_ip_block *ip_block)
return r;
}
- /* set up the gfx ring */
- for (i = 0; i < adev->gfx.me.num_me; i++) {
- for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
- for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
- if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
- continue;
-
- r = gfx_v11_0_gfx_ring_init(adev, ring_id,
- i, k, j);
- if (r)
- return r;
- ring_id++;
+ if (adev->gfx.num_gfx_rings) {
+ ring_id = 0;
+ /* set up the gfx ring */
+ for (i = 0; i < adev->gfx.me.num_me; i++) {
+ for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
+ for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
+ if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
+ continue;
+
+ r = gfx_v11_0_gfx_ring_init(adev, ring_id,
+ i, k, j);
+ if (r)
+ return r;
+ ring_id++;
+ }
}
}
}
- ring_id = 0;
- /* set up the compute queues - allocate horizontally across pipes */
- for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
- for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
- for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
- if (!amdgpu_gfx_is_mec_queue_enabled(adev, 0, i,
- k, j))
- continue;
+ if (adev->gfx.num_compute_rings) {
+ ring_id = 0;
+ /* set up the compute queues - allocate horizontally across pipes */
+ for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
+ for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
+ for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
+ if (!amdgpu_gfx_is_mec_queue_enabled(adev, 0, i,
+ k, j))
+ continue;
- r = gfx_v11_0_compute_ring_init(adev, ring_id,
- i, k, j);
- if (r)
- return r;
+ r = gfx_v11_0_compute_ring_init(adev, ring_id,
+ i, k, j);
+ if (r)
+ return r;
- ring_id++;
+ ring_id++;
+ }
}
}
}
@@ -4578,11 +4587,23 @@ static int gfx_v11_0_cp_resume(struct amdgpu_device *adev)
return r;
}
- for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
- ring = &adev->gfx.gfx_ring[i];
- r = amdgpu_ring_test_helper(ring);
- if (r)
- return r;
+ if (adev->gfx.disable_kq) {
+ for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
+ ring = &adev->gfx.gfx_ring[i];
+ /* we don't want to set ring->ready */
+ r = amdgpu_ring_test_ring(ring);
+ if (r)
+ return r;
+ }
+ if (amdgpu_async_gfx_ring)
+ amdgpu_gfx_disable_kgq(adev, 0);
+ } else {
+ for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
+ ring = &adev->gfx.gfx_ring[i];
+ r = amdgpu_ring_test_helper(ring);
+ if (r)
+ return r;
+ }
}
for (i = 0; i < adev->gfx.num_compute_rings; i++) {
@@ -4791,6 +4812,46 @@ static int gfx_v11_0_hw_init(struct amdgpu_ip_block *ip_block)
return r;
}
+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;
+
+ 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;
+ if (enable)
+ r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
+ irq_type);
+ else
+ r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
+ irq_type);
+ if (r)
+ return r;
+ }
+ }
+
+ 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
+ + (m * adev->gfx.mec.num_pipe_per_mec)
+ + p;
+ if (enable)
+ r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
+ irq_type);
+ else
+ r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
+ irq_type);
+ if (r)
+ return r;
+ }
+ }
+ }
+ return 0;
+}
+
static int gfx_v11_0_hw_fini(struct amdgpu_ip_block *ip_block)
{
struct amdgpu_device *adev = ip_block->adev;
@@ -4800,9 +4861,11 @@ static int gfx_v11_0_hw_fini(struct amdgpu_ip_block *ip_block)
amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
amdgpu_irq_put(adev, &adev->gfx.priv_inst_irq, 0);
amdgpu_irq_put(adev, &adev->gfx.bad_op_irq, 0);
+ gfx_v11_0_set_userq_eop_interrupts(adev, false);
if (!adev->no_hw_access) {
- if (amdgpu_async_gfx_ring) {
+ if (amdgpu_async_gfx_ring &&
+ !adev->gfx.disable_kq) {
if (amdgpu_gfx_disable_kgq(adev, 0))
DRM_ERROR("KGQ disable failed\n");
}
@@ -5128,11 +5191,22 @@ static int gfx_v11_0_early_init(struct amdgpu_ip_block *ip_block)
{
struct amdgpu_device *adev = ip_block->adev;
+ if (amdgpu_disable_kq == 1)
+ adev->gfx.disable_kq = true;
+
adev->gfx.funcs = &gfx_v11_0_gfx_funcs;
- adev->gfx.num_gfx_rings = GFX11_NUM_GFX_RINGS;
- adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
- AMDGPU_MAX_COMPUTE_RINGS);
+ if (adev->gfx.disable_kq) {
+ /* We need one GFX ring temporarily to set up
+ * the clear state.
+ */
+ adev->gfx.num_gfx_rings = 1;
+ adev->gfx.num_compute_rings = 0;
+ } else {
+ adev->gfx.num_gfx_rings = GFX11_NUM_GFX_RINGS;
+ adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
+ AMDGPU_MAX_COMPUTE_RINGS);
+ }
gfx_v11_0_set_kiq_pm4_funcs(adev);
gfx_v11_0_set_ring_funcs(adev);
@@ -5163,6 +5237,11 @@ static int gfx_v11_0_late_init(struct amdgpu_ip_block *ip_block)
r = amdgpu_irq_get(adev, &adev->gfx.bad_op_irq, 0);
if (r)
return r;
+
+ r = gfx_v11_0_set_userq_eop_interrupts(adev, true);
+ if (r)
+ return r;
+
return 0;
}
@@ -6548,27 +6627,29 @@ static void gfx_v11_0_handle_priv_fault(struct amdgpu_device *adev,
pipe_id = (entry->ring_id & 0x03) >> 0;
queue_id = (entry->ring_id & 0x70) >> 4;
- switch (me_id) {
- case 0:
- for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
- ring = &adev->gfx.gfx_ring[i];
- if (ring->me == me_id && ring->pipe == pipe_id &&
- ring->queue == queue_id)
- drm_sched_fault(&ring->sched);
- }
- break;
- case 1:
- case 2:
- for (i = 0; i < adev->gfx.num_compute_rings; i++) {
- ring = &adev->gfx.compute_ring[i];
- if (ring->me == me_id && ring->pipe == pipe_id &&
- ring->queue == queue_id)
- drm_sched_fault(&ring->sched);
+ if (!adev->gfx.disable_kq) {
+ switch (me_id) {
+ case 0:
+ for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
+ ring = &adev->gfx.gfx_ring[i];
+ if (ring->me == me_id && ring->pipe == pipe_id &&
+ ring->queue == queue_id)
+ drm_sched_fault(&ring->sched);
+ }
+ break;
+ case 1:
+ case 2:
+ for (i = 0; i < adev->gfx.num_compute_rings; i++) {
+ ring = &adev->gfx.compute_ring[i];
+ if (ring->me == me_id && ring->pipe == pipe_id &&
+ ring->queue == queue_id)
+ drm_sched_fault(&ring->sched);
+ }
+ break;
+ default:
+ BUG();
+ break;
}
- break;
- default:
- BUG();
- break;
}
}
--
2.48.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 07/11] drm/amdgpu/gfx11: add support for disable_kq
2025-03-13 14:41 ` [PATCH 07/11] drm/amdgpu/gfx11: add support for disable_kq Alex Deucher
@ 2025-03-13 22:08 ` Rodrigo Siqueira
2025-03-14 1:50 ` Alex Deucher
2025-03-14 11:39 ` Khatri, Sunil
1 sibling, 1 reply; 41+ messages in thread
From: Rodrigo Siqueira @ 2025-03-13 22:08 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx
On 03/13, Alex Deucher wrote:
> Plumb in support for disabling kernel queues in
> GFX11. We have to bring up a GFX queue briefly in
> order to initialize the clear state. After that
> we can disable it.
>
> v2: use ring counts per Felix' suggestion
> v3: fix stream fault handler, enable EOP interrupts
> v4: fix MEC interrupt offset (Sunil)
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 191 ++++++++++++++++++-------
> 1 file changed, 136 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> index 95eefd9a40d28..fde8464cbd3b3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> @@ -1145,6 +1145,10 @@ static int gfx_v11_0_gfx_ring_init(struct amdgpu_device *adev, int ring_id,
>
> ring->ring_obj = NULL;
> ring->use_doorbell = true;
> + if (adev->gfx.disable_kq) {
> + ring->no_scheduler = true;
Hi Alex,
Just a question about this no_scheduler part.
Set no_scheduler to true, means that all of the queues of GFX11 will not
be preempted, right? I suppose you have to do it because you want to
initialize the clear state?
Thanks
> + ring->no_user_submission = true;
> + }
>
> if (!ring_id)
> ring->doorbell_index = adev->doorbell_index.gfx_ring0 << 1;
> @@ -1577,7 +1581,7 @@ static void gfx_v11_0_alloc_ip_dump(struct amdgpu_device *adev)
>
> static int gfx_v11_0_sw_init(struct amdgpu_ip_block *ip_block)
> {
> - int i, j, k, r, ring_id = 0;
> + int i, j, k, r, ring_id;
> int xcc_id = 0;
> struct amdgpu_device *adev = ip_block->adev;
>
> @@ -1710,37 +1714,42 @@ static int gfx_v11_0_sw_init(struct amdgpu_ip_block *ip_block)
> return r;
> }
>
> - /* set up the gfx ring */
> - for (i = 0; i < adev->gfx.me.num_me; i++) {
> - for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
> - for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
> - if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
> - continue;
> -
> - r = gfx_v11_0_gfx_ring_init(adev, ring_id,
> - i, k, j);
> - if (r)
> - return r;
> - ring_id++;
> + if (adev->gfx.num_gfx_rings) {
> + ring_id = 0;
> + /* set up the gfx ring */
> + for (i = 0; i < adev->gfx.me.num_me; i++) {
> + for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
> + for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
> + if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
> + continue;
> +
> + r = gfx_v11_0_gfx_ring_init(adev, ring_id,
> + i, k, j);
> + if (r)
> + return r;
> + ring_id++;
> + }
> }
> }
> }
>
> - ring_id = 0;
> - /* set up the compute queues - allocate horizontally across pipes */
> - for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
> - for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
> - for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
> - if (!amdgpu_gfx_is_mec_queue_enabled(adev, 0, i,
> - k, j))
> - continue;
> + if (adev->gfx.num_compute_rings) {
> + ring_id = 0;
> + /* set up the compute queues - allocate horizontally across pipes */
> + for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
> + for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
> + for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
> + if (!amdgpu_gfx_is_mec_queue_enabled(adev, 0, i,
> + k, j))
> + continue;
>
> - r = gfx_v11_0_compute_ring_init(adev, ring_id,
> - i, k, j);
> - if (r)
> - return r;
> + r = gfx_v11_0_compute_ring_init(adev, ring_id,
> + i, k, j);
> + if (r)
> + return r;
>
> - ring_id++;
> + ring_id++;
> + }
> }
> }
> }
> @@ -4578,11 +4587,23 @@ static int gfx_v11_0_cp_resume(struct amdgpu_device *adev)
> return r;
> }
>
> - for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> - ring = &adev->gfx.gfx_ring[i];
> - r = amdgpu_ring_test_helper(ring);
> - if (r)
> - return r;
> + if (adev->gfx.disable_kq) {
> + for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> + ring = &adev->gfx.gfx_ring[i];
> + /* we don't want to set ring->ready */
> + r = amdgpu_ring_test_ring(ring);
> + if (r)
> + return r;
> + }
> + if (amdgpu_async_gfx_ring)
> + amdgpu_gfx_disable_kgq(adev, 0);
> + } else {
> + for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> + ring = &adev->gfx.gfx_ring[i];
> + r = amdgpu_ring_test_helper(ring);
> + if (r)
> + return r;
> + }
> }
>
> for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> @@ -4791,6 +4812,46 @@ static int gfx_v11_0_hw_init(struct amdgpu_ip_block *ip_block)
> return r;
> }
>
> +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;
> +
> + 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;
> + if (enable)
> + r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
> + irq_type);
> + else
> + r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
> + irq_type);
> + if (r)
> + return r;
> + }
> + }
> +
> + 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
> + + (m * adev->gfx.mec.num_pipe_per_mec)
> + + p;
> + if (enable)
> + r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
> + irq_type);
> + else
> + r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
> + irq_type);
> + if (r)
> + return r;
> + }
> + }
> + }
> + return 0;
> +}
> +
> static int gfx_v11_0_hw_fini(struct amdgpu_ip_block *ip_block)
> {
> struct amdgpu_device *adev = ip_block->adev;
> @@ -4800,9 +4861,11 @@ static int gfx_v11_0_hw_fini(struct amdgpu_ip_block *ip_block)
> amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
> amdgpu_irq_put(adev, &adev->gfx.priv_inst_irq, 0);
> amdgpu_irq_put(adev, &adev->gfx.bad_op_irq, 0);
> + gfx_v11_0_set_userq_eop_interrupts(adev, false);
>
> if (!adev->no_hw_access) {
> - if (amdgpu_async_gfx_ring) {
> + if (amdgpu_async_gfx_ring &&
> + !adev->gfx.disable_kq) {
> if (amdgpu_gfx_disable_kgq(adev, 0))
> DRM_ERROR("KGQ disable failed\n");
> }
> @@ -5128,11 +5191,22 @@ static int gfx_v11_0_early_init(struct amdgpu_ip_block *ip_block)
> {
> struct amdgpu_device *adev = ip_block->adev;
>
> + if (amdgpu_disable_kq == 1)
> + adev->gfx.disable_kq = true;
> +
> adev->gfx.funcs = &gfx_v11_0_gfx_funcs;
>
> - adev->gfx.num_gfx_rings = GFX11_NUM_GFX_RINGS;
> - adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
> - AMDGPU_MAX_COMPUTE_RINGS);
> + if (adev->gfx.disable_kq) {
> + /* We need one GFX ring temporarily to set up
> + * the clear state.
> + */
> + adev->gfx.num_gfx_rings = 1;
> + adev->gfx.num_compute_rings = 0;
> + } else {
> + adev->gfx.num_gfx_rings = GFX11_NUM_GFX_RINGS;
> + adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
> + AMDGPU_MAX_COMPUTE_RINGS);
> + }
>
> gfx_v11_0_set_kiq_pm4_funcs(adev);
> gfx_v11_0_set_ring_funcs(adev);
> @@ -5163,6 +5237,11 @@ static int gfx_v11_0_late_init(struct amdgpu_ip_block *ip_block)
> r = amdgpu_irq_get(adev, &adev->gfx.bad_op_irq, 0);
> if (r)
> return r;
> +
> + r = gfx_v11_0_set_userq_eop_interrupts(adev, true);
> + if (r)
> + return r;
> +
> return 0;
> }
>
> @@ -6548,27 +6627,29 @@ static void gfx_v11_0_handle_priv_fault(struct amdgpu_device *adev,
> pipe_id = (entry->ring_id & 0x03) >> 0;
> queue_id = (entry->ring_id & 0x70) >> 4;
>
> - switch (me_id) {
> - case 0:
> - for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> - ring = &adev->gfx.gfx_ring[i];
> - if (ring->me == me_id && ring->pipe == pipe_id &&
> - ring->queue == queue_id)
> - drm_sched_fault(&ring->sched);
> - }
> - break;
> - case 1:
> - case 2:
> - for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> - ring = &adev->gfx.compute_ring[i];
> - if (ring->me == me_id && ring->pipe == pipe_id &&
> - ring->queue == queue_id)
> - drm_sched_fault(&ring->sched);
> + if (!adev->gfx.disable_kq) {
> + switch (me_id) {
> + case 0:
> + for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> + ring = &adev->gfx.gfx_ring[i];
> + if (ring->me == me_id && ring->pipe == pipe_id &&
> + ring->queue == queue_id)
> + drm_sched_fault(&ring->sched);
> + }
> + break;
> + case 1:
> + case 2:
> + for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> + ring = &adev->gfx.compute_ring[i];
> + if (ring->me == me_id && ring->pipe == pipe_id &&
> + ring->queue == queue_id)
> + drm_sched_fault(&ring->sched);
> + }
> + break;
> + default:
> + BUG();
> + break;
> }
> - break;
> - default:
> - BUG();
> - break;
> }
> }
>
> --
> 2.48.1
>
--
Rodrigo Siqueira
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 07/11] drm/amdgpu/gfx11: add support for disable_kq
2025-03-13 22:08 ` Rodrigo Siqueira
@ 2025-03-14 1:50 ` Alex Deucher
0 siblings, 0 replies; 41+ messages in thread
From: Alex Deucher @ 2025-03-14 1:50 UTC (permalink / raw)
To: Rodrigo Siqueira; +Cc: Alex Deucher, amd-gfx
On Thu, Mar 13, 2025 at 6:08 PM Rodrigo Siqueira <siqueira@igalia.com> wrote:
>
> On 03/13, Alex Deucher wrote:
> > Plumb in support for disabling kernel queues in
> > GFX11. We have to bring up a GFX queue briefly in
> > order to initialize the clear state. After that
> > we can disable it.
> >
> > v2: use ring counts per Felix' suggestion
> > v3: fix stream fault handler, enable EOP interrupts
> > v4: fix MEC interrupt offset (Sunil)
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 191 ++++++++++++++++++-------
> > 1 file changed, 136 insertions(+), 55 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > index 95eefd9a40d28..fde8464cbd3b3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > @@ -1145,6 +1145,10 @@ static int gfx_v11_0_gfx_ring_init(struct amdgpu_device *adev, int ring_id,
> >
> > ring->ring_obj = NULL;
> > ring->use_doorbell = true;
> > + if (adev->gfx.disable_kq) {
> > + ring->no_scheduler = true;
>
> Hi Alex,
>
> Just a question about this no_scheduler part.
>
> Set no_scheduler to true, means that all of the queues of GFX11 will not
> be preempted, right? I suppose you have to do it because you want to
> initialize the clear state?
Not exactly. We just spin up a gfx queue long enough to submit the
clear state setup and then we tear it down so its queue slot is
available for user queues. So it's not actually a usable kernel queue
at runtime. Setting the no_scheduler flag prevents a drm scheduler
from being initialized for the queue.
Alex
>
> Thanks
>
> > + ring->no_user_submission = true;
> > + }
> >
> > if (!ring_id)
> > ring->doorbell_index = adev->doorbell_index.gfx_ring0 << 1;
> > @@ -1577,7 +1581,7 @@ static void gfx_v11_0_alloc_ip_dump(struct amdgpu_device *adev)
> >
> > static int gfx_v11_0_sw_init(struct amdgpu_ip_block *ip_block)
> > {
> > - int i, j, k, r, ring_id = 0;
> > + int i, j, k, r, ring_id;
> > int xcc_id = 0;
> > struct amdgpu_device *adev = ip_block->adev;
> >
> > @@ -1710,37 +1714,42 @@ static int gfx_v11_0_sw_init(struct amdgpu_ip_block *ip_block)
> > return r;
> > }
> >
> > - /* set up the gfx ring */
> > - for (i = 0; i < adev->gfx.me.num_me; i++) {
> > - for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
> > - for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
> > - if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
> > - continue;
> > -
> > - r = gfx_v11_0_gfx_ring_init(adev, ring_id,
> > - i, k, j);
> > - if (r)
> > - return r;
> > - ring_id++;
> > + if (adev->gfx.num_gfx_rings) {
> > + ring_id = 0;
> > + /* set up the gfx ring */
> > + for (i = 0; i < adev->gfx.me.num_me; i++) {
> > + for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
> > + for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
> > + if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
> > + continue;
> > +
> > + r = gfx_v11_0_gfx_ring_init(adev, ring_id,
> > + i, k, j);
> > + if (r)
> > + return r;
> > + ring_id++;
> > + }
> > }
> > }
> > }
> >
> > - ring_id = 0;
> > - /* set up the compute queues - allocate horizontally across pipes */
> > - for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
> > - for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
> > - for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
> > - if (!amdgpu_gfx_is_mec_queue_enabled(adev, 0, i,
> > - k, j))
> > - continue;
> > + if (adev->gfx.num_compute_rings) {
> > + ring_id = 0;
> > + /* set up the compute queues - allocate horizontally across pipes */
> > + for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
> > + for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
> > + for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
> > + if (!amdgpu_gfx_is_mec_queue_enabled(adev, 0, i,
> > + k, j))
> > + continue;
> >
> > - r = gfx_v11_0_compute_ring_init(adev, ring_id,
> > - i, k, j);
> > - if (r)
> > - return r;
> > + r = gfx_v11_0_compute_ring_init(adev, ring_id,
> > + i, k, j);
> > + if (r)
> > + return r;
> >
> > - ring_id++;
> > + ring_id++;
> > + }
> > }
> > }
> > }
> > @@ -4578,11 +4587,23 @@ static int gfx_v11_0_cp_resume(struct amdgpu_device *adev)
> > return r;
> > }
> >
> > - for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> > - ring = &adev->gfx.gfx_ring[i];
> > - r = amdgpu_ring_test_helper(ring);
> > - if (r)
> > - return r;
> > + if (adev->gfx.disable_kq) {
> > + for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> > + ring = &adev->gfx.gfx_ring[i];
> > + /* we don't want to set ring->ready */
> > + r = amdgpu_ring_test_ring(ring);
> > + if (r)
> > + return r;
> > + }
> > + if (amdgpu_async_gfx_ring)
> > + amdgpu_gfx_disable_kgq(adev, 0);
> > + } else {
> > + for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> > + ring = &adev->gfx.gfx_ring[i];
> > + r = amdgpu_ring_test_helper(ring);
> > + if (r)
> > + return r;
> > + }
> > }
> >
> > for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> > @@ -4791,6 +4812,46 @@ static int gfx_v11_0_hw_init(struct amdgpu_ip_block *ip_block)
> > return r;
> > }
> >
> > +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;
> > +
> > + 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;
> > + if (enable)
> > + r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
> > + irq_type);
> > + else
> > + r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
> > + irq_type);
> > + if (r)
> > + return r;
> > + }
> > + }
> > +
> > + 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
> > + + (m * adev->gfx.mec.num_pipe_per_mec)
> > + + p;
> > + if (enable)
> > + r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
> > + irq_type);
> > + else
> > + r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
> > + irq_type);
> > + if (r)
> > + return r;
> > + }
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > static int gfx_v11_0_hw_fini(struct amdgpu_ip_block *ip_block)
> > {
> > struct amdgpu_device *adev = ip_block->adev;
> > @@ -4800,9 +4861,11 @@ static int gfx_v11_0_hw_fini(struct amdgpu_ip_block *ip_block)
> > amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
> > amdgpu_irq_put(adev, &adev->gfx.priv_inst_irq, 0);
> > amdgpu_irq_put(adev, &adev->gfx.bad_op_irq, 0);
> > + gfx_v11_0_set_userq_eop_interrupts(adev, false);
> >
> > if (!adev->no_hw_access) {
> > - if (amdgpu_async_gfx_ring) {
> > + if (amdgpu_async_gfx_ring &&
> > + !adev->gfx.disable_kq) {
> > if (amdgpu_gfx_disable_kgq(adev, 0))
> > DRM_ERROR("KGQ disable failed\n");
> > }
> > @@ -5128,11 +5191,22 @@ static int gfx_v11_0_early_init(struct amdgpu_ip_block *ip_block)
> > {
> > struct amdgpu_device *adev = ip_block->adev;
> >
> > + if (amdgpu_disable_kq == 1)
> > + adev->gfx.disable_kq = true;
> > +
> > adev->gfx.funcs = &gfx_v11_0_gfx_funcs;
> >
> > - adev->gfx.num_gfx_rings = GFX11_NUM_GFX_RINGS;
> > - adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
> > - AMDGPU_MAX_COMPUTE_RINGS);
> > + if (adev->gfx.disable_kq) {
> > + /* We need one GFX ring temporarily to set up
> > + * the clear state.
> > + */
> > + adev->gfx.num_gfx_rings = 1;
> > + adev->gfx.num_compute_rings = 0;
> > + } else {
> > + adev->gfx.num_gfx_rings = GFX11_NUM_GFX_RINGS;
> > + adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
> > + AMDGPU_MAX_COMPUTE_RINGS);
> > + }
> >
> > gfx_v11_0_set_kiq_pm4_funcs(adev);
> > gfx_v11_0_set_ring_funcs(adev);
> > @@ -5163,6 +5237,11 @@ static int gfx_v11_0_late_init(struct amdgpu_ip_block *ip_block)
> > r = amdgpu_irq_get(adev, &adev->gfx.bad_op_irq, 0);
> > if (r)
> > return r;
> > +
> > + r = gfx_v11_0_set_userq_eop_interrupts(adev, true);
> > + if (r)
> > + return r;
> > +
> > return 0;
> > }
> >
> > @@ -6548,27 +6627,29 @@ static void gfx_v11_0_handle_priv_fault(struct amdgpu_device *adev,
> > pipe_id = (entry->ring_id & 0x03) >> 0;
> > queue_id = (entry->ring_id & 0x70) >> 4;
> >
> > - switch (me_id) {
> > - case 0:
> > - for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> > - ring = &adev->gfx.gfx_ring[i];
> > - if (ring->me == me_id && ring->pipe == pipe_id &&
> > - ring->queue == queue_id)
> > - drm_sched_fault(&ring->sched);
> > - }
> > - break;
> > - case 1:
> > - case 2:
> > - for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> > - ring = &adev->gfx.compute_ring[i];
> > - if (ring->me == me_id && ring->pipe == pipe_id &&
> > - ring->queue == queue_id)
> > - drm_sched_fault(&ring->sched);
> > + if (!adev->gfx.disable_kq) {
> > + switch (me_id) {
> > + case 0:
> > + for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> > + ring = &adev->gfx.gfx_ring[i];
> > + if (ring->me == me_id && ring->pipe == pipe_id &&
> > + ring->queue == queue_id)
> > + drm_sched_fault(&ring->sched);
> > + }
> > + break;
> > + case 1:
> > + case 2:
> > + for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> > + ring = &adev->gfx.compute_ring[i];
> > + if (ring->me == me_id && ring->pipe == pipe_id &&
> > + ring->queue == queue_id)
> > + drm_sched_fault(&ring->sched);
> > + }
> > + break;
> > + default:
> > + BUG();
> > + break;
> > }
> > - break;
> > - default:
> > - BUG();
> > - break;
> > }
> > }
> >
> > --
> > 2.48.1
> >
>
> --
> Rodrigo Siqueira
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 07/11] drm/amdgpu/gfx11: add support for disable_kq
2025-03-13 14:41 ` [PATCH 07/11] drm/amdgpu/gfx11: add support for disable_kq Alex Deucher
2025-03-13 22:08 ` Rodrigo Siqueira
@ 2025-03-14 11:39 ` Khatri, Sunil
1 sibling, 0 replies; 41+ messages in thread
From: Khatri, Sunil @ 2025-03-14 11:39 UTC (permalink / raw)
To: Alex Deucher, amd-gfx
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
On 3/13/2025 8:11 PM, Alex Deucher wrote:
> Plumb in support for disabling kernel queues in
> GFX11. We have to bring up a GFX queue briefly in
> order to initialize the clear state. After that
> we can disable it.
>
> v2: use ring counts per Felix' suggestion
> v3: fix stream fault handler, enable EOP interrupts
> v4: fix MEC interrupt offset (Sunil)
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 191 ++++++++++++++++++-------
> 1 file changed, 136 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> index 95eefd9a40d28..fde8464cbd3b3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> @@ -1145,6 +1145,10 @@ static int gfx_v11_0_gfx_ring_init(struct amdgpu_device *adev, int ring_id,
>
> ring->ring_obj = NULL;
> ring->use_doorbell = true;
> + if (adev->gfx.disable_kq) {
> + ring->no_scheduler = true;
> + ring->no_user_submission = true;
> + }
>
> if (!ring_id)
> ring->doorbell_index = adev->doorbell_index.gfx_ring0 << 1;
> @@ -1577,7 +1581,7 @@ static void gfx_v11_0_alloc_ip_dump(struct amdgpu_device *adev)
>
> static int gfx_v11_0_sw_init(struct amdgpu_ip_block *ip_block)
> {
> - int i, j, k, r, ring_id = 0;
> + int i, j, k, r, ring_id;
> int xcc_id = 0;
> struct amdgpu_device *adev = ip_block->adev;
>
> @@ -1710,37 +1714,42 @@ static int gfx_v11_0_sw_init(struct amdgpu_ip_block *ip_block)
> return r;
> }
>
> - /* set up the gfx ring */
> - for (i = 0; i < adev->gfx.me.num_me; i++) {
> - for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
> - for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
> - if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
> - continue;
> -
> - r = gfx_v11_0_gfx_ring_init(adev, ring_id,
> - i, k, j);
> - if (r)
> - return r;
> - ring_id++;
> + if (adev->gfx.num_gfx_rings) {
> + ring_id = 0;
> + /* set up the gfx ring */
> + for (i = 0; i < adev->gfx.me.num_me; i++) {
> + for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
> + for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
> + if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
> + continue;
> +
> + r = gfx_v11_0_gfx_ring_init(adev, ring_id,
> + i, k, j);
> + if (r)
> + return r;
> + ring_id++;
> + }
> }
> }
> }
>
> - ring_id = 0;
> - /* set up the compute queues - allocate horizontally across pipes */
> - for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
> - for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
> - for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
> - if (!amdgpu_gfx_is_mec_queue_enabled(adev, 0, i,
> - k, j))
> - continue;
> + if (adev->gfx.num_compute_rings) {
> + ring_id = 0;
> + /* set up the compute queues - allocate horizontally across pipes */
> + for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
> + for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
> + for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
> + if (!amdgpu_gfx_is_mec_queue_enabled(adev, 0, i,
> + k, j))
> + continue;
>
> - r = gfx_v11_0_compute_ring_init(adev, ring_id,
> - i, k, j);
> - if (r)
> - return r;
> + r = gfx_v11_0_compute_ring_init(adev, ring_id,
> + i, k, j);
> + if (r)
> + return r;
>
> - ring_id++;
> + ring_id++;
> + }
> }
> }
> }
> @@ -4578,11 +4587,23 @@ static int gfx_v11_0_cp_resume(struct amdgpu_device *adev)
> return r;
> }
>
> - for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> - ring = &adev->gfx.gfx_ring[i];
> - r = amdgpu_ring_test_helper(ring);
> - if (r)
> - return r;
> + if (adev->gfx.disable_kq) {
> + for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> + ring = &adev->gfx.gfx_ring[i];
> + /* we don't want to set ring->ready */
> + r = amdgpu_ring_test_ring(ring);
> + if (r)
> + return r;
> + }
> + if (amdgpu_async_gfx_ring)
> + amdgpu_gfx_disable_kgq(adev, 0);
> + } else {
> + for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> + ring = &adev->gfx.gfx_ring[i];
> + r = amdgpu_ring_test_helper(ring);
> + if (r)
> + return r;
> + }
> }
>
> for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> @@ -4791,6 +4812,46 @@ static int gfx_v11_0_hw_init(struct amdgpu_ip_block *ip_block)
> return r;
> }
>
> +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;
> +
> + 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;
> + if (enable)
> + r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
> + irq_type);
> + else
> + r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
> + irq_type);
> + if (r)
> + return r;
> + }
> + }
> +
> + 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
> + + (m * adev->gfx.mec.num_pipe_per_mec)
> + + p;
> + if (enable)
> + r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
> + irq_type);
> + else
> + r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
> + irq_type);
> + if (r)
> + return r;
> + }
> + }
> + }
> + return 0;
> +}
> +
> static int gfx_v11_0_hw_fini(struct amdgpu_ip_block *ip_block)
> {
> struct amdgpu_device *adev = ip_block->adev;
> @@ -4800,9 +4861,11 @@ static int gfx_v11_0_hw_fini(struct amdgpu_ip_block *ip_block)
> amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
> amdgpu_irq_put(adev, &adev->gfx.priv_inst_irq, 0);
> amdgpu_irq_put(adev, &adev->gfx.bad_op_irq, 0);
> + gfx_v11_0_set_userq_eop_interrupts(adev, false);
>
> if (!adev->no_hw_access) {
> - if (amdgpu_async_gfx_ring) {
> + if (amdgpu_async_gfx_ring &&
> + !adev->gfx.disable_kq) {
> if (amdgpu_gfx_disable_kgq(adev, 0))
> DRM_ERROR("KGQ disable failed\n");
> }
> @@ -5128,11 +5191,22 @@ static int gfx_v11_0_early_init(struct amdgpu_ip_block *ip_block)
> {
> struct amdgpu_device *adev = ip_block->adev;
>
> + if (amdgpu_disable_kq == 1)
> + adev->gfx.disable_kq = true;
> +
> adev->gfx.funcs = &gfx_v11_0_gfx_funcs;
>
> - adev->gfx.num_gfx_rings = GFX11_NUM_GFX_RINGS;
> - adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
> - AMDGPU_MAX_COMPUTE_RINGS);
> + if (adev->gfx.disable_kq) {
> + /* We need one GFX ring temporarily to set up
> + * the clear state.
> + */
> + adev->gfx.num_gfx_rings = 1;
> + adev->gfx.num_compute_rings = 0;
> + } else {
> + adev->gfx.num_gfx_rings = GFX11_NUM_GFX_RINGS;
> + adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
> + AMDGPU_MAX_COMPUTE_RINGS);
> + }
>
> gfx_v11_0_set_kiq_pm4_funcs(adev);
> gfx_v11_0_set_ring_funcs(adev);
> @@ -5163,6 +5237,11 @@ static int gfx_v11_0_late_init(struct amdgpu_ip_block *ip_block)
> r = amdgpu_irq_get(adev, &adev->gfx.bad_op_irq, 0);
> if (r)
> return r;
> +
> + r = gfx_v11_0_set_userq_eop_interrupts(adev, true);
> + if (r)
> + return r;
> +
> return 0;
> }
>
> @@ -6548,27 +6627,29 @@ static void gfx_v11_0_handle_priv_fault(struct amdgpu_device *adev,
> pipe_id = (entry->ring_id & 0x03) >> 0;
> queue_id = (entry->ring_id & 0x70) >> 4;
>
> - switch (me_id) {
> - case 0:
> - for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> - ring = &adev->gfx.gfx_ring[i];
> - if (ring->me == me_id && ring->pipe == pipe_id &&
> - ring->queue == queue_id)
> - drm_sched_fault(&ring->sched);
> - }
> - break;
> - case 1:
> - case 2:
> - for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> - ring = &adev->gfx.compute_ring[i];
> - if (ring->me == me_id && ring->pipe == pipe_id &&
> - ring->queue == queue_id)
> - drm_sched_fault(&ring->sched);
> + if (!adev->gfx.disable_kq) {
> + switch (me_id) {
> + case 0:
> + for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> + ring = &adev->gfx.gfx_ring[i];
> + if (ring->me == me_id && ring->pipe == pipe_id &&
> + ring->queue == queue_id)
> + drm_sched_fault(&ring->sched);
> + }
> + break;
> + case 1:
> + case 2:
> + for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> + ring = &adev->gfx.compute_ring[i];
> + if (ring->me == me_id && ring->pipe == pipe_id &&
> + ring->queue == queue_id)
> + drm_sched_fault(&ring->sched);
> + }
> + break;
> + default:
> + BUG();
> + break;
> }
> - break;
> - default:
> - BUG();
> - break;
> }
> }
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 08/11] drm/amdgpu/gfx12: add support for disable_kq
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
` (6 preceding siblings ...)
2025-03-13 14:41 ` [PATCH 07/11] drm/amdgpu/gfx11: add support for disable_kq Alex Deucher
@ 2025-03-13 14:41 ` Alex Deucher
2025-03-14 11:43 ` Khatri, Sunil
2025-03-13 14:41 ` [PATCH 09/11] drm/amdgpu/sdma: add flag for tracking disable_kq Alex Deucher
` (3 subsequent siblings)
11 siblings, 1 reply; 41+ messages in thread
From: Alex Deucher @ 2025-03-13 14:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher
Plumb in support for disabling kernel queues.
v2: use ring counts per Felix' suggestion
v3: fix stream fault handler, enable EOP interrupts
v4: fix MEC interrupt offset (Sunil)
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 183 +++++++++++++++++--------
1 file changed, 125 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
index 34cf187e72d9f..a99507e4fdb27 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
@@ -1421,11 +1421,13 @@ static int gfx_v12_0_sw_init(struct amdgpu_ip_block *ip_block)
break;
}
- /* recalculate compute rings to use based on hardware configuration */
- num_compute_rings = (adev->gfx.mec.num_pipe_per_mec *
- adev->gfx.mec.num_queue_per_pipe) / 2;
- adev->gfx.num_compute_rings = min(adev->gfx.num_compute_rings,
- num_compute_rings);
+ if (adev->gfx.num_compute_rings) {
+ /* recalculate compute rings to use based on hardware configuration */
+ num_compute_rings = (adev->gfx.mec.num_pipe_per_mec *
+ adev->gfx.mec.num_queue_per_pipe) / 2;
+ adev->gfx.num_compute_rings = min(adev->gfx.num_compute_rings,
+ num_compute_rings);
+ }
/* EOP Event */
r = amdgpu_irq_add_id(adev, SOC21_IH_CLIENTID_GRBM_CP,
@@ -1471,37 +1473,41 @@ static int gfx_v12_0_sw_init(struct amdgpu_ip_block *ip_block)
return r;
}
- /* set up the gfx ring */
- for (i = 0; i < adev->gfx.me.num_me; i++) {
- for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
- for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
- if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
- continue;
-
- r = gfx_v12_0_gfx_ring_init(adev, ring_id,
- i, k, j);
- if (r)
- return r;
- ring_id++;
+ if (adev->gfx.num_gfx_rings) {
+ /* set up the gfx ring */
+ for (i = 0; i < adev->gfx.me.num_me; i++) {
+ for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
+ for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
+ if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
+ continue;
+
+ r = gfx_v12_0_gfx_ring_init(adev, ring_id,
+ i, k, j);
+ if (r)
+ return r;
+ ring_id++;
+ }
}
}
}
- ring_id = 0;
- /* set up the compute queues - allocate horizontally across pipes */
- for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
- for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
- for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
- if (!amdgpu_gfx_is_mec_queue_enabled(adev,
- 0, i, k, j))
- continue;
+ if (adev->gfx.num_compute_rings) {
+ ring_id = 0;
+ /* set up the compute queues - allocate horizontally across pipes */
+ for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
+ for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
+ for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
+ if (!amdgpu_gfx_is_mec_queue_enabled(adev,
+ 0, i, k, j))
+ continue;
- r = gfx_v12_0_compute_ring_init(adev, ring_id,
- i, k, j);
- if (r)
- return r;
+ r = gfx_v12_0_compute_ring_init(adev, ring_id,
+ i, k, j);
+ if (r)
+ return r;
- ring_id++;
+ ring_id++;
+ }
}
}
}
@@ -3495,12 +3501,18 @@ static int gfx_v12_0_cp_resume(struct amdgpu_device *adev)
if (r)
return r;
- if (!amdgpu_async_gfx_ring) {
- r = gfx_v12_0_cp_gfx_resume(adev);
- if (r)
- return r;
+ if (adev->gfx.num_gfx_rings) {
+ if (!amdgpu_async_gfx_ring) {
+ r = gfx_v12_0_cp_gfx_resume(adev);
+ if (r)
+ return r;
+ } else {
+ r = gfx_v12_0_cp_async_gfx_ring_resume(adev);
+ if (r)
+ return r;
+ }
} else {
- r = gfx_v12_0_cp_async_gfx_ring_resume(adev);
+ r = gfx_v12_0_cp_gfx_start(adev);
if (r)
return r;
}
@@ -3711,6 +3723,46 @@ static int gfx_v12_0_hw_init(struct amdgpu_ip_block *ip_block)
return r;
}
+static int gfx_v12_0_set_userq_eop_interrupts(struct amdgpu_device *adev,
+ bool enable)
+{
+ if (adev->gfx.disable_kq) {
+ unsigned int irq_type;
+ int m, p, r;
+
+ 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;
+ if (enable)
+ r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
+ irq_type);
+ else
+ r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
+ irq_type);
+ if (r)
+ return r;
+ }
+ }
+
+ 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
+ + (m * adev->gfx.mec.num_pipe_per_mec)
+ + p;
+ if (enable)
+ r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
+ irq_type);
+ else
+ r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
+ irq_type);
+ if (r)
+ return r;
+ }
+ }
+ }
+ return 0;
+}
+
static int gfx_v12_0_hw_fini(struct amdgpu_ip_block *ip_block)
{
struct amdgpu_device *adev = ip_block->adev;
@@ -3721,6 +3773,7 @@ static int gfx_v12_0_hw_fini(struct amdgpu_ip_block *ip_block)
amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
amdgpu_irq_put(adev, &adev->gfx.priv_inst_irq, 0);
amdgpu_irq_put(adev, &adev->gfx.bad_op_irq, 0);
+ gfx_v12_0_set_userq_eop_interrupts(adev, false);
if (!adev->no_hw_access) {
if (amdgpu_async_gfx_ring) {
@@ -3809,11 +3862,19 @@ static int gfx_v12_0_early_init(struct amdgpu_ip_block *ip_block)
{
struct amdgpu_device *adev = ip_block->adev;
+ if (amdgpu_disable_kq == 1)
+ adev->gfx.disable_kq = true;
+
adev->gfx.funcs = &gfx_v12_0_gfx_funcs;
- adev->gfx.num_gfx_rings = GFX12_NUM_GFX_RINGS;
- adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
- AMDGPU_MAX_COMPUTE_RINGS);
+ if (adev->gfx.disable_kq) {
+ adev->gfx.num_gfx_rings = 0;
+ adev->gfx.num_compute_rings = 0;
+ } else {
+ adev->gfx.num_gfx_rings = GFX12_NUM_GFX_RINGS;
+ adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
+ AMDGPU_MAX_COMPUTE_RINGS);
+ }
gfx_v12_0_set_kiq_pm4_funcs(adev);
gfx_v12_0_set_ring_funcs(adev);
@@ -3844,6 +3905,10 @@ static int gfx_v12_0_late_init(struct amdgpu_ip_block *ip_block)
if (r)
return r;
+ r = gfx_v12_0_set_userq_eop_interrupts(adev, true);
+ if (r)
+ return r;
+
return 0;
}
@@ -5043,27 +5108,29 @@ static void gfx_v12_0_handle_priv_fault(struct amdgpu_device *adev,
pipe_id = (entry->ring_id & 0x03) >> 0;
queue_id = (entry->ring_id & 0x70) >> 4;
- switch (me_id) {
- case 0:
- for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
- ring = &adev->gfx.gfx_ring[i];
- if (ring->me == me_id && ring->pipe == pipe_id &&
- ring->queue == queue_id)
- drm_sched_fault(&ring->sched);
- }
- break;
- case 1:
- case 2:
- for (i = 0; i < adev->gfx.num_compute_rings; i++) {
- ring = &adev->gfx.compute_ring[i];
- if (ring->me == me_id && ring->pipe == pipe_id &&
- ring->queue == queue_id)
- drm_sched_fault(&ring->sched);
+ if (!adev->gfx.disable_kq) {
+ switch (me_id) {
+ case 0:
+ for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
+ ring = &adev->gfx.gfx_ring[i];
+ if (ring->me == me_id && ring->pipe == pipe_id &&
+ ring->queue == queue_id)
+ drm_sched_fault(&ring->sched);
+ }
+ break;
+ case 1:
+ case 2:
+ for (i = 0; i < adev->gfx.num_compute_rings; i++) {
+ ring = &adev->gfx.compute_ring[i];
+ if (ring->me == me_id && ring->pipe == pipe_id &&
+ ring->queue == queue_id)
+ drm_sched_fault(&ring->sched);
+ }
+ break;
+ default:
+ BUG();
+ break;
}
- break;
- default:
- BUG();
- break;
}
}
--
2.48.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 08/11] drm/amdgpu/gfx12: add support for disable_kq
2025-03-13 14:41 ` [PATCH 08/11] drm/amdgpu/gfx12: " Alex Deucher
@ 2025-03-14 11:43 ` Khatri, Sunil
0 siblings, 0 replies; 41+ messages in thread
From: Khatri, Sunil @ 2025-03-14 11:43 UTC (permalink / raw)
To: Alex Deucher, amd-gfx
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
On 3/13/2025 8:11 PM, Alex Deucher wrote:
> Plumb in support for disabling kernel queues.
>
> v2: use ring counts per Felix' suggestion
> v3: fix stream fault handler, enable EOP interrupts
> v4: fix MEC interrupt offset (Sunil)
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 183 +++++++++++++++++--------
> 1 file changed, 125 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
> index 34cf187e72d9f..a99507e4fdb27 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
> @@ -1421,11 +1421,13 @@ static int gfx_v12_0_sw_init(struct amdgpu_ip_block *ip_block)
> break;
> }
>
> - /* recalculate compute rings to use based on hardware configuration */
> - num_compute_rings = (adev->gfx.mec.num_pipe_per_mec *
> - adev->gfx.mec.num_queue_per_pipe) / 2;
> - adev->gfx.num_compute_rings = min(adev->gfx.num_compute_rings,
> - num_compute_rings);
> + if (adev->gfx.num_compute_rings) {
> + /* recalculate compute rings to use based on hardware configuration */
> + num_compute_rings = (adev->gfx.mec.num_pipe_per_mec *
> + adev->gfx.mec.num_queue_per_pipe) / 2;
> + adev->gfx.num_compute_rings = min(adev->gfx.num_compute_rings,
> + num_compute_rings);
> + }
>
> /* EOP Event */
> r = amdgpu_irq_add_id(adev, SOC21_IH_CLIENTID_GRBM_CP,
> @@ -1471,37 +1473,41 @@ static int gfx_v12_0_sw_init(struct amdgpu_ip_block *ip_block)
> return r;
> }
>
> - /* set up the gfx ring */
> - for (i = 0; i < adev->gfx.me.num_me; i++) {
> - for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
> - for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
> - if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
> - continue;
> -
> - r = gfx_v12_0_gfx_ring_init(adev, ring_id,
> - i, k, j);
> - if (r)
> - return r;
> - ring_id++;
> + if (adev->gfx.num_gfx_rings) {
> + /* set up the gfx ring */
> + for (i = 0; i < adev->gfx.me.num_me; i++) {
> + for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
> + for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
> + if (!amdgpu_gfx_is_me_queue_enabled(adev, i, k, j))
> + continue;
> +
> + r = gfx_v12_0_gfx_ring_init(adev, ring_id,
> + i, k, j);
> + if (r)
> + return r;
> + ring_id++;
> + }
> }
> }
> }
>
> - ring_id = 0;
> - /* set up the compute queues - allocate horizontally across pipes */
> - for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
> - for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
> - for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
> - if (!amdgpu_gfx_is_mec_queue_enabled(adev,
> - 0, i, k, j))
> - continue;
> + if (adev->gfx.num_compute_rings) {
> + ring_id = 0;
> + /* set up the compute queues - allocate horizontally across pipes */
> + for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
> + for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
> + for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
> + if (!amdgpu_gfx_is_mec_queue_enabled(adev,
> + 0, i, k, j))
> + continue;
>
> - r = gfx_v12_0_compute_ring_init(adev, ring_id,
> - i, k, j);
> - if (r)
> - return r;
> + r = gfx_v12_0_compute_ring_init(adev, ring_id,
> + i, k, j);
> + if (r)
> + return r;
>
> - ring_id++;
> + ring_id++;
> + }
> }
> }
> }
> @@ -3495,12 +3501,18 @@ static int gfx_v12_0_cp_resume(struct amdgpu_device *adev)
> if (r)
> return r;
>
> - if (!amdgpu_async_gfx_ring) {
> - r = gfx_v12_0_cp_gfx_resume(adev);
> - if (r)
> - return r;
> + if (adev->gfx.num_gfx_rings) {
> + if (!amdgpu_async_gfx_ring) {
> + r = gfx_v12_0_cp_gfx_resume(adev);
> + if (r)
> + return r;
> + } else {
> + r = gfx_v12_0_cp_async_gfx_ring_resume(adev);
> + if (r)
> + return r;
> + }
> } else {
> - r = gfx_v12_0_cp_async_gfx_ring_resume(adev);
> + r = gfx_v12_0_cp_gfx_start(adev);
> if (r)
> return r;
> }
> @@ -3711,6 +3723,46 @@ static int gfx_v12_0_hw_init(struct amdgpu_ip_block *ip_block)
> return r;
> }
>
> +static int gfx_v12_0_set_userq_eop_interrupts(struct amdgpu_device *adev,
> + bool enable)
> +{
> + if (adev->gfx.disable_kq) {
> + unsigned int irq_type;
> + int m, p, r;
> +
> + 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;
> + if (enable)
> + r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
> + irq_type);
> + else
> + r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
> + irq_type);
> + if (r)
> + return r;
> + }
> + }
> +
> + 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
> + + (m * adev->gfx.mec.num_pipe_per_mec)
> + + p;
> + if (enable)
> + r = amdgpu_irq_get(adev, &adev->gfx.eop_irq,
> + irq_type);
> + else
> + r = amdgpu_irq_put(adev, &adev->gfx.eop_irq,
> + irq_type);
> + if (r)
> + return r;
> + }
> + }
> + }
> + return 0;
> +}
> +
> static int gfx_v12_0_hw_fini(struct amdgpu_ip_block *ip_block)
> {
> struct amdgpu_device *adev = ip_block->adev;
> @@ -3721,6 +3773,7 @@ static int gfx_v12_0_hw_fini(struct amdgpu_ip_block *ip_block)
> amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
> amdgpu_irq_put(adev, &adev->gfx.priv_inst_irq, 0);
> amdgpu_irq_put(adev, &adev->gfx.bad_op_irq, 0);
> + gfx_v12_0_set_userq_eop_interrupts(adev, false);
>
> if (!adev->no_hw_access) {
> if (amdgpu_async_gfx_ring) {
> @@ -3809,11 +3862,19 @@ static int gfx_v12_0_early_init(struct amdgpu_ip_block *ip_block)
> {
> struct amdgpu_device *adev = ip_block->adev;
>
> + if (amdgpu_disable_kq == 1)
> + adev->gfx.disable_kq = true;
> +
> adev->gfx.funcs = &gfx_v12_0_gfx_funcs;
>
> - adev->gfx.num_gfx_rings = GFX12_NUM_GFX_RINGS;
> - adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
> - AMDGPU_MAX_COMPUTE_RINGS);
> + if (adev->gfx.disable_kq) {
> + adev->gfx.num_gfx_rings = 0;
> + adev->gfx.num_compute_rings = 0;
> + } else {
> + adev->gfx.num_gfx_rings = GFX12_NUM_GFX_RINGS;
> + adev->gfx.num_compute_rings = min(amdgpu_gfx_get_num_kcq(adev),
> + AMDGPU_MAX_COMPUTE_RINGS);
> + }
>
> gfx_v12_0_set_kiq_pm4_funcs(adev);
> gfx_v12_0_set_ring_funcs(adev);
> @@ -3844,6 +3905,10 @@ static int gfx_v12_0_late_init(struct amdgpu_ip_block *ip_block)
> if (r)
> return r;
>
> + r = gfx_v12_0_set_userq_eop_interrupts(adev, true);
> + if (r)
> + return r;
> +
> return 0;
> }
>
> @@ -5043,27 +5108,29 @@ static void gfx_v12_0_handle_priv_fault(struct amdgpu_device *adev,
> pipe_id = (entry->ring_id & 0x03) >> 0;
> queue_id = (entry->ring_id & 0x70) >> 4;
>
> - switch (me_id) {
> - case 0:
> - for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> - ring = &adev->gfx.gfx_ring[i];
> - if (ring->me == me_id && ring->pipe == pipe_id &&
> - ring->queue == queue_id)
> - drm_sched_fault(&ring->sched);
> - }
> - break;
> - case 1:
> - case 2:
> - for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> - ring = &adev->gfx.compute_ring[i];
> - if (ring->me == me_id && ring->pipe == pipe_id &&
> - ring->queue == queue_id)
> - drm_sched_fault(&ring->sched);
> + if (!adev->gfx.disable_kq) {
> + switch (me_id) {
> + case 0:
> + for (i = 0; i < adev->gfx.num_gfx_rings; i++) {
> + ring = &adev->gfx.gfx_ring[i];
> + if (ring->me == me_id && ring->pipe == pipe_id &&
> + ring->queue == queue_id)
> + drm_sched_fault(&ring->sched);
> + }
> + break;
> + case 1:
> + case 2:
> + for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> + ring = &adev->gfx.compute_ring[i];
> + if (ring->me == me_id && ring->pipe == pipe_id &&
> + ring->queue == queue_id)
> + drm_sched_fault(&ring->sched);
> + }
> + break;
> + default:
> + BUG();
> + break;
> }
> - break;
> - default:
> - BUG();
> - break;
> }
> }
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 09/11] drm/amdgpu/sdma: add flag for tracking disable_kq
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
` (7 preceding siblings ...)
2025-03-13 14:41 ` [PATCH 08/11] drm/amdgpu/gfx12: " Alex Deucher
@ 2025-03-13 14:41 ` Alex Deucher
2025-03-13 14:41 ` [PATCH 10/11] drm/amdgpu/sdma6: add support for disable_kq Alex Deucher
` (2 subsequent siblings)
11 siblings, 0 replies; 41+ messages in thread
From: Alex Deucher @ 2025-03-13 14:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Sunil Khatri
For SDMA, we still need kernel queues for paging so
they need to be initialized, but we no not want to
accept submissions from userspace when disable_kq
is set.
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
index 9651693200655..edc856e10337a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
@@ -129,6 +129,7 @@ struct amdgpu_sdma {
/* track guilty state of GFX and PAGE queues */
bool gfx_guilty;
bool page_guilty;
+ bool no_user_submission;
};
/*
--
2.48.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 10/11] drm/amdgpu/sdma6: add support for disable_kq
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
` (8 preceding siblings ...)
2025-03-13 14:41 ` [PATCH 09/11] drm/amdgpu/sdma: add flag for tracking disable_kq Alex Deucher
@ 2025-03-13 14:41 ` Alex Deucher
2025-03-13 14:41 ` [PATCH 11/11] drm/amdgpu/sdma7: " Alex Deucher
2025-03-13 22:21 ` [PATCH V5 00/11] Add disable kernel queue support Rodrigo Siqueira
11 siblings, 0 replies; 41+ messages in thread
From: Alex Deucher @ 2025-03-13 14:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Sunil Khatri
When the parameter is set, disable user submissions
to kernel queues.
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
index 3aa4fec4d9e4a..bcc72737f8084 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
@@ -1304,6 +1304,9 @@ static int sdma_v6_0_early_init(struct amdgpu_ip_block *ip_block)
struct amdgpu_device *adev = ip_block->adev;
int r;
+ if (amdgpu_disable_kq == 1)
+ adev->sdma.no_user_submission = true;
+
r = amdgpu_sdma_init_microcode(adev, 0, true);
if (r)
return r;
@@ -1338,6 +1341,7 @@ static int sdma_v6_0_sw_init(struct amdgpu_ip_block *ip_block)
ring->ring_obj = NULL;
ring->use_doorbell = true;
ring->me = i;
+ ring->no_user_submission = adev->sdma.no_user_submission;
DRM_DEBUG("SDMA %d use_doorbell being set to: [%s]\n", i,
ring->use_doorbell?"true":"false");
--
2.48.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 11/11] drm/amdgpu/sdma7: add support for disable_kq
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
` (9 preceding siblings ...)
2025-03-13 14:41 ` [PATCH 10/11] drm/amdgpu/sdma6: add support for disable_kq Alex Deucher
@ 2025-03-13 14:41 ` Alex Deucher
2025-03-13 22:10 ` Rodrigo Siqueira
2025-03-13 22:21 ` [PATCH V5 00/11] Add disable kernel queue support Rodrigo Siqueira
11 siblings, 1 reply; 41+ messages in thread
From: Alex Deucher @ 2025-03-13 14:41 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Sunil Khatri
When the parameter is set, disable user submissions
to kernel queues.
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
index 92a79296708ae..40d45f738c0a8 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
@@ -1316,6 +1316,9 @@ static int sdma_v7_0_early_init(struct amdgpu_ip_block *ip_block)
struct amdgpu_device *adev = ip_block->adev;
int r;
+ if (amdgpu_disable_kq == 1)
+ adev->sdma.no_user_submission = true;
+
r = amdgpu_sdma_init_microcode(adev, 0, true);
if (r) {
DRM_ERROR("Failed to init sdma firmware!\n");
@@ -1351,6 +1354,7 @@ static int sdma_v7_0_sw_init(struct amdgpu_ip_block *ip_block)
ring->ring_obj = NULL;
ring->use_doorbell = true;
ring->me = i;
+ ring->no_user_submission = adev->sdma.no_user_submission;
DRM_DEBUG("SDMA %d use_doorbell being set to: [%s]\n", i,
ring->use_doorbell?"true":"false");
--
2.48.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 11/11] drm/amdgpu/sdma7: add support for disable_kq
2025-03-13 14:41 ` [PATCH 11/11] drm/amdgpu/sdma7: " Alex Deucher
@ 2025-03-13 22:10 ` Rodrigo Siqueira
0 siblings, 0 replies; 41+ messages in thread
From: Rodrigo Siqueira @ 2025-03-13 22:10 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx, Sunil Khatri
On 03/13, Alex Deucher wrote:
> When the parameter is set, disable user submissions
> to kernel queues.
>
> Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
> index 92a79296708ae..40d45f738c0a8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
> @@ -1316,6 +1316,9 @@ static int sdma_v7_0_early_init(struct amdgpu_ip_block *ip_block)
> struct amdgpu_device *adev = ip_block->adev;
> int r;
>
> + if (amdgpu_disable_kq == 1)
> + adev->sdma.no_user_submission = true;
> +
> r = amdgpu_sdma_init_microcode(adev, 0, true);
> if (r) {
> DRM_ERROR("Failed to init sdma firmware!\n");
> @@ -1351,6 +1354,7 @@ static int sdma_v7_0_sw_init(struct amdgpu_ip_block *ip_block)
> ring->ring_obj = NULL;
> ring->use_doorbell = true;
> ring->me = i;
> + ring->no_user_submission = adev->sdma.no_user_submission;
>
> DRM_DEBUG("SDMA %d use_doorbell being set to: [%s]\n", i,
> ring->use_doorbell?"true":"false");
> --
> 2.48.1
>
Hi Alex,
I think patch 9-11 could be a squashed in a single one.
Thanks
--
Rodrigo Siqueira
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V5 00/11] Add disable kernel queue support
2025-03-13 14:41 [PATCH V5 00/11] Add disable kernel queue support Alex Deucher
` (10 preceding siblings ...)
2025-03-13 14:41 ` [PATCH 11/11] drm/amdgpu/sdma7: " Alex Deucher
@ 2025-03-13 22:21 ` Rodrigo Siqueira
2025-03-14 2:28 ` Alex Deucher
11 siblings, 1 reply; 41+ messages in thread
From: Rodrigo Siqueira @ 2025-03-13 22:21 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx
n 03/13, Alex Deucher wrote:
> To better evaluate user queues, add a module parameter
> to disable kernel queues. With this set kernel queues
> are disabled and only user queues are available. This
> frees up hardware resources for use in user queues which
> would otherwise be used by kernel queues and provides
> a way to validate user queues without the presence
> of kernel queues.
Hi Alex,
I'm trying to understand how GFX and MES deal with different queues, and
I used this patchset to guide me through that. In this sense, could you
help me with the following points?
FWIU, the GFX has what are called pipes, which in turn have hardware
queues associated with them. For example, a GFX can have 2 pipes, and
each pipe could have 2 hardware queues; or it could have 1 pipe and 8
queue. Is this correct?
(for this next part, suppose 1 pipe 2 hardware queues)
By default, one of the hardware queues is reserved for the Kernel Queue,
and the user space could use the other. GFX has the MES block "connected"
to all pipe queues, and MES is responsible for scheduling different ring
buffers (in memory) in the pipe's hardware queue (effectively making the
ring active). However, since the kernel queue is always present, MES
only performs scheduling in one of the hardware queues. This scheduling
occurs with the MES mapping and unmapping available Rings in memory to
the hardware queue.
Does the above description sound correct to you? How about the below
diagram? Does it look correct to you?
(I hope the diagram looks fine in your email client; if not, I can
attach a picture of it.)
+-------------------------------------------------------------------------------------------------------------------------------------------+
| GFX |
| |
| +-----------------------------+ |
| +---------------------------------------------+ (Hw Queue 0) | Kernel Queue (No eviction) +------- No MES Scheduling |
| | (Hardware Queue 0) | ------------------->| | | |
|PIPE 0 | ------------------------------------- | +-----------------------------+ X |
| | (Hardware Queue 1) | +----------+---------+ |
| | ------------------------------------- |--+ | | |
| | | | +----------------------------+ | | |
| +---------------------------------------------+ | (Hw Queue 1) | | | MES Schedules | |
| +----------------> | User Queue +-----+ | |
| | | | | |
| +----------------------------+ | | |
| +--------------------+ |
| | |
| +-------------------------------------+ |
| |Un/Map Ring |
| | |
+-------------------------------------------------------------------------------------------------------------------------------------------+
|
+---------------------+--------------------------------------------+
| MEMORY v |
| |
| |
| +----------+ |
| | | +---------+ +--------+ |
| | Ring 0| | Ring 1 | ... | Ring N | |
| | | | | | | |
| +----------+ +---------+ +--------+ |
| |
| |
+------------------------------------------------------------------+
Is the idea in this series to experiment with making the kernel queue
not fully occupy one of the hardware queue? By making the kernel queue
able to be scheduled, this would provide one extra queue to be used for
other things. Is this correct?
I'm unsure if I fully understand this series's idea; please correct me
if I'm wrong.
Also, please elaborate more on the type of tasks that the kernel queue
handles. Tbh, I did not fully understand the idea behind it.
Thanks
>
> v2: use num_gfx_rings and num_compute_rings per
> Felix suggestion
> v3: include num_gfx_rings fix in amdgpu_gfx.c
> v4: additional fixes
> v5: MEC EOP interrupt handling fix (Sunil)
>
> Alex Deucher (11):
> drm/amdgpu: add parameter to disable kernel queues
> drm/amdgpu: add ring flag for no user submissions
> drm/amdgpu/gfx: add generic handling for disable_kq
> drm/amdgpu/mes: centralize gfx_hqd mask management
> drm/amdgpu/mes: update hqd masks when disable_kq is set
> drm/amdgpu/mes: make more vmids available when disable_kq=1
> drm/amdgpu/gfx11: add support for disable_kq
> drm/amdgpu/gfx12: add support for disable_kq
> drm/amdgpu/sdma: add flag for tracking disable_kq
> drm/amdgpu/sdma6: add support for disable_kq
> drm/amdgpu/sdma7: add support for disable_kq
>
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 8 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 30 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 26 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h | 1 +
> drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 191 ++++++++++++++++-------
> drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 183 +++++++++++++++-------
> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 16 +-
> drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 15 +-
> drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 4 +
> drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c | 4 +
> 17 files changed, 345 insertions(+), 155 deletions(-)
>
> --
> 2.48.1
>
--
Rodrigo Siqueira
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH V5 00/11] Add disable kernel queue support
2025-03-13 22:21 ` [PATCH V5 00/11] Add disable kernel queue support Rodrigo Siqueira
@ 2025-03-14 2:28 ` Alex Deucher
2025-03-18 17:46 ` Rodrigo Siqueira
0 siblings, 1 reply; 41+ messages in thread
From: Alex Deucher @ 2025-03-14 2:28 UTC (permalink / raw)
To: Rodrigo Siqueira; +Cc: Alex Deucher, amd-gfx
On Thu, Mar 13, 2025 at 6:21 PM Rodrigo Siqueira <siqueira@igalia.com> wrote:
>
> n 03/13, Alex Deucher wrote:
> > To better evaluate user queues, add a module parameter
> > to disable kernel queues. With this set kernel queues
> > are disabled and only user queues are available. This
> > frees up hardware resources for use in user queues which
> > would otherwise be used by kernel queues and provides
> > a way to validate user queues without the presence
> > of kernel queues.
>
> Hi Alex,
>
> I'm trying to understand how GFX and MES deal with different queues, and
> I used this patchset to guide me through that. In this sense, could you
> help me with the following points?
>
> FWIU, the GFX has what are called pipes, which in turn have hardware
> queues associated with them. For example, a GFX can have 2 pipes, and
> each pipe could have 2 hardware queues; or it could have 1 pipe and 8
> queue. Is this correct?
Right. For gfx, compute, and SDMA you have pipes (called instances on
SDMA) and queues. A pipe can only execute one queue at a time. The
pipe will switch between all of the mapped queues. You have storage
in memory (called an MDQ -- Memory Queue Descriptor) which defines the
state of the queue (GPU virtual addresses of the queue itself, save
areas, doorbell, etc.). The queues that the pipe switches between are
defined by HQDs (Hardware Queue Descriptors). These are basically
register based memory for the queues that the pipe can switch between.
The driver sets up an MQD for each queue that it creates. The MQDs
are then handed to the MES firmware for mapping. The MES firmware can
map a queue as a legacy queue (i.e. a kernel queue) or a user queue.
The difference is that a legacy queue is statically mapped to a HQD
and is never preempted. User queues are dynamically mapped to the
HQDs by the MES firmware. If there are more MQDs than HQDs, the MES
firmware will preempt other user queues to make sure each queue gets a
time slice.
>
> (for this next part, suppose 1 pipe 2 hardware queues)
> By default, one of the hardware queues is reserved for the Kernel Queue,
> and the user space could use the other. GFX has the MES block "connected"
> to all pipe queues, and MES is responsible for scheduling different ring
> buffers (in memory) in the pipe's hardware queue (effectively making the
> ring active). However, since the kernel queue is always present, MES
> only performs scheduling in one of the hardware queues. This scheduling
> occurs with the MES mapping and unmapping available Rings in memory to
> the hardware queue.
>
> Does the above description sound correct to you? How about the below
> diagram? Does it look correct to you?
More or less. The MES handles all of the queues (kernel or user).
The only real difference is that kernel queues are statically mapped
to an HQD while user queues are dynamically scheduled in the available
HQDs based on level of over-subscription. E.g., if you have hardware
with 1 pipe and 2 HQDs you could have a kernel queue on 1 HQD and the
MES would schedule all of the user queues on the remaining 1 HQD. If
you don't enable any kernel queues, then you have 2 HQDs that the MES
can use for scheduling user queues.
>
> (I hope the diagram looks fine in your email client; if not, I can
> attach a picture of it.)
>
> +-------------------------------------------------------------------------------------------------------------------------------------------+
> | GFX |
> | |
> | +-----------------------------+ |
> | +---------------------------------------------+ (Hw Queue 0) | Kernel Queue (No eviction) +------- No MES Scheduling |
> | | (Hardware Queue 0) | ------------------->| | | |
> |PIPE 0 | ------------------------------------- | +-----------------------------+ X |
> | | (Hardware Queue 1) | +----------+---------+ |
> | | ------------------------------------- |--+ | | |
> | | | | +----------------------------+ | | |
> | +---------------------------------------------+ | (Hw Queue 1) | | | MES Schedules | |
> | +----------------> | User Queue +-----+ | |
> | | | | | |
> | +----------------------------+ | | |
> | +--------------------+ |
> | | |
> | +-------------------------------------+ |
> | |Un/Map Ring |
> | | |
> +-------------------------------------------------------------------------------------------------------------------------------------------+
> |
> +---------------------+--------------------------------------------+
> | MEMORY v |
> | |
> | |
> | +----------+ |
> | | | +---------+ +--------+ |
> | | Ring 0| | Ring 1 | ... | Ring N | |
> | | | | | | | |
> | +----------+ +---------+ +--------+ |
> | |
> | |
> +------------------------------------------------------------------+
>
> Is the idea in this series to experiment with making the kernel queue
> not fully occupy one of the hardware queue? By making the kernel queue
> able to be scheduled, this would provide one extra queue to be used for
> other things. Is this correct?
Right. This series paves the way for getting rid of kernel queues all
together. Having no kernel queues leaves all of the resources
available to user queues.
>
> I'm unsure if I fully understand this series's idea; please correct me
> if I'm wrong.
>
> Also, please elaborate more on the type of tasks that the kernel queue
> handles. Tbh, I did not fully understand the idea behind it.
In the future of user queues, kernel queues would not be created or
used at all. Today, on most existing hardware, kernel queues are all
that is available. Today, when an application submits work to the
kernel driver, the kernel driver submits all of the application
command buffers to kernel queues. E.g., in most cases there is a
single kernel GFX queue and all applications which want to use the GFX
engine funnel into that queue. The CS IOCTL basically takes the
command buffers from the applications and schedules them on the kernel
queue. With user queues, each application will create its own user
queues and will submit work directly to its user queues. No need for
an IOCTL for each submission, no need to share a single kernel queue,
etc.
Alex
>
> Thanks
>
> >
> > v2: use num_gfx_rings and num_compute_rings per
> > Felix suggestion
> > v3: include num_gfx_rings fix in amdgpu_gfx.c
> > v4: additional fixes
> > v5: MEC EOP interrupt handling fix (Sunil)
> >
> > Alex Deucher (11):
> > drm/amdgpu: add parameter to disable kernel queues
> > drm/amdgpu: add ring flag for no user submissions
> > drm/amdgpu/gfx: add generic handling for disable_kq
> > drm/amdgpu/mes: centralize gfx_hqd mask management
> > drm/amdgpu/mes: update hqd masks when disable_kq is set
> > drm/amdgpu/mes: make more vmids available when disable_kq=1
> > drm/amdgpu/gfx11: add support for disable_kq
> > drm/amdgpu/gfx12: add support for disable_kq
> > drm/amdgpu/sdma: add flag for tracking disable_kq
> > drm/amdgpu/sdma6: add support for disable_kq
> > drm/amdgpu/sdma7: add support for disable_kq
> >
> > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 ++
> > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 8 +-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 +
> > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 30 ++--
> > drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 26 ++-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h | 1 +
> > drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 191 ++++++++++++++++-------
> > drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 183 +++++++++++++++-------
> > drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 16 +-
> > drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 15 +-
> > drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 4 +
> > drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c | 4 +
> > 17 files changed, 345 insertions(+), 155 deletions(-)
> >
> > --
> > 2.48.1
> >
>
> --
> Rodrigo Siqueira
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V5 00/11] Add disable kernel queue support
2025-03-14 2:28 ` Alex Deucher
@ 2025-03-18 17:46 ` Rodrigo Siqueira
2025-03-18 20:06 ` Alex Deucher
0 siblings, 1 reply; 41+ messages in thread
From: Rodrigo Siqueira @ 2025-03-18 17:46 UTC (permalink / raw)
To: Alex Deucher; +Cc: Alex Deucher, amd-gfx
On 03/13, Alex Deucher wrote:
> On Thu, Mar 13, 2025 at 6:21 PM Rodrigo Siqueira <siqueira@igalia.com> wrote:
> >
> > n 03/13, Alex Deucher wrote:
> > > To better evaluate user queues, add a module parameter
> > > to disable kernel queues. With this set kernel queues
> > > are disabled and only user queues are available. This
> > > frees up hardware resources for use in user queues which
> > > would otherwise be used by kernel queues and provides
> > > a way to validate user queues without the presence
> > > of kernel queues.
> >
> > Hi Alex,
> >
> > I'm trying to understand how GFX and MES deal with different queues, and
> > I used this patchset to guide me through that. In this sense, could you
> > help me with the following points?
> >
> > FWIU, the GFX has what are called pipes, which in turn have hardware
> > queues associated with them. For example, a GFX can have 2 pipes, and
> > each pipe could have 2 hardware queues; or it could have 1 pipe and 8
> > queue. Is this correct?
>
Hi Alex, first of all, thanks a lot for your detailed explanation.
I still have some other questions, see it inline.
> Right. For gfx, compute, and SDMA you have pipes (called instances on
> SDMA) and queues. A pipe can only execute one queue at a time. The
What is the difference between GFX and Compute? Tbh, I thought they were
the same component.
I was also thinking about the concept of a pipe, and I'm trying to
define what a pipe is in this context (the word pipe is one of those
words with many meanings in computers). Is the below definition accurate
enough?
Pipe, in the context of GFX, Compute, and SDMA, is a mechanism for
running threads.
> pipe will switch between all of the mapped queues. You have storage
Above, you said that each pipe will switch between queues, and a little
bit below, in your explanation about MES, you said:
[..] If there are more MQDs than HQDs, the MES firmware will preempt
other user queues to make sure each queue gets a time slice.
Does it mean that the GFX Pipe has the mechanic of switching queues
while MES has the scheduling logic?
Does the below example and explanation make sense?
Suppose the following scenario:
- One pipe (pipe0) and two queues (queue[0], and queue[1]).
- 3 MQDs (mqd[0], mqd[1], and mqd[2]).
- pipe0 is running an user queue in queue[1].
- pipe0 is running a kernel queue in queue[0].
Fwiu, a pipe can change the current queue in execution, but it does not
do it by itself. In other words, it has no scheduling logic; it only has
the mechanics of switching queues inside it. When the pipe switches
between queues, it uses Mid Command Buffer Preemption (MCBP), which
saves some very basic information but has no register state; in other
words, those registers must be stored in memory (MES handles it?).
In turn, MES has access to all MQDs handed over to it, which means that
MES has all the queue states available for the scheduling and
communication with the GFX pipe. Suppose that the GFX pipe is running
mqd[2] in the queue[1], and now MES wants to replace it with mqd[0]. The
communication will be something like the following:
1. MES to GFX pipe0: Replace(mqd[2], in pipe0, queue[1]) with mqd[0].
2. GFX pipe0: Just stop the current pipe, and start mqd[0].
Does it looks correct to you?
> in memory (called an MDQ -- Memory Queue Descriptor) which defines the
> state of the queue (GPU virtual addresses of the queue itself, save
> areas, doorbell, etc.). The queues that the pipe switches between are
> defined by HQDs (Hardware Queue Descriptors). These are basically
> register based memory for the queues that the pipe can switch between.
I was thinking about this register-based memory part. Does it mean that
switching between it is just a matter of updating one of those LOW and
HIGH registers?
> The driver sets up an MQD for each queue that it creates. The MQDs
> are then handed to the MES firmware for mapping. The MES firmware can
> map a queue as a legacy queue (i.e. a kernel queue) or a user queue.
> The difference is that a legacy queue is statically mapped to a HQD
> and is never preempted. User queues are dynamically mapped to the
> HQDs by the MES firmware. If there are more MQDs than HQDs, the MES
> firmware will preempt other user queues to make sure each queue gets a
> time slice.
>
> >
> > (for this next part, suppose 1 pipe 2 hardware queues)
> > By default, one of the hardware queues is reserved for the Kernel Queue,
> > and the user space could use the other. GFX has the MES block "connected"
> > to all pipe queues, and MES is responsible for scheduling different ring
> > buffers (in memory) in the pipe's hardware queue (effectively making the
> > ring active). However, since the kernel queue is always present, MES
> > only performs scheduling in one of the hardware queues. This scheduling
> > occurs with the MES mapping and unmapping available Rings in memory to
> > the hardware queue.
> >
> > Does the above description sound correct to you? How about the below
> > diagram? Does it look correct to you?
>
> More or less. The MES handles all of the queues (kernel or user).
> The only real difference is that kernel queues are statically mapped
> to an HQD while user queues are dynamically scheduled in the available
> HQDs based on level of over-subscription. E.g., if you have hardware
> with 1 pipe and 2 HQDs you could have a kernel queue on 1 HQD and the
> MES would schedule all of the user queues on the remaining 1 HQD. If
> you don't enable any kernel queues, then you have 2 HQDs that the MES
> can use for scheduling user queues.
>
> >
> > (I hope the diagram looks fine in your email client; if not, I can
> > attach a picture of it.)
> >
> > +-------------------------------------------------------------------------------------------------------------------------------------------+
> > | GFX |
> > | |
> > | +-----------------------------+ |
> > | +---------------------------------------------+ (Hw Queue 0) | Kernel Queue (No eviction) +------- No MES Scheduling |
> > | | (Hardware Queue 0) | ------------------->| | | |
> > |PIPE 0 | ------------------------------------- | +-----------------------------+ X |
> > | | (Hardware Queue 1) | +----------+---------+ |
> > | | ------------------------------------- |--+ | | |
> > | | | | +----------------------------+ | | |
> > | +---------------------------------------------+ | (Hw Queue 1) | | | MES Schedules | |
> > | +----------------> | User Queue +-----+ | |
> > | | | | | |
> > | +----------------------------+ | | |
> > | +--------------------+ |
> > | | |
> > | +-------------------------------------+ |
> > | |Un/Map Ring |
> > | | |
> > +-------------------------------------------------------------------------------------------------------------------------------------------+
> > |
> > +---------------------+--------------------------------------------+
> > | MEMORY v |
> > | |
> > | |
> > | +----------+ |
> > | | | +---------+ +--------+ |
> > | | Ring 0| | Ring 1 | ... | Ring N | |
> > | | | | | | | |
> > | +----------+ +---------+ +--------+ |
> > | |
> > | |
> > +------------------------------------------------------------------+
> >
> > Is the idea in this series to experiment with making the kernel queue
> > not fully occupy one of the hardware queue? By making the kernel queue
> > able to be scheduled, this would provide one extra queue to be used for
> > other things. Is this correct?
>
> Right. This series paves the way for getting rid of kernel queues all
> together. Having no kernel queues leaves all of the resources
> available to user queues.
Another question: I guess kernel queues use VMID 0, and all of the other
user queues will use a different VMID, right? Does the VMID matter for
this transition to make the kernel queue legacy?
Thanks
>
> >
> > I'm unsure if I fully understand this series's idea; please correct me
> > if I'm wrong.
> >
> > Also, please elaborate more on the type of tasks that the kernel queue
> > handles. Tbh, I did not fully understand the idea behind it.
>
> In the future of user queues, kernel queues would not be created or
> used at all. Today, on most existing hardware, kernel queues are all
> that is available. Today, when an application submits work to the
> kernel driver, the kernel driver submits all of the application
> command buffers to kernel queues. E.g., in most cases there is a
> single kernel GFX queue and all applications which want to use the GFX
> engine funnel into that queue. The CS IOCTL basically takes the
> command buffers from the applications and schedules them on the kernel
> queue. With user queues, each application will create its own user
> queues and will submit work directly to its user queues. No need for
> an IOCTL for each submission, no need to share a single kernel queue,
> etc.
>
> Alex
>
> >
> > Thanks
> >
> > >
> > > v2: use num_gfx_rings and num_compute_rings per
> > > Felix suggestion
> > > v3: include num_gfx_rings fix in amdgpu_gfx.c
> > > v4: additional fixes
> > > v5: MEC EOP interrupt handling fix (Sunil)
> > >
> > > Alex Deucher (11):
> > > drm/amdgpu: add parameter to disable kernel queues
> > > drm/amdgpu: add ring flag for no user submissions
> > > drm/amdgpu/gfx: add generic handling for disable_kq
> > > drm/amdgpu/mes: centralize gfx_hqd mask management
> > > drm/amdgpu/mes: update hqd masks when disable_kq is set
> > > drm/amdgpu/mes: make more vmids available when disable_kq=1
> > > drm/amdgpu/gfx11: add support for disable_kq
> > > drm/amdgpu/gfx12: add support for disable_kq
> > > drm/amdgpu/sdma: add flag for tracking disable_kq
> > > drm/amdgpu/sdma6: add support for disable_kq
> > > drm/amdgpu/sdma7: add support for disable_kq
> > >
> > > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 ++
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 8 +-
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 +
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 30 ++--
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 26 ++-
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h | 1 +
> > > drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 191 ++++++++++++++++-------
> > > drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 183 +++++++++++++++-------
> > > drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 2 +-
> > > drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c | 2 +-
> > > drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 16 +-
> > > drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 15 +-
> > > drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 4 +
> > > drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c | 4 +
> > > 17 files changed, 345 insertions(+), 155 deletions(-)
> > >
> > > --
> > > 2.48.1
> > >
> >
> > --
> > Rodrigo Siqueira
--
Rodrigo Siqueira
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V5 00/11] Add disable kernel queue support
2025-03-18 17:46 ` Rodrigo Siqueira
@ 2025-03-18 20:06 ` Alex Deucher
2025-03-25 17:32 ` Rodrigo Siqueira
0 siblings, 1 reply; 41+ messages in thread
From: Alex Deucher @ 2025-03-18 20:06 UTC (permalink / raw)
To: Rodrigo Siqueira; +Cc: Alex Deucher, amd-gfx
On Tue, Mar 18, 2025 at 1:46 PM Rodrigo Siqueira <siqueira@igalia.com> wrote:
>
> On 03/13, Alex Deucher wrote:
> > On Thu, Mar 13, 2025 at 6:21 PM Rodrigo Siqueira <siqueira@igalia.com> wrote:
> > >
> > > n 03/13, Alex Deucher wrote:
> > > > To better evaluate user queues, add a module parameter
> > > > to disable kernel queues. With this set kernel queues
> > > > are disabled and only user queues are available. This
> > > > frees up hardware resources for use in user queues which
> > > > would otherwise be used by kernel queues and provides
> > > > a way to validate user queues without the presence
> > > > of kernel queues.
> > >
> > > Hi Alex,
> > >
> > > I'm trying to understand how GFX and MES deal with different queues, and
> > > I used this patchset to guide me through that. In this sense, could you
> > > help me with the following points?
> > >
> > > FWIU, the GFX has what are called pipes, which in turn have hardware
> > > queues associated with them. For example, a GFX can have 2 pipes, and
> > > each pipe could have 2 hardware queues; or it could have 1 pipe and 8
> > > queue. Is this correct?
> >
>
> Hi Alex, first of all, thanks a lot for your detailed explanation.
>
> I still have some other questions, see it inline.
>
> > Right. For gfx, compute, and SDMA you have pipes (called instances on
> > SDMA) and queues. A pipe can only execute one queue at a time. The
>
> What is the difference between GFX and Compute? Tbh, I thought they were
> the same component.
They both share access to the shader cores, but they have different
front ends. GFX has a bunch of fixed function blocks used by draws
while compute dispatches directly to the shaders. There are separate
pipes for each. You can send dispatch packets to GFX, but you can't
send draw packets to compute.
>
> I was also thinking about the concept of a pipe, and I'm trying to
> define what a pipe is in this context (the word pipe is one of those
> words with many meanings in computers). Is the below definition accurate
> enough?
>
> Pipe, in the context of GFX, Compute, and SDMA, is a mechanism for
> running threads.
Yes. It's the hardware that actually processes the packets in a queue.
You have multiple HQDs associated with a pipe, only one will be
processed by a pipe at a time.
>
> > pipe will switch between all of the mapped queues. You have storage
>
> Above, you said that each pipe will switch between queues, and a little
> bit below, in your explanation about MES, you said:
>
> [..] If there are more MQDs than HQDs, the MES firmware will preempt
> other user queues to make sure each queue gets a time slice.
>
> Does it mean that the GFX Pipe has the mechanic of switching queues
> while MES has the scheduling logic?
The pipes have hardware logic to switch between the HQD slots. MES is
a separate microcontroller which handles the mapping and unmapping of
MQDs into HQDs. It handles priorities and oversubscription (more MQDs
than HQDs).
>
> Does the below example and explanation make sense?
>
> Suppose the following scenario:
> - One pipe (pipe0) and two queues (queue[0], and queue[1]).
> - 3 MQDs (mqd[0], mqd[1], and mqd[2]).
> - pipe0 is running an user queue in queue[1].
> - pipe0 is running a kernel queue in queue[0].
Yes. A pipe can only execute one queue at a time, it will dynamically
switch between the active HQDs.
>
> Fwiu, a pipe can change the current queue in execution, but it does not
> do it by itself. In other words, it has no scheduling logic; it only has
> the mechanics of switching queues inside it. When the pipe switches
> between queues, it uses Mid Command Buffer Preemption (MCBP), which
> saves some very basic information but has no register state; in other
> words, those registers must be stored in memory (MES handles it?).
More or less. A pipe will switch between queues on a command stream
or on a packet by packet basis, depending on the engine. You can
preempt a queue if you want. In general the driver will ask MES to do
this if it needs to preempt a queue. The MES will also do this
internally for scheduling reasons. MES firmware handles the saving of
state to the MQD.
>
> In turn, MES has access to all MQDs handed over to it, which means that
> MES has all the queue states available for the scheduling and
> communication with the GFX pipe. Suppose that the GFX pipe is running
> mqd[2] in the queue[1], and now MES wants to replace it with mqd[0]. The
> communication will be something like the following:
>
> 1. MES to GFX pipe0: Replace(mqd[2], in pipe0, queue[1]) with mqd[0].
> 2. GFX pipe0: Just stop the current pipe, and start mqd[0].
>
> Does it looks correct to you?
MES would talk to the hardware to unmap queue[1] and save its state to
mqd[2]. It would then talk to the hardware to map the state from
mqd[0] into queue[1].
>
> > in memory (called an MDQ -- Memory Queue Descriptor) which defines the
> > state of the queue (GPU virtual addresses of the queue itself, save
> > areas, doorbell, etc.). The queues that the pipe switches between are
> > defined by HQDs (Hardware Queue Descriptors). These are basically
> > register based memory for the queues that the pipe can switch between.
>
> I was thinking about this register-based memory part. Does it mean that
> switching between it is just a matter of updating one of those LOW and
> HIGH registers?
Not exactly, but close. The HQD registers are saved in/out of the MQD
and the MQD also has pointers to other buffers which store other
things like pipeline state, etc. Firmware basically tells the hw to
preempt or umap the queues, waits for that to complete (waits for
HQD_ACTIVE bit for the queue to go low), then saves the state to the
MQD. For resuming or mapping a queue, the opposite happens, firmware
copies the state out of the MQD into the HQD registers and loads any
additional state. Setting the HQD_ACTIVE bit for the queue is what
ultimately enables it.
>
> > The driver sets up an MQD for each queue that it creates. The MQDs
> > are then handed to the MES firmware for mapping. The MES firmware can
> > map a queue as a legacy queue (i.e. a kernel queue) or a user queue.
> > The difference is that a legacy queue is statically mapped to a HQD
> > and is never preempted. User queues are dynamically mapped to the
> > HQDs by the MES firmware. If there are more MQDs than HQDs, the MES
> > firmware will preempt other user queues to make sure each queue gets a
> > time slice.
> >
> > >
> > > (for this next part, suppose 1 pipe 2 hardware queues)
> > > By default, one of the hardware queues is reserved for the Kernel Queue,
> > > and the user space could use the other. GFX has the MES block "connected"
> > > to all pipe queues, and MES is responsible for scheduling different ring
> > > buffers (in memory) in the pipe's hardware queue (effectively making the
> > > ring active). However, since the kernel queue is always present, MES
> > > only performs scheduling in one of the hardware queues. This scheduling
> > > occurs with the MES mapping and unmapping available Rings in memory to
> > > the hardware queue.
> > >
> > > Does the above description sound correct to you? How about the below
> > > diagram? Does it look correct to you?
> >
> > More or less. The MES handles all of the queues (kernel or user).
> > The only real difference is that kernel queues are statically mapped
> > to an HQD while user queues are dynamically scheduled in the available
> > HQDs based on level of over-subscription. E.g., if you have hardware
> > with 1 pipe and 2 HQDs you could have a kernel queue on 1 HQD and the
> > MES would schedule all of the user queues on the remaining 1 HQD. If
> > you don't enable any kernel queues, then you have 2 HQDs that the MES
> > can use for scheduling user queues.
> >
> > >
> > > (I hope the diagram looks fine in your email client; if not, I can
> > > attach a picture of it.)
> > >
> > > +-------------------------------------------------------------------------------------------------------------------------------------------+
> > > | GFX |
> > > | |
> > > | +-----------------------------+ |
> > > | +---------------------------------------------+ (Hw Queue 0) | Kernel Queue (No eviction) +------- No MES Scheduling |
> > > | | (Hardware Queue 0) | ------------------->| | | |
> > > |PIPE 0 | ------------------------------------- | +-----------------------------+ X |
> > > | | (Hardware Queue 1) | +----------+---------+ |
> > > | | ------------------------------------- |--+ | | |
> > > | | | | +----------------------------+ | | |
> > > | +---------------------------------------------+ | (Hw Queue 1) | | | MES Schedules | |
> > > | +----------------> | User Queue +-----+ | |
> > > | | | | | |
> > > | +----------------------------+ | | |
> > > | +--------------------+ |
> > > | | |
> > > | +-------------------------------------+ |
> > > | |Un/Map Ring |
> > > | | |
> > > +-------------------------------------------------------------------------------------------------------------------------------------------+
> > > |
> > > +---------------------+--------------------------------------------+
> > > | MEMORY v |
> > > | |
> > > | |
> > > | +----------+ |
> > > | | | +---------+ +--------+ |
> > > | | Ring 0| | Ring 1 | ... | Ring N | |
> > > | | | | | | | |
> > > | +----------+ +---------+ +--------+ |
> > > | |
> > > | |
> > > +------------------------------------------------------------------+
> > >
> > > Is the idea in this series to experiment with making the kernel queue
> > > not fully occupy one of the hardware queue? By making the kernel queue
> > > able to be scheduled, this would provide one extra queue to be used for
> > > other things. Is this correct?
> >
> > Right. This series paves the way for getting rid of kernel queues all
> > together. Having no kernel queues leaves all of the resources
> > available to user queues.
>
> Another question: I guess kernel queues use VMID 0, and all of the other
> user queues will use a different VMID, right? Does the VMID matter for
> this transition to make the kernel queue legacy?
vmid 0 is the GPU virtual address space used for all kernel driver
operations. For kernel queues, the queue itself operates in the vmid
0 address space, but each command buffer (Indirect Buffer -- IB)
operates in a driver assigned non-0 vmid address space. For kernel
queues, the driver manages the vmids. For user queues, the queue and
IBs both operate in the user's non-0 vmid address space. The MES
manages the vmids assignments for user queues. The driver provides a
pointer to the user's GPU VM page tables and MES assigns a vmid when
it maps the queue. Driver provides a mask which vmids the MES can use
so that there are no conflicts when mixing kernel and user queues.
Alex
>
> Thanks
>
> >
> > >
> > > I'm unsure if I fully understand this series's idea; please correct me
> > > if I'm wrong.
> > >
> > > Also, please elaborate more on the type of tasks that the kernel queue
> > > handles. Tbh, I did not fully understand the idea behind it.
> >
> > In the future of user queues, kernel queues would not be created or
> > used at all. Today, on most existing hardware, kernel queues are all
> > that is available. Today, when an application submits work to the
> > kernel driver, the kernel driver submits all of the application
> > command buffers to kernel queues. E.g., in most cases there is a
> > single kernel GFX queue and all applications which want to use the GFX
> > engine funnel into that queue. The CS IOCTL basically takes the
> > command buffers from the applications and schedules them on the kernel
> > queue. With user queues, each application will create its own user
> > queues and will submit work directly to its user queues. No need for
> > an IOCTL for each submission, no need to share a single kernel queue,
> > etc.
> >
> > Alex
> >
> > >
> > > Thanks
> > >
> > > >
> > > > v2: use num_gfx_rings and num_compute_rings per
> > > > Felix suggestion
> > > > v3: include num_gfx_rings fix in amdgpu_gfx.c
> > > > v4: additional fixes
> > > > v5: MEC EOP interrupt handling fix (Sunil)
> > > >
> > > > Alex Deucher (11):
> > > > drm/amdgpu: add parameter to disable kernel queues
> > > > drm/amdgpu: add ring flag for no user submissions
> > > > drm/amdgpu/gfx: add generic handling for disable_kq
> > > > drm/amdgpu/mes: centralize gfx_hqd mask management
> > > > drm/amdgpu/mes: update hqd masks when disable_kq is set
> > > > drm/amdgpu/mes: make more vmids available when disable_kq=1
> > > > drm/amdgpu/gfx11: add support for disable_kq
> > > > drm/amdgpu/gfx12: add support for disable_kq
> > > > drm/amdgpu/sdma: add flag for tracking disable_kq
> > > > drm/amdgpu/sdma6: add support for disable_kq
> > > > drm/amdgpu/sdma7: add support for disable_kq
> > > >
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 ++
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 8 +-
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 +
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 30 ++--
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 26 ++-
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h | 1 +
> > > > drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 191 ++++++++++++++++-------
> > > > drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 183 +++++++++++++++-------
> > > > drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 2 +-
> > > > drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c | 2 +-
> > > > drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 16 +-
> > > > drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 15 +-
> > > > drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 4 +
> > > > drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c | 4 +
> > > > 17 files changed, 345 insertions(+), 155 deletions(-)
> > > >
> > > > --
> > > > 2.48.1
> > > >
> > >
> > > --
> > > Rodrigo Siqueira
>
> --
> Rodrigo Siqueira
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V5 00/11] Add disable kernel queue support
2025-03-18 20:06 ` Alex Deucher
@ 2025-03-25 17:32 ` Rodrigo Siqueira
0 siblings, 0 replies; 41+ messages in thread
From: Rodrigo Siqueira @ 2025-03-25 17:32 UTC (permalink / raw)
To: Alex Deucher; +Cc: Alex Deucher, amd-gfx
On 03/18, Alex Deucher wrote:
> On Tue, Mar 18, 2025 at 1:46 PM Rodrigo Siqueira <siqueira@igalia.com> wrote:
> >
> > On 03/13, Alex Deucher wrote:
> > > On Thu, Mar 13, 2025 at 6:21 PM Rodrigo Siqueira <siqueira@igalia.com> wrote:
> > > >
> > > > n 03/13, Alex Deucher wrote:
> > > > > To better evaluate user queues, add a module parameter
> > > > > to disable kernel queues. With this set kernel queues
> > > > > are disabled and only user queues are available. This
> > > > > frees up hardware resources for use in user queues which
> > > > > would otherwise be used by kernel queues and provides
> > > > > a way to validate user queues without the presence
> > > > > of kernel queues.
> > > >
> > > > Hi Alex,
> > > >
> > > > I'm trying to understand how GFX and MES deal with different queues, and
> > > > I used this patchset to guide me through that. In this sense, could you
> > > > help me with the following points?
> > > >
> > > > FWIU, the GFX has what are called pipes, which in turn have hardware
> > > > queues associated with them. For example, a GFX can have 2 pipes, and
> > > > each pipe could have 2 hardware queues; or it could have 1 pipe and 8
> > > > queue. Is this correct?
> > >
> >
> > Hi Alex, first of all, thanks a lot for your detailed explanation.
> >
> > I still have some other questions, see it inline.
> >
> > > Right. For gfx, compute, and SDMA you have pipes (called instances on
> > > SDMA) and queues. A pipe can only execute one queue at a time. The
> >
> > What is the difference between GFX and Compute? Tbh, I thought they were
> > the same component.
>
> They both share access to the shader cores, but they have different
> front ends. GFX has a bunch of fixed function blocks used by draws
> while compute dispatches directly to the shaders. There are separate
> pipes for each. You can send dispatch packets to GFX, but you can't
> send draw packets to compute.
>
> >
> > I was also thinking about the concept of a pipe, and I'm trying to
> > define what a pipe is in this context (the word pipe is one of those
> > words with many meanings in computers). Is the below definition accurate
> > enough?
> >
> > Pipe, in the context of GFX, Compute, and SDMA, is a mechanism for
> > running threads.
>
> Yes. It's the hardware that actually processes the packets in a queue.
> You have multiple HQDs associated with a pipe, only one will be
> processed by a pipe at a time.
>
> >
> > > pipe will switch between all of the mapped queues. You have storage
> >
> > Above, you said that each pipe will switch between queues, and a little
> > bit below, in your explanation about MES, you said:
> >
> > [..] If there are more MQDs than HQDs, the MES firmware will preempt
> > other user queues to make sure each queue gets a time slice.
> >
> > Does it mean that the GFX Pipe has the mechanic of switching queues
> > while MES has the scheduling logic?
>
> The pipes have hardware logic to switch between the HQD slots. MES is
> a separate microcontroller which handles the mapping and unmapping of
> MQDs into HQDs. It handles priorities and oversubscription (more MQDs
> than HQDs).
>
> >
> > Does the below example and explanation make sense?
> >
> > Suppose the following scenario:
> > - One pipe (pipe0) and two queues (queue[0], and queue[1]).
> > - 3 MQDs (mqd[0], mqd[1], and mqd[2]).
> > - pipe0 is running an user queue in queue[1].
> > - pipe0 is running a kernel queue in queue[0].
>
> Yes. A pipe can only execute one queue at a time, it will dynamically
> switch between the active HQDs.
>
> >
> > Fwiu, a pipe can change the current queue in execution, but it does not
> > do it by itself. In other words, it has no scheduling logic; it only has
> > the mechanics of switching queues inside it. When the pipe switches
> > between queues, it uses Mid Command Buffer Preemption (MCBP), which
> > saves some very basic information but has no register state; in other
> > words, those registers must be stored in memory (MES handles it?).
>
> More or less. A pipe will switch between queues on a command stream
> or on a packet by packet basis, depending on the engine. You can
> preempt a queue if you want. In general the driver will ask MES to do
> this if it needs to preempt a queue. The MES will also do this
> internally for scheduling reasons. MES firmware handles the saving of
> state to the MQD.
>
> >
> > In turn, MES has access to all MQDs handed over to it, which means that
> > MES has all the queue states available for the scheduling and
> > communication with the GFX pipe. Suppose that the GFX pipe is running
> > mqd[2] in the queue[1], and now MES wants to replace it with mqd[0]. The
> > communication will be something like the following:
> >
> > 1. MES to GFX pipe0: Replace(mqd[2], in pipe0, queue[1]) with mqd[0].
> > 2. GFX pipe0: Just stop the current pipe, and start mqd[0].
> >
> > Does it looks correct to you?
>
> MES would talk to the hardware to unmap queue[1] and save its state to
> mqd[2]. It would then talk to the hardware to map the state from
> mqd[0] into queue[1].
>
> >
> > > in memory (called an MDQ -- Memory Queue Descriptor) which defines the
> > > state of the queue (GPU virtual addresses of the queue itself, save
> > > areas, doorbell, etc.). The queues that the pipe switches between are
> > > defined by HQDs (Hardware Queue Descriptors). These are basically
> > > register based memory for the queues that the pipe can switch between.
> >
> > I was thinking about this register-based memory part. Does it mean that
> > switching between it is just a matter of updating one of those LOW and
> > HIGH registers?
>
> Not exactly, but close. The HQD registers are saved in/out of the MQD
> and the MQD also has pointers to other buffers which store other
> things like pipeline state, etc. Firmware basically tells the hw to
> preempt or umap the queues, waits for that to complete (waits for
> HQD_ACTIVE bit for the queue to go low), then saves the state to the
> MQD. For resuming or mapping a queue, the opposite happens, firmware
> copies the state out of the MQD into the HQD registers and loads any
> additional state. Setting the HQD_ACTIVE bit for the queue is what
> ultimately enables it.
>
> >
> > > The driver sets up an MQD for each queue that it creates. The MQDs
> > > are then handed to the MES firmware for mapping. The MES firmware can
> > > map a queue as a legacy queue (i.e. a kernel queue) or a user queue.
> > > The difference is that a legacy queue is statically mapped to a HQD
> > > and is never preempted. User queues are dynamically mapped to the
> > > HQDs by the MES firmware. If there are more MQDs than HQDs, the MES
> > > firmware will preempt other user queues to make sure each queue gets a
> > > time slice.
> > >
> > > >
> > > > (for this next part, suppose 1 pipe 2 hardware queues)
> > > > By default, one of the hardware queues is reserved for the Kernel Queue,
> > > > and the user space could use the other. GFX has the MES block "connected"
> > > > to all pipe queues, and MES is responsible for scheduling different ring
> > > > buffers (in memory) in the pipe's hardware queue (effectively making the
> > > > ring active). However, since the kernel queue is always present, MES
> > > > only performs scheduling in one of the hardware queues. This scheduling
> > > > occurs with the MES mapping and unmapping available Rings in memory to
> > > > the hardware queue.
> > > >
> > > > Does the above description sound correct to you? How about the below
> > > > diagram? Does it look correct to you?
> > >
> > > More or less. The MES handles all of the queues (kernel or user).
> > > The only real difference is that kernel queues are statically mapped
> > > to an HQD while user queues are dynamically scheduled in the available
> > > HQDs based on level of over-subscription. E.g., if you have hardware
> > > with 1 pipe and 2 HQDs you could have a kernel queue on 1 HQD and the
> > > MES would schedule all of the user queues on the remaining 1 HQD. If
> > > you don't enable any kernel queues, then you have 2 HQDs that the MES
> > > can use for scheduling user queues.
> > >
> > > >
> > > > (I hope the diagram looks fine in your email client; if not, I can
> > > > attach a picture of it.)
> > > >
> > > > +-------------------------------------------------------------------------------------------------------------------------------------------+
> > > > | GFX |
> > > > | |
> > > > | +-----------------------------+ |
> > > > | +---------------------------------------------+ (Hw Queue 0) | Kernel Queue (No eviction) +------- No MES Scheduling |
> > > > | | (Hardware Queue 0) | ------------------->| | | |
> > > > |PIPE 0 | ------------------------------------- | +-----------------------------+ X |
> > > > | | (Hardware Queue 1) | +----------+---------+ |
> > > > | | ------------------------------------- |--+ | | |
> > > > | | | | +----------------------------+ | | |
> > > > | +---------------------------------------------+ | (Hw Queue 1) | | | MES Schedules | |
> > > > | +----------------> | User Queue +-----+ | |
> > > > | | | | | |
> > > > | +----------------------------+ | | |
> > > > | +--------------------+ |
> > > > | | |
> > > > | +-------------------------------------+ |
> > > > | |Un/Map Ring |
> > > > | | |
> > > > +-------------------------------------------------------------------------------------------------------------------------------------------+
> > > > |
> > > > +---------------------+--------------------------------------------+
> > > > | MEMORY v |
> > > > | |
> > > > | |
> > > > | +----------+ |
> > > > | | | +---------+ +--------+ |
> > > > | | Ring 0| | Ring 1 | ... | Ring N | |
> > > > | | | | | | | |
> > > > | +----------+ +---------+ +--------+ |
> > > > | |
> > > > | |
> > > > +------------------------------------------------------------------+
> > > >
> > > > Is the idea in this series to experiment with making the kernel queue
> > > > not fully occupy one of the hardware queue? By making the kernel queue
> > > > able to be scheduled, this would provide one extra queue to be used for
> > > > other things. Is this correct?
> > >
> > > Right. This series paves the way for getting rid of kernel queues all
> > > together. Having no kernel queues leaves all of the resources
> > > available to user queues.
> >
> > Another question: I guess kernel queues use VMID 0, and all of the other
> > user queues will use a different VMID, right? Does the VMID matter for
> > this transition to make the kernel queue legacy?
>
> vmid 0 is the GPU virtual address space used for all kernel driver
> operations. For kernel queues, the queue itself operates in the vmid
> 0 address space, but each command buffer (Indirect Buffer -- IB)
> operates in a driver assigned non-0 vmid address space. For kernel
> queues, the driver manages the vmids. For user queues, the queue and
> IBs both operate in the user's non-0 vmid address space. The MES
> manages the vmids assignments for user queues. The driver provides a
> pointer to the user's GPU VM page tables and MES assigns a vmid when
> it maps the queue. Driver provides a mask which vmids the MES can use
> so that there are no conflicts when mixing kernel and user queues.
>
> Alex
Hi Alex,
Thanks a lot for all the detailed explanations and patience. I tried to
condense all the knowledge that you shared here and in other places in a
patchset available at:
https://lore.kernel.org/amd-gfx/20250325172623.225901-1-siqueira@igalia.com/T/#t
Thanks again!
>
> >
> > Thanks
> >
> > >
> > > >
> > > > I'm unsure if I fully understand this series's idea; please correct me
> > > > if I'm wrong.
> > > >
> > > > Also, please elaborate more on the type of tasks that the kernel queue
> > > > handles. Tbh, I did not fully understand the idea behind it.
> > >
> > > In the future of user queues, kernel queues would not be created or
> > > used at all. Today, on most existing hardware, kernel queues are all
> > > that is available. Today, when an application submits work to the
> > > kernel driver, the kernel driver submits all of the application
> > > command buffers to kernel queues. E.g., in most cases there is a
> > > single kernel GFX queue and all applications which want to use the GFX
> > > engine funnel into that queue. The CS IOCTL basically takes the
> > > command buffers from the applications and schedules them on the kernel
> > > queue. With user queues, each application will create its own user
> > > queues and will submit work directly to its user queues. No need for
> > > an IOCTL for each submission, no need to share a single kernel queue,
> > > etc.
> > >
> > > Alex
> > >
> > > >
> > > > Thanks
> > > >
> > > > >
> > > > > v2: use num_gfx_rings and num_compute_rings per
> > > > > Felix suggestion
> > > > > v3: include num_gfx_rings fix in amdgpu_gfx.c
> > > > > v4: additional fixes
> > > > > v5: MEC EOP interrupt handling fix (Sunil)
> > > > >
> > > > > Alex Deucher (11):
> > > > > drm/amdgpu: add parameter to disable kernel queues
> > > > > drm/amdgpu: add ring flag for no user submissions
> > > > > drm/amdgpu/gfx: add generic handling for disable_kq
> > > > > drm/amdgpu/mes: centralize gfx_hqd mask management
> > > > > drm/amdgpu/mes: update hqd masks when disable_kq is set
> > > > > drm/amdgpu/mes: make more vmids available when disable_kq=1
> > > > > drm/amdgpu/gfx11: add support for disable_kq
> > > > > drm/amdgpu/gfx12: add support for disable_kq
> > > > > drm/amdgpu/sdma: add flag for tracking disable_kq
> > > > > drm/amdgpu/sdma6: add support for disable_kq
> > > > > drm/amdgpu/sdma7: add support for disable_kq
> > > > >
> > > > > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +
> > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 ++
> > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 8 +-
> > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 +
> > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 30 ++--
> > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 26 ++-
> > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
> > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h | 1 +
> > > > > drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 191 ++++++++++++++++-------
> > > > > drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 183 +++++++++++++++-------
> > > > > drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 2 +-
> > > > > drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c | 2 +-
> > > > > drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 16 +-
> > > > > drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 15 +-
> > > > > drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 4 +
> > > > > drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c | 4 +
> > > > > 17 files changed, 345 insertions(+), 155 deletions(-)
> > > > >
> > > > > --
> > > > > 2.48.1
> > > > >
> > > >
> > > > --
> > > > Rodrigo Siqueira
> >
> > --
> > Rodrigo Siqueira
--
Rodrigo Siqueira
^ permalink raw reply [flat|nested] 41+ messages in thread