* [PATCH 1/1] drm/amdgpu: rework context priority handling
@ 2021-08-25 11:22 Nirmoy Das
2021-08-25 11:29 ` Christian König
2021-08-25 12:20 ` Lazar, Lijo
0 siblings, 2 replies; 6+ messages in thread
From: Nirmoy Das @ 2021-08-25 11:22 UTC (permalink / raw)
To: amd-gfx; +Cc: Christian.Koenig, satyajit.sahu, Nirmoy Das
To get a hardware queue priority for a context, we are currently
mapping AMDGPU_CTX_PRIORITY_* to DRM_SCHED_PRIORITY_* and then
to hardware queue priority, which is not the right way to do that
as DRM_SCHED_PRIORITY_* is software scheduler's priority and it is
independent from a hardware queue priority.
Use userspace provided context priority, AMDGPU_CTX_PRIORITY_* to
map a context to proper hardware queue priority.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 127 ++++++++++++++++------
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 8 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 44 ++------
3 files changed, 105 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index e7a010b7ca1f..c88c5c6c54a2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -43,14 +43,61 @@ const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = {
[AMDGPU_HW_IP_VCN_JPEG] = 1,
};
+bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio)
+{
+ switch (ctx_prio) {
+ case AMDGPU_CTX_PRIORITY_UNSET:
+ case AMDGPU_CTX_PRIORITY_VERY_LOW:
+ case AMDGPU_CTX_PRIORITY_LOW:
+ case AMDGPU_CTX_PRIORITY_NORMAL:
+ case AMDGPU_CTX_PRIORITY_HIGH:
+ case AMDGPU_CTX_PRIORITY_VERY_HIGH:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static enum drm_sched_priority
+amdgpu_ctx_to_drm_sched_prio(int32_t ctx_prio)
+{
+ switch (ctx_prio) {
+ case AMDGPU_CTX_PRIORITY_UNSET:
+ return DRM_SCHED_PRIORITY_UNSET;
+
+ case AMDGPU_CTX_PRIORITY_VERY_LOW:
+ return DRM_SCHED_PRIORITY_MIN;
+
+ case AMDGPU_CTX_PRIORITY_LOW:
+ return DRM_SCHED_PRIORITY_MIN;
+
+ case AMDGPU_CTX_PRIORITY_NORMAL:
+ return DRM_SCHED_PRIORITY_NORMAL;
+
+ case AMDGPU_CTX_PRIORITY_HIGH:
+ return DRM_SCHED_PRIORITY_HIGH;
+
+ case AMDGPU_CTX_PRIORITY_VERY_HIGH:
+ return DRM_SCHED_PRIORITY_HIGH;
+
+ /* This should not happen as we sanitized userspace provided priority
+ * already, WARN if this happens.
+ */
+ default:
+ WARN(1, "Invalid context priority %d\n", ctx_prio);
+ return DRM_SCHED_PRIORITY_NORMAL;
+ }
+
+}
+
static int amdgpu_ctx_priority_permit(struct drm_file *filp,
- enum drm_sched_priority priority)
+ int32_t priority)
{
- if (priority < 0 || priority >= DRM_SCHED_PRIORITY_COUNT)
+ if (!amdgpu_ctx_priority_is_valid(priority))
return -EINVAL;
/* NORMAL and below are accessible by everyone */
- if (priority <= DRM_SCHED_PRIORITY_NORMAL)
+ if (priority <= AMDGPU_CTX_PRIORITY_NORMAL)
return 0;
if (capable(CAP_SYS_NICE))
@@ -62,26 +109,35 @@ static int amdgpu_ctx_priority_permit(struct drm_file *filp,
return -EACCES;
}
-static enum gfx_pipe_priority amdgpu_ctx_sched_prio_to_compute_prio(enum drm_sched_priority prio)
+static enum gfx_pipe_priority amdgpu_ctx_prio_to_compute_prio(int32_t prio)
{
switch (prio) {
- case DRM_SCHED_PRIORITY_HIGH:
- case DRM_SCHED_PRIORITY_KERNEL:
+ case AMDGPU_CTX_PRIORITY_HIGH:
+ case AMDGPU_CTX_PRIORITY_VERY_HIGH:
return AMDGPU_GFX_PIPE_PRIO_HIGH;
default:
return AMDGPU_GFX_PIPE_PRIO_NORMAL;
}
}
-static unsigned int amdgpu_ctx_prio_sched_to_hw(struct amdgpu_device *adev,
- enum drm_sched_priority prio,
- u32 hw_ip)
+static unsigned int amdgpu_ctx_get_hw_prio(struct amdgpu_ctx *ctx, u32 hw_ip)
{
+ struct amdgpu_device *adev = ctx->adev;
+ int32_t ctx_prio;
unsigned int hw_prio;
- hw_prio = (hw_ip == AMDGPU_HW_IP_COMPUTE) ?
- amdgpu_ctx_sched_prio_to_compute_prio(prio) :
- AMDGPU_RING_PRIO_DEFAULT;
+ ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
+ ctx->init_priority : ctx->override_priority;
+
+ switch (hw_ip) {
+ case AMDGPU_HW_IP_COMPUTE:
+ hw_prio = amdgpu_ctx_prio_to_compute_prio(ctx_prio);
+ break;
+ default:
+ hw_prio = AMDGPU_RING_PRIO_DEFAULT;
+ break;
+ }
+
hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
if (adev->gpu_sched[hw_ip][hw_prio].num_scheds == 0)
hw_prio = AMDGPU_RING_PRIO_DEFAULT;
@@ -89,15 +145,17 @@ static unsigned int amdgpu_ctx_prio_sched_to_hw(struct amdgpu_device *adev,
return hw_prio;
}
+
static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
- const u32 ring)
+ const u32 ring)
{
struct amdgpu_device *adev = ctx->adev;
struct amdgpu_ctx_entity *entity;
struct drm_gpu_scheduler **scheds = NULL, *sched = NULL;
unsigned num_scheds = 0;
+ int32_t ctx_prio;
unsigned int hw_prio;
- enum drm_sched_priority priority;
+ enum drm_sched_priority drm_prio;
int r;
entity = kzalloc(struct_size(entity, fences, amdgpu_sched_jobs),
@@ -105,10 +163,11 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
if (!entity)
return -ENOMEM;
+ ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
+ ctx->init_priority : ctx->override_priority;
entity->sequence = 1;
- priority = (ctx->override_priority == DRM_SCHED_PRIORITY_UNSET) ?
- ctx->init_priority : ctx->override_priority;
- hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority, hw_ip);
+ hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
+ drm_prio = amdgpu_ctx_to_drm_sched_prio(ctx_prio);
hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
@@ -124,7 +183,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
num_scheds = 1;
}
- r = drm_sched_entity_init(&entity->entity, priority, scheds, num_scheds,
+ r = drm_sched_entity_init(&entity->entity, drm_prio, scheds, num_scheds,
&ctx->guilty);
if (r)
goto error_free_entity;
@@ -139,7 +198,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
}
static int amdgpu_ctx_init(struct amdgpu_device *adev,
- enum drm_sched_priority priority,
+ int32_t priority,
struct drm_file *filp,
struct amdgpu_ctx *ctx)
{
@@ -161,7 +220,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
ctx->reset_counter_query = ctx->reset_counter;
ctx->vram_lost_counter = atomic_read(&adev->vram_lost_counter);
ctx->init_priority = priority;
- ctx->override_priority = DRM_SCHED_PRIORITY_UNSET;
+ ctx->override_priority = AMDGPU_CTX_PRIORITY_UNSET;
return 0;
}
@@ -234,7 +293,7 @@ int amdgpu_ctx_get_entity(struct amdgpu_ctx *ctx, u32 hw_ip, u32 instance,
static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
struct amdgpu_fpriv *fpriv,
struct drm_file *filp,
- enum drm_sched_priority priority,
+ int32_t priority,
uint32_t *id)
{
struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
@@ -397,19 +456,19 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
{
int r;
uint32_t id;
- enum drm_sched_priority priority;
+ int32_t priority;
union drm_amdgpu_ctx *args = data;
struct amdgpu_device *adev = drm_to_adev(dev);
struct amdgpu_fpriv *fpriv = filp->driver_priv;
id = args->in.ctx_id;
- r = amdgpu_to_sched_priority(args->in.priority, &priority);
+ priority = args->in.priority;
/* For backwards compatibility reasons, we need to accept
* ioctls with garbage in the priority field */
- if (r == -EINVAL)
- priority = DRM_SCHED_PRIORITY_NORMAL;
+ if (!amdgpu_ctx_priority_is_valid(priority))
+ priority = AMDGPU_CTX_PRIORITY_NORMAL;
switch (args->in.op) {
case AMDGPU_CTX_OP_ALLOC_CTX:
@@ -515,9 +574,9 @@ struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
}
static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
- struct amdgpu_ctx_entity *aentity,
- int hw_ip,
- enum drm_sched_priority priority)
+ struct amdgpu_ctx_entity *aentity,
+ int hw_ip,
+ int32_t priority)
{
struct amdgpu_device *adev = ctx->adev;
unsigned int hw_prio;
@@ -525,12 +584,12 @@ static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
unsigned num_scheds;
/* set sw priority */
- drm_sched_entity_set_priority(&aentity->entity, priority);
+ drm_sched_entity_set_priority(&aentity->entity,
+ amdgpu_ctx_to_drm_sched_prio(priority));
/* set hw priority */
if (hw_ip == AMDGPU_HW_IP_COMPUTE) {
- hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority,
- AMDGPU_HW_IP_COMPUTE);
+ hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
hw_prio = array_index_nospec(hw_prio, AMDGPU_RING_PRIO_MAX);
scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
num_scheds = adev->gpu_sched[hw_ip][hw_prio].num_scheds;
@@ -540,14 +599,14 @@ static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
}
void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
- enum drm_sched_priority priority)
+ int32_t priority)
{
- enum drm_sched_priority ctx_prio;
+ int32_t ctx_prio;
unsigned i, j;
ctx->override_priority = priority;
- ctx_prio = (ctx->override_priority == DRM_SCHED_PRIORITY_UNSET) ?
+ ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
ctx->init_priority : ctx->override_priority;
for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
index 14db16bc3322..a44b8b8ed39c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
@@ -47,8 +47,8 @@ struct amdgpu_ctx {
spinlock_t ring_lock;
struct amdgpu_ctx_entity *entities[AMDGPU_HW_IP_NUM][AMDGPU_MAX_ENTITY_NUM];
bool preamble_presented;
- enum drm_sched_priority init_priority;
- enum drm_sched_priority override_priority;
+ int32_t init_priority;
+ int32_t override_priority;
struct mutex lock;
atomic_t guilty;
unsigned long ras_counter_ce;
@@ -75,8 +75,8 @@ void amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx,
struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
struct drm_sched_entity *entity,
uint64_t seq);
-void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
- enum drm_sched_priority priority);
+bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio);
+void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx, int32_t ctx_prio);
int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
index b7d861ed5284..e9b45089a28a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
@@ -32,37 +32,9 @@
#include "amdgpu_sched.h"
#include "amdgpu_vm.h"
-int amdgpu_to_sched_priority(int amdgpu_priority,
- enum drm_sched_priority *prio)
-{
- switch (amdgpu_priority) {
- case AMDGPU_CTX_PRIORITY_VERY_HIGH:
- *prio = DRM_SCHED_PRIORITY_HIGH;
- break;
- case AMDGPU_CTX_PRIORITY_HIGH:
- *prio = DRM_SCHED_PRIORITY_HIGH;
- break;
- case AMDGPU_CTX_PRIORITY_NORMAL:
- *prio = DRM_SCHED_PRIORITY_NORMAL;
- break;
- case AMDGPU_CTX_PRIORITY_LOW:
- case AMDGPU_CTX_PRIORITY_VERY_LOW:
- *prio = DRM_SCHED_PRIORITY_MIN;
- break;
- case AMDGPU_CTX_PRIORITY_UNSET:
- *prio = DRM_SCHED_PRIORITY_UNSET;
- break;
- default:
- WARN(1, "Invalid context priority %d\n", amdgpu_priority);
- return -EINVAL;
- }
-
- return 0;
-}
-
static int amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
int fd,
- enum drm_sched_priority priority)
+ int32_t priority)
{
struct fd f = fdget(fd);
struct amdgpu_fpriv *fpriv;
@@ -89,7 +61,7 @@ static int amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
static int amdgpu_sched_context_priority_override(struct amdgpu_device *adev,
int fd,
unsigned ctx_id,
- enum drm_sched_priority priority)
+ int32_t priority)
{
struct fd f = fdget(fd);
struct amdgpu_fpriv *fpriv;
@@ -124,7 +96,6 @@ int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
{
union drm_amdgpu_sched *args = data;
struct amdgpu_device *adev = drm_to_adev(dev);
- enum drm_sched_priority priority;
int r;
/* First check the op, then the op's argument.
@@ -138,21 +109,22 @@ int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
return -EINVAL;
}
- r = amdgpu_to_sched_priority(args->in.priority, &priority);
- if (r)
- return r;
+ if (!amdgpu_ctx_priority_is_valid(args->in.priority)) {
+ WARN(1, "Invalid context priority %d\n", args->in.priority);
+ return -EINVAL;
+ }
switch (args->in.op) {
case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
r = amdgpu_sched_process_priority_override(adev,
args->in.fd,
- priority);
+ args->in.priority);
break;
case AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE:
r = amdgpu_sched_context_priority_override(adev,
args->in.fd,
args->in.ctx_id,
- priority);
+ args->in.priority);
break;
default:
/* Impossible.
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: rework context priority handling
2021-08-25 11:22 [PATCH 1/1] drm/amdgpu: rework context priority handling Nirmoy Das
@ 2021-08-25 11:29 ` Christian König
2021-08-25 12:20 ` Lazar, Lijo
1 sibling, 0 replies; 6+ messages in thread
From: Christian König @ 2021-08-25 11:29 UTC (permalink / raw)
To: Nirmoy Das, amd-gfx; +Cc: satyajit.sahu
Am 25.08.21 um 13:22 schrieb Nirmoy Das:
> To get a hardware queue priority for a context, we are currently
> mapping AMDGPU_CTX_PRIORITY_* to DRM_SCHED_PRIORITY_* and then
> to hardware queue priority, which is not the right way to do that
> as DRM_SCHED_PRIORITY_* is software scheduler's priority and it is
> independent from a hardware queue priority.
>
> Use userspace provided context priority, AMDGPU_CTX_PRIORITY_* to
> map a context to proper hardware queue priority.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 127 ++++++++++++++++------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 8 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 44 ++------
> 3 files changed, 105 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index e7a010b7ca1f..c88c5c6c54a2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -43,14 +43,61 @@ const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = {
> [AMDGPU_HW_IP_VCN_JPEG] = 1,
> };
>
> +bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio)
> +{
> + switch (ctx_prio) {
> + case AMDGPU_CTX_PRIORITY_UNSET:
> + case AMDGPU_CTX_PRIORITY_VERY_LOW:
> + case AMDGPU_CTX_PRIORITY_LOW:
> + case AMDGPU_CTX_PRIORITY_NORMAL:
> + case AMDGPU_CTX_PRIORITY_HIGH:
> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static enum drm_sched_priority
> +amdgpu_ctx_to_drm_sched_prio(int32_t ctx_prio)
> +{
> + switch (ctx_prio) {
> + case AMDGPU_CTX_PRIORITY_UNSET:
> + return DRM_SCHED_PRIORITY_UNSET;
> +
> + case AMDGPU_CTX_PRIORITY_VERY_LOW:
> + return DRM_SCHED_PRIORITY_MIN;
> +
> + case AMDGPU_CTX_PRIORITY_LOW:
> + return DRM_SCHED_PRIORITY_MIN;
> +
> + case AMDGPU_CTX_PRIORITY_NORMAL:
> + return DRM_SCHED_PRIORITY_NORMAL;
> +
> + case AMDGPU_CTX_PRIORITY_HIGH:
> + return DRM_SCHED_PRIORITY_HIGH;
> +
> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
> + return DRM_SCHED_PRIORITY_HIGH;
> +
> + /* This should not happen as we sanitized userspace provided priority
> + * already, WARN if this happens.
> + */
> + default:
> + WARN(1, "Invalid context priority %d\n", ctx_prio);
> + return DRM_SCHED_PRIORITY_NORMAL;
> + }
> +
> +}
> +
> static int amdgpu_ctx_priority_permit(struct drm_file *filp,
> - enum drm_sched_priority priority)
> + int32_t priority)
> {
> - if (priority < 0 || priority >= DRM_SCHED_PRIORITY_COUNT)
> + if (!amdgpu_ctx_priority_is_valid(priority))
> return -EINVAL;
>
> /* NORMAL and below are accessible by everyone */
> - if (priority <= DRM_SCHED_PRIORITY_NORMAL)
> + if (priority <= AMDGPU_CTX_PRIORITY_NORMAL)
> return 0;
>
> if (capable(CAP_SYS_NICE))
> @@ -62,26 +109,35 @@ static int amdgpu_ctx_priority_permit(struct drm_file *filp,
> return -EACCES;
> }
>
> -static enum gfx_pipe_priority amdgpu_ctx_sched_prio_to_compute_prio(enum drm_sched_priority prio)
> +static enum gfx_pipe_priority amdgpu_ctx_prio_to_compute_prio(int32_t prio)
> {
> switch (prio) {
> - case DRM_SCHED_PRIORITY_HIGH:
> - case DRM_SCHED_PRIORITY_KERNEL:
> + case AMDGPU_CTX_PRIORITY_HIGH:
> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
> return AMDGPU_GFX_PIPE_PRIO_HIGH;
> default:
> return AMDGPU_GFX_PIPE_PRIO_NORMAL;
> }
> }
>
> -static unsigned int amdgpu_ctx_prio_sched_to_hw(struct amdgpu_device *adev,
> - enum drm_sched_priority prio,
> - u32 hw_ip)
> +static unsigned int amdgpu_ctx_get_hw_prio(struct amdgpu_ctx *ctx, u32 hw_ip)
> {
> + struct amdgpu_device *adev = ctx->adev;
> + int32_t ctx_prio;
> unsigned int hw_prio;
>
> - hw_prio = (hw_ip == AMDGPU_HW_IP_COMPUTE) ?
> - amdgpu_ctx_sched_prio_to_compute_prio(prio) :
> - AMDGPU_RING_PRIO_DEFAULT;
> + ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
> + ctx->init_priority : ctx->override_priority;
> +
> + switch (hw_ip) {
> + case AMDGPU_HW_IP_COMPUTE:
> + hw_prio = amdgpu_ctx_prio_to_compute_prio(ctx_prio);
> + break;
> + default:
> + hw_prio = AMDGPU_RING_PRIO_DEFAULT;
> + break;
> + }
> +
> hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
> if (adev->gpu_sched[hw_ip][hw_prio].num_scheds == 0)
> hw_prio = AMDGPU_RING_PRIO_DEFAULT;
> @@ -89,15 +145,17 @@ static unsigned int amdgpu_ctx_prio_sched_to_hw(struct amdgpu_device *adev,
> return hw_prio;
> }
>
> +
> static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
> - const u32 ring)
> + const u32 ring)
> {
> struct amdgpu_device *adev = ctx->adev;
> struct amdgpu_ctx_entity *entity;
> struct drm_gpu_scheduler **scheds = NULL, *sched = NULL;
> unsigned num_scheds = 0;
> + int32_t ctx_prio;
> unsigned int hw_prio;
> - enum drm_sched_priority priority;
> + enum drm_sched_priority drm_prio;
> int r;
>
> entity = kzalloc(struct_size(entity, fences, amdgpu_sched_jobs),
> @@ -105,10 +163,11 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
> if (!entity)
> return -ENOMEM;
>
> + ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
> + ctx->init_priority : ctx->override_priority;
> entity->sequence = 1;
> - priority = (ctx->override_priority == DRM_SCHED_PRIORITY_UNSET) ?
> - ctx->init_priority : ctx->override_priority;
> - hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority, hw_ip);
> + hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
> + drm_prio = amdgpu_ctx_to_drm_sched_prio(ctx_prio);
>
> hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
> scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
> @@ -124,7 +183,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
> num_scheds = 1;
> }
>
> - r = drm_sched_entity_init(&entity->entity, priority, scheds, num_scheds,
> + r = drm_sched_entity_init(&entity->entity, drm_prio, scheds, num_scheds,
> &ctx->guilty);
> if (r)
> goto error_free_entity;
> @@ -139,7 +198,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
> }
>
> static int amdgpu_ctx_init(struct amdgpu_device *adev,
> - enum drm_sched_priority priority,
> + int32_t priority,
> struct drm_file *filp,
> struct amdgpu_ctx *ctx)
> {
> @@ -161,7 +220,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
> ctx->reset_counter_query = ctx->reset_counter;
> ctx->vram_lost_counter = atomic_read(&adev->vram_lost_counter);
> ctx->init_priority = priority;
> - ctx->override_priority = DRM_SCHED_PRIORITY_UNSET;
> + ctx->override_priority = AMDGPU_CTX_PRIORITY_UNSET;
>
> return 0;
> }
> @@ -234,7 +293,7 @@ int amdgpu_ctx_get_entity(struct amdgpu_ctx *ctx, u32 hw_ip, u32 instance,
> static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
> struct amdgpu_fpriv *fpriv,
> struct drm_file *filp,
> - enum drm_sched_priority priority,
> + int32_t priority,
> uint32_t *id)
> {
> struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
> @@ -397,19 +456,19 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
> {
> int r;
> uint32_t id;
> - enum drm_sched_priority priority;
> + int32_t priority;
>
> union drm_amdgpu_ctx *args = data;
> struct amdgpu_device *adev = drm_to_adev(dev);
> struct amdgpu_fpriv *fpriv = filp->driver_priv;
>
> id = args->in.ctx_id;
> - r = amdgpu_to_sched_priority(args->in.priority, &priority);
> + priority = args->in.priority;
>
> /* For backwards compatibility reasons, we need to accept
> * ioctls with garbage in the priority field */
> - if (r == -EINVAL)
> - priority = DRM_SCHED_PRIORITY_NORMAL;
> + if (!amdgpu_ctx_priority_is_valid(priority))
> + priority = AMDGPU_CTX_PRIORITY_NORMAL;
>
> switch (args->in.op) {
> case AMDGPU_CTX_OP_ALLOC_CTX:
> @@ -515,9 +574,9 @@ struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
> }
>
> static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
> - struct amdgpu_ctx_entity *aentity,
> - int hw_ip,
> - enum drm_sched_priority priority)
> + struct amdgpu_ctx_entity *aentity,
> + int hw_ip,
> + int32_t priority)
> {
> struct amdgpu_device *adev = ctx->adev;
> unsigned int hw_prio;
> @@ -525,12 +584,12 @@ static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
> unsigned num_scheds;
>
> /* set sw priority */
> - drm_sched_entity_set_priority(&aentity->entity, priority);
> + drm_sched_entity_set_priority(&aentity->entity,
> + amdgpu_ctx_to_drm_sched_prio(priority));
>
> /* set hw priority */
> if (hw_ip == AMDGPU_HW_IP_COMPUTE) {
> - hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority,
> - AMDGPU_HW_IP_COMPUTE);
> + hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
> hw_prio = array_index_nospec(hw_prio, AMDGPU_RING_PRIO_MAX);
> scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
> num_scheds = adev->gpu_sched[hw_ip][hw_prio].num_scheds;
> @@ -540,14 +599,14 @@ static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
> }
>
> void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
> - enum drm_sched_priority priority)
> + int32_t priority)
> {
> - enum drm_sched_priority ctx_prio;
> + int32_t ctx_prio;
> unsigned i, j;
>
> ctx->override_priority = priority;
>
> - ctx_prio = (ctx->override_priority == DRM_SCHED_PRIORITY_UNSET) ?
> + ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
> ctx->init_priority : ctx->override_priority;
> for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
> for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> index 14db16bc3322..a44b8b8ed39c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> @@ -47,8 +47,8 @@ struct amdgpu_ctx {
> spinlock_t ring_lock;
> struct amdgpu_ctx_entity *entities[AMDGPU_HW_IP_NUM][AMDGPU_MAX_ENTITY_NUM];
> bool preamble_presented;
> - enum drm_sched_priority init_priority;
> - enum drm_sched_priority override_priority;
> + int32_t init_priority;
> + int32_t override_priority;
> struct mutex lock;
> atomic_t guilty;
> unsigned long ras_counter_ce;
> @@ -75,8 +75,8 @@ void amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx,
> struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
> struct drm_sched_entity *entity,
> uint64_t seq);
> -void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
> - enum drm_sched_priority priority);
> +bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio);
> +void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx, int32_t ctx_prio);
>
> int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> index b7d861ed5284..e9b45089a28a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> @@ -32,37 +32,9 @@
> #include "amdgpu_sched.h"
> #include "amdgpu_vm.h"
>
> -int amdgpu_to_sched_priority(int amdgpu_priority,
> - enum drm_sched_priority *prio)
> -{
> - switch (amdgpu_priority) {
> - case AMDGPU_CTX_PRIORITY_VERY_HIGH:
> - *prio = DRM_SCHED_PRIORITY_HIGH;
> - break;
> - case AMDGPU_CTX_PRIORITY_HIGH:
> - *prio = DRM_SCHED_PRIORITY_HIGH;
> - break;
> - case AMDGPU_CTX_PRIORITY_NORMAL:
> - *prio = DRM_SCHED_PRIORITY_NORMAL;
> - break;
> - case AMDGPU_CTX_PRIORITY_LOW:
> - case AMDGPU_CTX_PRIORITY_VERY_LOW:
> - *prio = DRM_SCHED_PRIORITY_MIN;
> - break;
> - case AMDGPU_CTX_PRIORITY_UNSET:
> - *prio = DRM_SCHED_PRIORITY_UNSET;
> - break;
> - default:
> - WARN(1, "Invalid context priority %d\n", amdgpu_priority);
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -
> static int amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
> int fd,
> - enum drm_sched_priority priority)
> + int32_t priority)
> {
> struct fd f = fdget(fd);
> struct amdgpu_fpriv *fpriv;
> @@ -89,7 +61,7 @@ static int amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
> static int amdgpu_sched_context_priority_override(struct amdgpu_device *adev,
> int fd,
> unsigned ctx_id,
> - enum drm_sched_priority priority)
> + int32_t priority)
> {
> struct fd f = fdget(fd);
> struct amdgpu_fpriv *fpriv;
> @@ -124,7 +96,6 @@ int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
> {
> union drm_amdgpu_sched *args = data;
> struct amdgpu_device *adev = drm_to_adev(dev);
> - enum drm_sched_priority priority;
> int r;
>
> /* First check the op, then the op's argument.
> @@ -138,21 +109,22 @@ int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
> return -EINVAL;
> }
>
> - r = amdgpu_to_sched_priority(args->in.priority, &priority);
> - if (r)
> - return r;
> + if (!amdgpu_ctx_priority_is_valid(args->in.priority)) {
> + WARN(1, "Invalid context priority %d\n", args->in.priority);
> + return -EINVAL;
> + }
>
> switch (args->in.op) {
> case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
> r = amdgpu_sched_process_priority_override(adev,
> args->in.fd,
> - priority);
> + args->in.priority);
> break;
> case AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE:
> r = amdgpu_sched_context_priority_override(adev,
> args->in.fd,
> args->in.ctx_id,
> - priority);
> + args->in.priority);
> break;
> default:
> /* Impossible.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: rework context priority handling
2021-08-25 11:22 [PATCH 1/1] drm/amdgpu: rework context priority handling Nirmoy Das
2021-08-25 11:29 ` Christian König
@ 2021-08-25 12:20 ` Lazar, Lijo
2021-08-25 12:29 ` Christian König
1 sibling, 1 reply; 6+ messages in thread
From: Lazar, Lijo @ 2021-08-25 12:20 UTC (permalink / raw)
To: Nirmoy Das, amd-gfx; +Cc: Christian.Koenig, satyajit.sahu
On 8/25/2021 4:52 PM, Nirmoy Das wrote:
> To get a hardware queue priority for a context, we are currently
> mapping AMDGPU_CTX_PRIORITY_* to DRM_SCHED_PRIORITY_* and then
> to hardware queue priority, which is not the right way to do that
> as DRM_SCHED_PRIORITY_* is software scheduler's priority and it is
> independent from a hardware queue priority.
>
> Use userspace provided context priority, AMDGPU_CTX_PRIORITY_* to
> map a context to proper hardware queue priority.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 127 ++++++++++++++++------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 8 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 44 ++------
> 3 files changed, 105 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index e7a010b7ca1f..c88c5c6c54a2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -43,14 +43,61 @@ const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = {
> [AMDGPU_HW_IP_VCN_JPEG] = 1,
> };
>
> +bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio)
> +{
> + switch (ctx_prio) {
> + case AMDGPU_CTX_PRIORITY_UNSET:
> + case AMDGPU_CTX_PRIORITY_VERY_LOW:
> + case AMDGPU_CTX_PRIORITY_LOW:
> + case AMDGPU_CTX_PRIORITY_NORMAL:
> + case AMDGPU_CTX_PRIORITY_HIGH:
> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static enum drm_sched_priority
> +amdgpu_ctx_to_drm_sched_prio(int32_t ctx_prio)
> +{
> + switch (ctx_prio) {
> + case AMDGPU_CTX_PRIORITY_UNSET:
> + return DRM_SCHED_PRIORITY_UNSET;
> +
> + case AMDGPU_CTX_PRIORITY_VERY_LOW:
> + return DRM_SCHED_PRIORITY_MIN;
> +
> + case AMDGPU_CTX_PRIORITY_LOW:
> + return DRM_SCHED_PRIORITY_MIN;
> +
> + case AMDGPU_CTX_PRIORITY_NORMAL:
> + return DRM_SCHED_PRIORITY_NORMAL;
> +
> + case AMDGPU_CTX_PRIORITY_HIGH:
> + return DRM_SCHED_PRIORITY_HIGH;
> +
> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
> + return DRM_SCHED_PRIORITY_HIGH;
> +
> + /* This should not happen as we sanitized userspace provided priority
> + * already, WARN if this happens.
> + */
> + default:
> + WARN(1, "Invalid context priority %d\n", ctx_prio);
> + return DRM_SCHED_PRIORITY_NORMAL;
> + }
> +
> +}
> +
> static int amdgpu_ctx_priority_permit(struct drm_file *filp,
> - enum drm_sched_priority priority)
> + int32_t priority)
> {
> - if (priority < 0 || priority >= DRM_SCHED_PRIORITY_COUNT)
> + if (!amdgpu_ctx_priority_is_valid(priority))
> return -EINVAL;
>
> /* NORMAL and below are accessible by everyone */
> - if (priority <= DRM_SCHED_PRIORITY_NORMAL)
> + if (priority <= AMDGPU_CTX_PRIORITY_NORMAL)
> return 0;
>
> if (capable(CAP_SYS_NICE))
> @@ -62,26 +109,35 @@ static int amdgpu_ctx_priority_permit(struct drm_file *filp,
> return -EACCES;
> }
>
> -static enum gfx_pipe_priority amdgpu_ctx_sched_prio_to_compute_prio(enum drm_sched_priority prio)
> +static enum gfx_pipe_priority amdgpu_ctx_prio_to_compute_prio(int32_t prio)
> {
> switch (prio) {
> - case DRM_SCHED_PRIORITY_HIGH:
> - case DRM_SCHED_PRIORITY_KERNEL:
> + case AMDGPU_CTX_PRIORITY_HIGH:
> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
> return AMDGPU_GFX_PIPE_PRIO_HIGH;
> default:
> return AMDGPU_GFX_PIPE_PRIO_NORMAL;
> }
> }
>
> -static unsigned int amdgpu_ctx_prio_sched_to_hw(struct amdgpu_device *adev,
> - enum drm_sched_priority prio,
> - u32 hw_ip)
> +static unsigned int amdgpu_ctx_get_hw_prio(struct amdgpu_ctx *ctx, u32 hw_ip)
> {
> + struct amdgpu_device *adev = ctx->adev;
> + int32_t ctx_prio;
> unsigned int hw_prio;
>
> - hw_prio = (hw_ip == AMDGPU_HW_IP_COMPUTE) ?
> - amdgpu_ctx_sched_prio_to_compute_prio(prio) :
> - AMDGPU_RING_PRIO_DEFAULT;
> + ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
> + ctx->init_priority : ctx->override_priority;
> +
> + switch (hw_ip) {
> + case AMDGPU_HW_IP_COMPUTE:
> + hw_prio = amdgpu_ctx_prio_to_compute_prio(ctx_prio);
> + break;
> + default:
> + hw_prio = AMDGPU_RING_PRIO_DEFAULT;
> + break;
> + }
> +
> hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
> if (adev->gpu_sched[hw_ip][hw_prio].num_scheds == 0)
> hw_prio = AMDGPU_RING_PRIO_DEFAULT;
> @@ -89,15 +145,17 @@ static unsigned int amdgpu_ctx_prio_sched_to_hw(struct amdgpu_device *adev,
> return hw_prio;
> }
>
> +
> static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
> - const u32 ring)
> + const u32 ring)
> {
> struct amdgpu_device *adev = ctx->adev;
> struct amdgpu_ctx_entity *entity;
> struct drm_gpu_scheduler **scheds = NULL, *sched = NULL;
> unsigned num_scheds = 0;
> + int32_t ctx_prio;
> unsigned int hw_prio;
> - enum drm_sched_priority priority;
> + enum drm_sched_priority drm_prio;
> int r;
>
> entity = kzalloc(struct_size(entity, fences, amdgpu_sched_jobs),
> @@ -105,10 +163,11 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
> if (!entity)
> return -ENOMEM;
>
> + ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
> + ctx->init_priority : ctx->override_priority;
> entity->sequence = 1;
> - priority = (ctx->override_priority == DRM_SCHED_PRIORITY_UNSET) ?
> - ctx->init_priority : ctx->override_priority;
> - hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority, hw_ip);
> + hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
> + drm_prio = amdgpu_ctx_to_drm_sched_prio(ctx_prio);
>
> hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
> scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
> @@ -124,7 +183,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
> num_scheds = 1;
> }
>
> - r = drm_sched_entity_init(&entity->entity, priority, scheds, num_scheds,
> + r = drm_sched_entity_init(&entity->entity, drm_prio, scheds, num_scheds,
> &ctx->guilty);
> if (r)
> goto error_free_entity;
> @@ -139,7 +198,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
> }
>
> static int amdgpu_ctx_init(struct amdgpu_device *adev,
> - enum drm_sched_priority priority,
> + int32_t priority,
> struct drm_file *filp,
> struct amdgpu_ctx *ctx)
> {
> @@ -161,7 +220,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
> ctx->reset_counter_query = ctx->reset_counter;
> ctx->vram_lost_counter = atomic_read(&adev->vram_lost_counter);
> ctx->init_priority = priority;
> - ctx->override_priority = DRM_SCHED_PRIORITY_UNSET;
> + ctx->override_priority = AMDGPU_CTX_PRIORITY_UNSET;
>
> return 0;
> }
> @@ -234,7 +293,7 @@ int amdgpu_ctx_get_entity(struct amdgpu_ctx *ctx, u32 hw_ip, u32 instance,
> static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
> struct amdgpu_fpriv *fpriv,
> struct drm_file *filp,
> - enum drm_sched_priority priority,
> + int32_t priority,
> uint32_t *id)
> {
> struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
> @@ -397,19 +456,19 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
> {
> int r;
> uint32_t id;
> - enum drm_sched_priority priority;
> + int32_t priority;
>
> union drm_amdgpu_ctx *args = data;
> struct amdgpu_device *adev = drm_to_adev(dev);
> struct amdgpu_fpriv *fpriv = filp->driver_priv;
>
> id = args->in.ctx_id;
> - r = amdgpu_to_sched_priority(args->in.priority, &priority);
> + priority = args->in.priority;
>
> /* For backwards compatibility reasons, we need to accept
> * ioctls with garbage in the priority field */
> - if (r == -EINVAL)
> - priority = DRM_SCHED_PRIORITY_NORMAL;
> + if (!amdgpu_ctx_priority_is_valid(priority))
> + priority = AMDGPU_CTX_PRIORITY_NORMAL;
>
> switch (args->in.op) {
> case AMDGPU_CTX_OP_ALLOC_CTX:
> @@ -515,9 +574,9 @@ struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
> }
>
> static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
> - struct amdgpu_ctx_entity *aentity,
> - int hw_ip,
> - enum drm_sched_priority priority)
> + struct amdgpu_ctx_entity *aentity,
> + int hw_ip,
> + int32_t priority)
> {
> struct amdgpu_device *adev = ctx->adev;
> unsigned int hw_prio;
> @@ -525,12 +584,12 @@ static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
> unsigned num_scheds;
>
> /* set sw priority */
> - drm_sched_entity_set_priority(&aentity->entity, priority);
> + drm_sched_entity_set_priority(&aentity->entity,
> + amdgpu_ctx_to_drm_sched_prio(priority));
>
> /* set hw priority */
> if (hw_ip == AMDGPU_HW_IP_COMPUTE) {
> - hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority,
> - AMDGPU_HW_IP_COMPUTE);
> + hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
> hw_prio = array_index_nospec(hw_prio, AMDGPU_RING_PRIO_MAX);
Not related to this patch, but this kind of logic may break some day.
There is a HWIP specific priority and there is another RING_PRIO which
is unmapped to HWIP priority Ex: when a new priority is added for VCN
which is higher than any of the existing priorities.
Thanks,
Lijo
> scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
> num_scheds = adev->gpu_sched[hw_ip][hw_prio].num_scheds;
> @@ -540,14 +599,14 @@ static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
> }
>
> void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
> - enum drm_sched_priority priority)
> + int32_t priority)
> {
> - enum drm_sched_priority ctx_prio;
> + int32_t ctx_prio;
> unsigned i, j;
>
> ctx->override_priority = priority;
>
> - ctx_prio = (ctx->override_priority == DRM_SCHED_PRIORITY_UNSET) ?
> + ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
> ctx->init_priority : ctx->override_priority;
> for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
> for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> index 14db16bc3322..a44b8b8ed39c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> @@ -47,8 +47,8 @@ struct amdgpu_ctx {
> spinlock_t ring_lock;
> struct amdgpu_ctx_entity *entities[AMDGPU_HW_IP_NUM][AMDGPU_MAX_ENTITY_NUM];
> bool preamble_presented;
> - enum drm_sched_priority init_priority;
> - enum drm_sched_priority override_priority;
> + int32_t init_priority;
> + int32_t override_priority;
> struct mutex lock;
> atomic_t guilty;
> unsigned long ras_counter_ce;
> @@ -75,8 +75,8 @@ void amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx,
> struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
> struct drm_sched_entity *entity,
> uint64_t seq);
> -void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
> - enum drm_sched_priority priority);
> +bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio);
> +void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx, int32_t ctx_prio);
>
> int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> index b7d861ed5284..e9b45089a28a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> @@ -32,37 +32,9 @@
> #include "amdgpu_sched.h"
> #include "amdgpu_vm.h"
>
> -int amdgpu_to_sched_priority(int amdgpu_priority,
> - enum drm_sched_priority *prio)
> -{
> - switch (amdgpu_priority) {
> - case AMDGPU_CTX_PRIORITY_VERY_HIGH:
> - *prio = DRM_SCHED_PRIORITY_HIGH;
> - break;
> - case AMDGPU_CTX_PRIORITY_HIGH:
> - *prio = DRM_SCHED_PRIORITY_HIGH;
> - break;
> - case AMDGPU_CTX_PRIORITY_NORMAL:
> - *prio = DRM_SCHED_PRIORITY_NORMAL;
> - break;
> - case AMDGPU_CTX_PRIORITY_LOW:
> - case AMDGPU_CTX_PRIORITY_VERY_LOW:
> - *prio = DRM_SCHED_PRIORITY_MIN;
> - break;
> - case AMDGPU_CTX_PRIORITY_UNSET:
> - *prio = DRM_SCHED_PRIORITY_UNSET;
> - break;
> - default:
> - WARN(1, "Invalid context priority %d\n", amdgpu_priority);
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -
> static int amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
> int fd,
> - enum drm_sched_priority priority)
> + int32_t priority)
> {
> struct fd f = fdget(fd);
> struct amdgpu_fpriv *fpriv;
> @@ -89,7 +61,7 @@ static int amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
> static int amdgpu_sched_context_priority_override(struct amdgpu_device *adev,
> int fd,
> unsigned ctx_id,
> - enum drm_sched_priority priority)
> + int32_t priority)
> {
> struct fd f = fdget(fd);
> struct amdgpu_fpriv *fpriv;
> @@ -124,7 +96,6 @@ int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
> {
> union drm_amdgpu_sched *args = data;
> struct amdgpu_device *adev = drm_to_adev(dev);
> - enum drm_sched_priority priority;
> int r;
>
> /* First check the op, then the op's argument.
> @@ -138,21 +109,22 @@ int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
> return -EINVAL;
> }
>
> - r = amdgpu_to_sched_priority(args->in.priority, &priority);
> - if (r)
> - return r;
> + if (!amdgpu_ctx_priority_is_valid(args->in.priority)) {
> + WARN(1, "Invalid context priority %d\n", args->in.priority);
> + return -EINVAL;
> + }
>
> switch (args->in.op) {
> case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
> r = amdgpu_sched_process_priority_override(adev,
> args->in.fd,
> - priority);
> + args->in.priority);
> break;
> case AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE:
> r = amdgpu_sched_context_priority_override(adev,
> args->in.fd,
> args->in.ctx_id,
> - priority);
> + args->in.priority);
> break;
> default:
> /* Impossible.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: rework context priority handling
2021-08-25 12:20 ` Lazar, Lijo
@ 2021-08-25 12:29 ` Christian König
2021-08-25 13:35 ` Das, Nirmoy
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2021-08-25 12:29 UTC (permalink / raw)
To: Lazar, Lijo, Nirmoy Das, amd-gfx; +Cc: satyajit.sahu
Am 25.08.21 um 14:20 schrieb Lazar, Lijo:
> On 8/25/2021 4:52 PM, Nirmoy Das wrote:
>> To get a hardware queue priority for a context, we are currently
>> mapping AMDGPU_CTX_PRIORITY_* to DRM_SCHED_PRIORITY_* and then
>> to hardware queue priority, which is not the right way to do that
>> as DRM_SCHED_PRIORITY_* is software scheduler's priority and it is
>> independent from a hardware queue priority.
>>
>> Use userspace provided context priority, AMDGPU_CTX_PRIORITY_* to
>> map a context to proper hardware queue priority.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 127 ++++++++++++++++------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 8 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 44 ++------
>> 3 files changed, 105 insertions(+), 74 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> index e7a010b7ca1f..c88c5c6c54a2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> @@ -43,14 +43,61 @@ const unsigned int
>> amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = {
>> [AMDGPU_HW_IP_VCN_JPEG] = 1,
>> };
>> +bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio)
>> +{
>> + switch (ctx_prio) {
>> + case AMDGPU_CTX_PRIORITY_UNSET:
>> + case AMDGPU_CTX_PRIORITY_VERY_LOW:
>> + case AMDGPU_CTX_PRIORITY_LOW:
>> + case AMDGPU_CTX_PRIORITY_NORMAL:
>> + case AMDGPU_CTX_PRIORITY_HIGH:
>> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>> + return true;
>> + default:
>> + return false;
>> + }
>> +}
>> +
>> +static enum drm_sched_priority
>> +amdgpu_ctx_to_drm_sched_prio(int32_t ctx_prio)
>> +{
>> + switch (ctx_prio) {
>> + case AMDGPU_CTX_PRIORITY_UNSET:
>> + return DRM_SCHED_PRIORITY_UNSET;
>> +
>> + case AMDGPU_CTX_PRIORITY_VERY_LOW:
>> + return DRM_SCHED_PRIORITY_MIN;
>> +
>> + case AMDGPU_CTX_PRIORITY_LOW:
>> + return DRM_SCHED_PRIORITY_MIN;
>> +
>> + case AMDGPU_CTX_PRIORITY_NORMAL:
>> + return DRM_SCHED_PRIORITY_NORMAL;
>> +
>> + case AMDGPU_CTX_PRIORITY_HIGH:
>> + return DRM_SCHED_PRIORITY_HIGH;
>> +
>> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>> + return DRM_SCHED_PRIORITY_HIGH;
>> +
>> + /* This should not happen as we sanitized userspace provided
>> priority
>> + * already, WARN if this happens.
>> + */
>> + default:
>> + WARN(1, "Invalid context priority %d\n", ctx_prio);
>> + return DRM_SCHED_PRIORITY_NORMAL;
>> + }
>> +
>> +}
>> +
>> static int amdgpu_ctx_priority_permit(struct drm_file *filp,
>> - enum drm_sched_priority priority)
>> + int32_t priority)
>> {
>> - if (priority < 0 || priority >= DRM_SCHED_PRIORITY_COUNT)
>> + if (!amdgpu_ctx_priority_is_valid(priority))
>> return -EINVAL;
>> /* NORMAL and below are accessible by everyone */
>> - if (priority <= DRM_SCHED_PRIORITY_NORMAL)
>> + if (priority <= AMDGPU_CTX_PRIORITY_NORMAL)
>> return 0;
>> if (capable(CAP_SYS_NICE))
>> @@ -62,26 +109,35 @@ static int amdgpu_ctx_priority_permit(struct
>> drm_file *filp,
>> return -EACCES;
>> }
>> -static enum gfx_pipe_priority
>> amdgpu_ctx_sched_prio_to_compute_prio(enum drm_sched_priority prio)
>> +static enum gfx_pipe_priority
>> amdgpu_ctx_prio_to_compute_prio(int32_t prio)
>> {
>> switch (prio) {
>> - case DRM_SCHED_PRIORITY_HIGH:
>> - case DRM_SCHED_PRIORITY_KERNEL:
>> + case AMDGPU_CTX_PRIORITY_HIGH:
>> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>> return AMDGPU_GFX_PIPE_PRIO_HIGH;
>> default:
>> return AMDGPU_GFX_PIPE_PRIO_NORMAL;
>> }
>> }
>> -static unsigned int amdgpu_ctx_prio_sched_to_hw(struct
>> amdgpu_device *adev,
>> - enum drm_sched_priority prio,
>> - u32 hw_ip)
>> +static unsigned int amdgpu_ctx_get_hw_prio(struct amdgpu_ctx *ctx,
>> u32 hw_ip)
>> {
>> + struct amdgpu_device *adev = ctx->adev;
>> + int32_t ctx_prio;
>> unsigned int hw_prio;
>> - hw_prio = (hw_ip == AMDGPU_HW_IP_COMPUTE) ?
>> - amdgpu_ctx_sched_prio_to_compute_prio(prio) :
>> - AMDGPU_RING_PRIO_DEFAULT;
>> + ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
>> + ctx->init_priority : ctx->override_priority;
>> +
>> + switch (hw_ip) {
>> + case AMDGPU_HW_IP_COMPUTE:
>> + hw_prio = amdgpu_ctx_prio_to_compute_prio(ctx_prio);
>> + break;
>> + default:
>> + hw_prio = AMDGPU_RING_PRIO_DEFAULT;
>> + break;
>> + }
>> +
>> hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
>> if (adev->gpu_sched[hw_ip][hw_prio].num_scheds == 0)
>> hw_prio = AMDGPU_RING_PRIO_DEFAULT;
>> @@ -89,15 +145,17 @@ static unsigned int
>> amdgpu_ctx_prio_sched_to_hw(struct amdgpu_device *adev,
>> return hw_prio;
>> }
>> +
>> static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
>> - const u32 ring)
>> + const u32 ring)
>> {
>> struct amdgpu_device *adev = ctx->adev;
>> struct amdgpu_ctx_entity *entity;
>> struct drm_gpu_scheduler **scheds = NULL, *sched = NULL;
>> unsigned num_scheds = 0;
>> + int32_t ctx_prio;
>> unsigned int hw_prio;
>> - enum drm_sched_priority priority;
>> + enum drm_sched_priority drm_prio;
>> int r;
>> entity = kzalloc(struct_size(entity, fences, amdgpu_sched_jobs),
>> @@ -105,10 +163,11 @@ static int amdgpu_ctx_init_entity(struct
>> amdgpu_ctx *ctx, u32 hw_ip,
>> if (!entity)
>> return -ENOMEM;
>> + ctx_prio = (ctx->override_priority ==
>> AMDGPU_CTX_PRIORITY_UNSET) ?
>> + ctx->init_priority : ctx->override_priority;
>> entity->sequence = 1;
>> - priority = (ctx->override_priority == DRM_SCHED_PRIORITY_UNSET) ?
>> - ctx->init_priority : ctx->override_priority;
>> - hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority, hw_ip);
>> + hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
>> + drm_prio = amdgpu_ctx_to_drm_sched_prio(ctx_prio);
>> hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
>> scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
>> @@ -124,7 +183,7 @@ static int amdgpu_ctx_init_entity(struct
>> amdgpu_ctx *ctx, u32 hw_ip,
>> num_scheds = 1;
>> }
>> - r = drm_sched_entity_init(&entity->entity, priority, scheds,
>> num_scheds,
>> + r = drm_sched_entity_init(&entity->entity, drm_prio, scheds,
>> num_scheds,
>> &ctx->guilty);
>> if (r)
>> goto error_free_entity;
>> @@ -139,7 +198,7 @@ static int amdgpu_ctx_init_entity(struct
>> amdgpu_ctx *ctx, u32 hw_ip,
>> }
>> static int amdgpu_ctx_init(struct amdgpu_device *adev,
>> - enum drm_sched_priority priority,
>> + int32_t priority,
>> struct drm_file *filp,
>> struct amdgpu_ctx *ctx)
>> {
>> @@ -161,7 +220,7 @@ static int amdgpu_ctx_init(struct amdgpu_device
>> *adev,
>> ctx->reset_counter_query = ctx->reset_counter;
>> ctx->vram_lost_counter = atomic_read(&adev->vram_lost_counter);
>> ctx->init_priority = priority;
>> - ctx->override_priority = DRM_SCHED_PRIORITY_UNSET;
>> + ctx->override_priority = AMDGPU_CTX_PRIORITY_UNSET;
>> return 0;
>> }
>> @@ -234,7 +293,7 @@ int amdgpu_ctx_get_entity(struct amdgpu_ctx *ctx,
>> u32 hw_ip, u32 instance,
>> static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
>> struct amdgpu_fpriv *fpriv,
>> struct drm_file *filp,
>> - enum drm_sched_priority priority,
>> + int32_t priority,
>> uint32_t *id)
>> {
>> struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
>> @@ -397,19 +456,19 @@ int amdgpu_ctx_ioctl(struct drm_device *dev,
>> void *data,
>> {
>> int r;
>> uint32_t id;
>> - enum drm_sched_priority priority;
>> + int32_t priority;
>> union drm_amdgpu_ctx *args = data;
>> struct amdgpu_device *adev = drm_to_adev(dev);
>> struct amdgpu_fpriv *fpriv = filp->driver_priv;
>> id = args->in.ctx_id;
>> - r = amdgpu_to_sched_priority(args->in.priority, &priority);
>> + priority = args->in.priority;
>> /* For backwards compatibility reasons, we need to accept
>> * ioctls with garbage in the priority field */
>> - if (r == -EINVAL)
>> - priority = DRM_SCHED_PRIORITY_NORMAL;
>> + if (!amdgpu_ctx_priority_is_valid(priority))
>> + priority = AMDGPU_CTX_PRIORITY_NORMAL;
>> switch (args->in.op) {
>> case AMDGPU_CTX_OP_ALLOC_CTX:
>> @@ -515,9 +574,9 @@ struct dma_fence *amdgpu_ctx_get_fence(struct
>> amdgpu_ctx *ctx,
>> }
>> static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
>> - struct amdgpu_ctx_entity *aentity,
>> - int hw_ip,
>> - enum drm_sched_priority priority)
>> + struct amdgpu_ctx_entity *aentity,
>> + int hw_ip,
>> + int32_t priority)
>> {
>> struct amdgpu_device *adev = ctx->adev;
>> unsigned int hw_prio;
>> @@ -525,12 +584,12 @@ static void
>> amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
>> unsigned num_scheds;
>> /* set sw priority */
>> - drm_sched_entity_set_priority(&aentity->entity, priority);
>> + drm_sched_entity_set_priority(&aentity->entity,
>> + amdgpu_ctx_to_drm_sched_prio(priority));
>> /* set hw priority */
>> if (hw_ip == AMDGPU_HW_IP_COMPUTE) {
>> - hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority,
>> - AMDGPU_HW_IP_COMPUTE);
>> + hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
>> hw_prio = array_index_nospec(hw_prio, AMDGPU_RING_PRIO_MAX);
>
> Not related to this patch, but this kind of logic may break some day.
> There is a HWIP specific priority and there is another RING_PRIO which
> is unmapped to HWIP priority Ex: when a new priority is added for VCN
> which is higher than any of the existing priorities.
Yes, that's something I've noticed as well.
Either we should leave the exact mapping to the engine or have a global
definition of possible hw priorities (the later is preferable I think).
Christian.
>
> Thanks,
> Lijo
>
>> scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
>> num_scheds = adev->gpu_sched[hw_ip][hw_prio].num_scheds;
>> @@ -540,14 +599,14 @@ static void
>> amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
>> }
>> void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
>> - enum drm_sched_priority priority)
>> + int32_t priority)
>> {
>> - enum drm_sched_priority ctx_prio;
>> + int32_t ctx_prio;
>> unsigned i, j;
>> ctx->override_priority = priority;
>> - ctx_prio = (ctx->override_priority == DRM_SCHED_PRIORITY_UNSET) ?
>> + ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
>> ctx->init_priority : ctx->override_priority;
>> for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
>> for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j) {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>> index 14db16bc3322..a44b8b8ed39c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>> @@ -47,8 +47,8 @@ struct amdgpu_ctx {
>> spinlock_t ring_lock;
>> struct amdgpu_ctx_entity
>> *entities[AMDGPU_HW_IP_NUM][AMDGPU_MAX_ENTITY_NUM];
>> bool preamble_presented;
>> - enum drm_sched_priority init_priority;
>> - enum drm_sched_priority override_priority;
>> + int32_t init_priority;
>> + int32_t override_priority;
>> struct mutex lock;
>> atomic_t guilty;
>> unsigned long ras_counter_ce;
>> @@ -75,8 +75,8 @@ void amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx,
>> struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
>> struct drm_sched_entity *entity,
>> uint64_t seq);
>> -void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
>> - enum drm_sched_priority priority);
>> +bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio);
>> +void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx, int32_t
>> ctx_prio);
>> int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
>> struct drm_file *filp);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>> index b7d861ed5284..e9b45089a28a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>> @@ -32,37 +32,9 @@
>> #include "amdgpu_sched.h"
>> #include "amdgpu_vm.h"
>> -int amdgpu_to_sched_priority(int amdgpu_priority,
>> - enum drm_sched_priority *prio)
>> -{
>> - switch (amdgpu_priority) {
>> - case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>> - *prio = DRM_SCHED_PRIORITY_HIGH;
>> - break;
>> - case AMDGPU_CTX_PRIORITY_HIGH:
>> - *prio = DRM_SCHED_PRIORITY_HIGH;
>> - break;
>> - case AMDGPU_CTX_PRIORITY_NORMAL:
>> - *prio = DRM_SCHED_PRIORITY_NORMAL;
>> - break;
>> - case AMDGPU_CTX_PRIORITY_LOW:
>> - case AMDGPU_CTX_PRIORITY_VERY_LOW:
>> - *prio = DRM_SCHED_PRIORITY_MIN;
>> - break;
>> - case AMDGPU_CTX_PRIORITY_UNSET:
>> - *prio = DRM_SCHED_PRIORITY_UNSET;
>> - break;
>> - default:
>> - WARN(1, "Invalid context priority %d\n", amdgpu_priority);
>> - return -EINVAL;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> static int amdgpu_sched_process_priority_override(struct
>> amdgpu_device *adev,
>> int fd,
>> - enum drm_sched_priority priority)
>> + int32_t priority)
>> {
>> struct fd f = fdget(fd);
>> struct amdgpu_fpriv *fpriv;
>> @@ -89,7 +61,7 @@ static int
>> amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
>> static int amdgpu_sched_context_priority_override(struct
>> amdgpu_device *adev,
>> int fd,
>> unsigned ctx_id,
>> - enum drm_sched_priority priority)
>> + int32_t priority)
>> {
>> struct fd f = fdget(fd);
>> struct amdgpu_fpriv *fpriv;
>> @@ -124,7 +96,6 @@ int amdgpu_sched_ioctl(struct drm_device *dev,
>> void *data,
>> {
>> union drm_amdgpu_sched *args = data;
>> struct amdgpu_device *adev = drm_to_adev(dev);
>> - enum drm_sched_priority priority;
>> int r;
>> /* First check the op, then the op's argument.
>> @@ -138,21 +109,22 @@ int amdgpu_sched_ioctl(struct drm_device *dev,
>> void *data,
>> return -EINVAL;
>> }
>> - r = amdgpu_to_sched_priority(args->in.priority, &priority);
>> - if (r)
>> - return r;
>> + if (!amdgpu_ctx_priority_is_valid(args->in.priority)) {
>> + WARN(1, "Invalid context priority %d\n", args->in.priority);
>> + return -EINVAL;
>> + }
>> switch (args->in.op) {
>> case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
>> r = amdgpu_sched_process_priority_override(adev,
>> args->in.fd,
>> - priority);
>> + args->in.priority);
>> break;
>> case AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE:
>> r = amdgpu_sched_context_priority_override(adev,
>> args->in.fd,
>> args->in.ctx_id,
>> - priority);
>> + args->in.priority);
>> break;
>> default:
>> /* Impossible.
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: rework context priority handling
2021-08-25 12:29 ` Christian König
@ 2021-08-25 13:35 ` Das, Nirmoy
2021-08-25 13:52 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: Das, Nirmoy @ 2021-08-25 13:35 UTC (permalink / raw)
To: Christian König, Lazar, Lijo, amd-gfx; +Cc: satyajit.sahu
On 8/25/2021 2:29 PM, Christian König wrote:
> Am 25.08.21 um 14:20 schrieb Lazar, Lijo:
>> On 8/25/2021 4:52 PM, Nirmoy Das wrote:
>>> To get a hardware queue priority for a context, we are currently
>>> mapping AMDGPU_CTX_PRIORITY_* to DRM_SCHED_PRIORITY_* and then
>>> to hardware queue priority, which is not the right way to do that
>>> as DRM_SCHED_PRIORITY_* is software scheduler's priority and it is
>>> independent from a hardware queue priority.
>>>
>>> Use userspace provided context priority, AMDGPU_CTX_PRIORITY_* to
>>> map a context to proper hardware queue priority.
>>>
>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 127
>>> ++++++++++++++++------
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 8 +-
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 44 ++------
>>> 3 files changed, 105 insertions(+), 74 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> index e7a010b7ca1f..c88c5c6c54a2 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> @@ -43,14 +43,61 @@ const unsigned int
>>> amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = {
>>> [AMDGPU_HW_IP_VCN_JPEG] = 1,
>>> };
>>> +bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio)
>>> +{
>>> + switch (ctx_prio) {
>>> + case AMDGPU_CTX_PRIORITY_UNSET:
>>> + case AMDGPU_CTX_PRIORITY_VERY_LOW:
>>> + case AMDGPU_CTX_PRIORITY_LOW:
>>> + case AMDGPU_CTX_PRIORITY_NORMAL:
>>> + case AMDGPU_CTX_PRIORITY_HIGH:
>>> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>>> + return true;
>>> + default:
>>> + return false;
>>> + }
>>> +}
>>> +
>>> +static enum drm_sched_priority
>>> +amdgpu_ctx_to_drm_sched_prio(int32_t ctx_prio)
>>> +{
>>> + switch (ctx_prio) {
>>> + case AMDGPU_CTX_PRIORITY_UNSET:
>>> + return DRM_SCHED_PRIORITY_UNSET;
>>> +
>>> + case AMDGPU_CTX_PRIORITY_VERY_LOW:
>>> + return DRM_SCHED_PRIORITY_MIN;
>>> +
>>> + case AMDGPU_CTX_PRIORITY_LOW:
>>> + return DRM_SCHED_PRIORITY_MIN;
>>> +
>>> + case AMDGPU_CTX_PRIORITY_NORMAL:
>>> + return DRM_SCHED_PRIORITY_NORMAL;
>>> +
>>> + case AMDGPU_CTX_PRIORITY_HIGH:
>>> + return DRM_SCHED_PRIORITY_HIGH;
>>> +
>>> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>>> + return DRM_SCHED_PRIORITY_HIGH;
>>> +
>>> + /* This should not happen as we sanitized userspace provided
>>> priority
>>> + * already, WARN if this happens.
>>> + */
>>> + default:
>>> + WARN(1, "Invalid context priority %d\n", ctx_prio);
>>> + return DRM_SCHED_PRIORITY_NORMAL;
>>> + }
>>> +
>>> +}
>>> +
>>> static int amdgpu_ctx_priority_permit(struct drm_file *filp,
>>> - enum drm_sched_priority priority)
>>> + int32_t priority)
>>> {
>>> - if (priority < 0 || priority >= DRM_SCHED_PRIORITY_COUNT)
>>> + if (!amdgpu_ctx_priority_is_valid(priority))
>>> return -EINVAL;
>>> /* NORMAL and below are accessible by everyone */
>>> - if (priority <= DRM_SCHED_PRIORITY_NORMAL)
>>> + if (priority <= AMDGPU_CTX_PRIORITY_NORMAL)
>>> return 0;
>>> if (capable(CAP_SYS_NICE))
>>> @@ -62,26 +109,35 @@ static int amdgpu_ctx_priority_permit(struct
>>> drm_file *filp,
>>> return -EACCES;
>>> }
>>> -static enum gfx_pipe_priority
>>> amdgpu_ctx_sched_prio_to_compute_prio(enum drm_sched_priority prio)
>>> +static enum gfx_pipe_priority
>>> amdgpu_ctx_prio_to_compute_prio(int32_t prio)
>>> {
>>> switch (prio) {
>>> - case DRM_SCHED_PRIORITY_HIGH:
>>> - case DRM_SCHED_PRIORITY_KERNEL:
>>> + case AMDGPU_CTX_PRIORITY_HIGH:
>>> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>>> return AMDGPU_GFX_PIPE_PRIO_HIGH;
>>> default:
>>> return AMDGPU_GFX_PIPE_PRIO_NORMAL;
>>> }
>>> }
>>> -static unsigned int amdgpu_ctx_prio_sched_to_hw(struct
>>> amdgpu_device *adev,
>>> - enum drm_sched_priority prio,
>>> - u32 hw_ip)
>>> +static unsigned int amdgpu_ctx_get_hw_prio(struct amdgpu_ctx *ctx,
>>> u32 hw_ip)
>>> {
>>> + struct amdgpu_device *adev = ctx->adev;
>>> + int32_t ctx_prio;
>>> unsigned int hw_prio;
>>> - hw_prio = (hw_ip == AMDGPU_HW_IP_COMPUTE) ?
>>> - amdgpu_ctx_sched_prio_to_compute_prio(prio) :
>>> - AMDGPU_RING_PRIO_DEFAULT;
>>> + ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
>>> + ctx->init_priority : ctx->override_priority;
>>> +
>>> + switch (hw_ip) {
>>> + case AMDGPU_HW_IP_COMPUTE:
>>> + hw_prio = amdgpu_ctx_prio_to_compute_prio(ctx_prio);
>>> + break;
>>> + default:
>>> + hw_prio = AMDGPU_RING_PRIO_DEFAULT;
>>> + break;
>>> + }
>>> +
>>> hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
>>> if (adev->gpu_sched[hw_ip][hw_prio].num_scheds == 0)
>>> hw_prio = AMDGPU_RING_PRIO_DEFAULT;
>>> @@ -89,15 +145,17 @@ static unsigned int
>>> amdgpu_ctx_prio_sched_to_hw(struct amdgpu_device *adev,
>>> return hw_prio;
>>> }
>>> +
>>> static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
>>> - const u32 ring)
>>> + const u32 ring)
>>> {
>>> struct amdgpu_device *adev = ctx->adev;
>>> struct amdgpu_ctx_entity *entity;
>>> struct drm_gpu_scheduler **scheds = NULL, *sched = NULL;
>>> unsigned num_scheds = 0;
>>> + int32_t ctx_prio;
>>> unsigned int hw_prio;
>>> - enum drm_sched_priority priority;
>>> + enum drm_sched_priority drm_prio;
>>> int r;
>>> entity = kzalloc(struct_size(entity, fences,
>>> amdgpu_sched_jobs),
>>> @@ -105,10 +163,11 @@ static int amdgpu_ctx_init_entity(struct
>>> amdgpu_ctx *ctx, u32 hw_ip,
>>> if (!entity)
>>> return -ENOMEM;
>>> + ctx_prio = (ctx->override_priority ==
>>> AMDGPU_CTX_PRIORITY_UNSET) ?
>>> + ctx->init_priority : ctx->override_priority;
>>> entity->sequence = 1;
>>> - priority = (ctx->override_priority == DRM_SCHED_PRIORITY_UNSET) ?
>>> - ctx->init_priority : ctx->override_priority;
>>> - hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority, hw_ip);
>>> + hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
>>> + drm_prio = amdgpu_ctx_to_drm_sched_prio(ctx_prio);
>>> hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
>>> scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
>>> @@ -124,7 +183,7 @@ static int amdgpu_ctx_init_entity(struct
>>> amdgpu_ctx *ctx, u32 hw_ip,
>>> num_scheds = 1;
>>> }
>>> - r = drm_sched_entity_init(&entity->entity, priority, scheds,
>>> num_scheds,
>>> + r = drm_sched_entity_init(&entity->entity, drm_prio, scheds,
>>> num_scheds,
>>> &ctx->guilty);
>>> if (r)
>>> goto error_free_entity;
>>> @@ -139,7 +198,7 @@ static int amdgpu_ctx_init_entity(struct
>>> amdgpu_ctx *ctx, u32 hw_ip,
>>> }
>>> static int amdgpu_ctx_init(struct amdgpu_device *adev,
>>> - enum drm_sched_priority priority,
>>> + int32_t priority,
>>> struct drm_file *filp,
>>> struct amdgpu_ctx *ctx)
>>> {
>>> @@ -161,7 +220,7 @@ static int amdgpu_ctx_init(struct amdgpu_device
>>> *adev,
>>> ctx->reset_counter_query = ctx->reset_counter;
>>> ctx->vram_lost_counter = atomic_read(&adev->vram_lost_counter);
>>> ctx->init_priority = priority;
>>> - ctx->override_priority = DRM_SCHED_PRIORITY_UNSET;
>>> + ctx->override_priority = AMDGPU_CTX_PRIORITY_UNSET;
>>> return 0;
>>> }
>>> @@ -234,7 +293,7 @@ int amdgpu_ctx_get_entity(struct amdgpu_ctx
>>> *ctx, u32 hw_ip, u32 instance,
>>> static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
>>> struct amdgpu_fpriv *fpriv,
>>> struct drm_file *filp,
>>> - enum drm_sched_priority priority,
>>> + int32_t priority,
>>> uint32_t *id)
>>> {
>>> struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
>>> @@ -397,19 +456,19 @@ int amdgpu_ctx_ioctl(struct drm_device *dev,
>>> void *data,
>>> {
>>> int r;
>>> uint32_t id;
>>> - enum drm_sched_priority priority;
>>> + int32_t priority;
>>> union drm_amdgpu_ctx *args = data;
>>> struct amdgpu_device *adev = drm_to_adev(dev);
>>> struct amdgpu_fpriv *fpriv = filp->driver_priv;
>>> id = args->in.ctx_id;
>>> - r = amdgpu_to_sched_priority(args->in.priority, &priority);
>>> + priority = args->in.priority;
>>> /* For backwards compatibility reasons, we need to accept
>>> * ioctls with garbage in the priority field */
>>> - if (r == -EINVAL)
>>> - priority = DRM_SCHED_PRIORITY_NORMAL;
>>> + if (!amdgpu_ctx_priority_is_valid(priority))
>>> + priority = AMDGPU_CTX_PRIORITY_NORMAL;
>>> switch (args->in.op) {
>>> case AMDGPU_CTX_OP_ALLOC_CTX:
>>> @@ -515,9 +574,9 @@ struct dma_fence *amdgpu_ctx_get_fence(struct
>>> amdgpu_ctx *ctx,
>>> }
>>> static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
>>> - struct amdgpu_ctx_entity *aentity,
>>> - int hw_ip,
>>> - enum drm_sched_priority priority)
>>> + struct amdgpu_ctx_entity *aentity,
>>> + int hw_ip,
>>> + int32_t priority)
>>> {
>>> struct amdgpu_device *adev = ctx->adev;
>>> unsigned int hw_prio;
>>> @@ -525,12 +584,12 @@ static void
>>> amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
>>> unsigned num_scheds;
>>> /* set sw priority */
>>> - drm_sched_entity_set_priority(&aentity->entity, priority);
>>> + drm_sched_entity_set_priority(&aentity->entity,
>>> + amdgpu_ctx_to_drm_sched_prio(priority));
>>> /* set hw priority */
>>> if (hw_ip == AMDGPU_HW_IP_COMPUTE) {
>>> - hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority,
>>> - AMDGPU_HW_IP_COMPUTE);
>>> + hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
>>> hw_prio = array_index_nospec(hw_prio, AMDGPU_RING_PRIO_MAX);
>>
>> Not related to this patch, but this kind of logic may break some day.
>> There is a HWIP specific priority and there is another RING_PRIO
>> which is unmapped to HWIP priority Ex: when a new priority is added
>> for VCN which is higher than any of the existing priorities.
>
> Yes, that's something I've noticed as well.
>
> Either we should leave the exact mapping to the engine or have a
> global definition of possible hw priorities (the later is preferable I
> think).
Will this be sufficient :
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index d43fe2ed8116..937320293029 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -43,9 +43,9 @@
#define AMDGPU_MAX_COMPUTE_QUEUES KGD_MAX_QUEUES
enum gfx_pipe_priority {
- AMDGPU_GFX_PIPE_PRIO_NORMAL = 1,
- AMDGPU_GFX_PIPE_PRIO_HIGH,
- AMDGPU_GFX_PIPE_PRIO_MAX
+ AMDGPU_GFX_PIPE_PRIO_NORMAL = AMDGPU_RING_PRIO_1,
+ AMDGPU_GFX_PIPE_PRIO_HIGH = AMDGPU_RING_PRIO_2,
+ AMDGPU_GFX_PIPE_PRIO_MAX = AMDGPU_RING_PRIO_3
};
/* Argument for PPSMC_MSG_GpuChangeState */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index e713d31619fe..c54539faf209 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -37,7 +37,14 @@
#define AMDGPU_MAX_UVD_ENC_RINGS 2
#define AMDGPU_RING_PRIO_DEFAULT 1
-#define AMDGPU_RING_PRIO_MAX AMDGPU_GFX_PIPE_PRIO_MAX
+
+enum amdgpu_ring_priority_level {
+ AMDGPU_RING_PRIO_0,
+ AMDGPU_RING_PRIO_1,
+ AMDGPU_RING_PRIO_2,
+ AMDGPU_RING_PRIO_3,
+ AMDGPU_RING_PRIO_MAX
+};
Regards,
Nirmoy
>
> Christian.
>
>>
>> Thanks,
>> Lijo
>>
>>> scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
>>> num_scheds = adev->gpu_sched[hw_ip][hw_prio].num_scheds;
>>> @@ -540,14 +599,14 @@ static void
>>> amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
>>> }
>>> void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
>>> - enum drm_sched_priority priority)
>>> + int32_t priority)
>>> {
>>> - enum drm_sched_priority ctx_prio;
>>> + int32_t ctx_prio;
>>> unsigned i, j;
>>> ctx->override_priority = priority;
>>> - ctx_prio = (ctx->override_priority ==
>>> DRM_SCHED_PRIORITY_UNSET) ?
>>> + ctx_prio = (ctx->override_priority == AMDGPU_CTX_PRIORITY_UNSET) ?
>>> ctx->init_priority : ctx->override_priority;
>>> for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
>>> for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j) {
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>> index 14db16bc3322..a44b8b8ed39c 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>> @@ -47,8 +47,8 @@ struct amdgpu_ctx {
>>> spinlock_t ring_lock;
>>> struct amdgpu_ctx_entity
>>> *entities[AMDGPU_HW_IP_NUM][AMDGPU_MAX_ENTITY_NUM];
>>> bool preamble_presented;
>>> - enum drm_sched_priority init_priority;
>>> - enum drm_sched_priority override_priority;
>>> + int32_t init_priority;
>>> + int32_t override_priority;
>>> struct mutex lock;
>>> atomic_t guilty;
>>> unsigned long ras_counter_ce;
>>> @@ -75,8 +75,8 @@ void amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx,
>>> struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
>>> struct drm_sched_entity *entity,
>>> uint64_t seq);
>>> -void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
>>> - enum drm_sched_priority priority);
>>> +bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio);
>>> +void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx, int32_t
>>> ctx_prio);
>>> int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
>>> struct drm_file *filp);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>> index b7d861ed5284..e9b45089a28a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>> @@ -32,37 +32,9 @@
>>> #include "amdgpu_sched.h"
>>> #include "amdgpu_vm.h"
>>> -int amdgpu_to_sched_priority(int amdgpu_priority,
>>> - enum drm_sched_priority *prio)
>>> -{
>>> - switch (amdgpu_priority) {
>>> - case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>>> - *prio = DRM_SCHED_PRIORITY_HIGH;
>>> - break;
>>> - case AMDGPU_CTX_PRIORITY_HIGH:
>>> - *prio = DRM_SCHED_PRIORITY_HIGH;
>>> - break;
>>> - case AMDGPU_CTX_PRIORITY_NORMAL:
>>> - *prio = DRM_SCHED_PRIORITY_NORMAL;
>>> - break;
>>> - case AMDGPU_CTX_PRIORITY_LOW:
>>> - case AMDGPU_CTX_PRIORITY_VERY_LOW:
>>> - *prio = DRM_SCHED_PRIORITY_MIN;
>>> - break;
>>> - case AMDGPU_CTX_PRIORITY_UNSET:
>>> - *prio = DRM_SCHED_PRIORITY_UNSET;
>>> - break;
>>> - default:
>>> - WARN(1, "Invalid context priority %d\n", amdgpu_priority);
>>> - return -EINVAL;
>>> - }
>>> -
>>> - return 0;
>>> -}
>>> -
>>> static int amdgpu_sched_process_priority_override(struct
>>> amdgpu_device *adev,
>>> int fd,
>>> - enum drm_sched_priority priority)
>>> + int32_t priority)
>>> {
>>> struct fd f = fdget(fd);
>>> struct amdgpu_fpriv *fpriv;
>>> @@ -89,7 +61,7 @@ static int
>>> amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
>>> static int amdgpu_sched_context_priority_override(struct
>>> amdgpu_device *adev,
>>> int fd,
>>> unsigned ctx_id,
>>> - enum drm_sched_priority priority)
>>> + int32_t priority)
>>> {
>>> struct fd f = fdget(fd);
>>> struct amdgpu_fpriv *fpriv;
>>> @@ -124,7 +96,6 @@ int amdgpu_sched_ioctl(struct drm_device *dev,
>>> void *data,
>>> {
>>> union drm_amdgpu_sched *args = data;
>>> struct amdgpu_device *adev = drm_to_adev(dev);
>>> - enum drm_sched_priority priority;
>>> int r;
>>> /* First check the op, then the op's argument.
>>> @@ -138,21 +109,22 @@ int amdgpu_sched_ioctl(struct drm_device *dev,
>>> void *data,
>>> return -EINVAL;
>>> }
>>> - r = amdgpu_to_sched_priority(args->in.priority, &priority);
>>> - if (r)
>>> - return r;
>>> + if (!amdgpu_ctx_priority_is_valid(args->in.priority)) {
>>> + WARN(1, "Invalid context priority %d\n", args->in.priority);
>>> + return -EINVAL;
>>> + }
>>> switch (args->in.op) {
>>> case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
>>> r = amdgpu_sched_process_priority_override(adev,
>>> args->in.fd,
>>> - priority);
>>> + args->in.priority);
>>> break;
>>> case AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE:
>>> r = amdgpu_sched_context_priority_override(adev,
>>> args->in.fd,
>>> args->in.ctx_id,
>>> - priority);
>>> + args->in.priority);
>>> break;
>>> default:
>>> /* Impossible.
>>>
>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: rework context priority handling
2021-08-25 13:35 ` Das, Nirmoy
@ 2021-08-25 13:52 ` Christian König
0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2021-08-25 13:52 UTC (permalink / raw)
To: Das, Nirmoy, Lazar, Lijo, amd-gfx; +Cc: satyajit.sahu
Am 25.08.21 um 15:35 schrieb Das, Nirmoy:
>
> On 8/25/2021 2:29 PM, Christian König wrote:
>> Am 25.08.21 um 14:20 schrieb Lazar, Lijo:
>>> On 8/25/2021 4:52 PM, Nirmoy Das wrote:
>>>> To get a hardware queue priority for a context, we are currently
>>>> mapping AMDGPU_CTX_PRIORITY_* to DRM_SCHED_PRIORITY_* and then
>>>> to hardware queue priority, which is not the right way to do that
>>>> as DRM_SCHED_PRIORITY_* is software scheduler's priority and it is
>>>> independent from a hardware queue priority.
>>>>
>>>> Use userspace provided context priority, AMDGPU_CTX_PRIORITY_* to
>>>> map a context to proper hardware queue priority.
>>>>
>>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 127
>>>> ++++++++++++++++------
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 8 +-
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 44 ++------
>>>> 3 files changed, 105 insertions(+), 74 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>>> index e7a010b7ca1f..c88c5c6c54a2 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>>> @@ -43,14 +43,61 @@ const unsigned int
>>>> amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = {
>>>> [AMDGPU_HW_IP_VCN_JPEG] = 1,
>>>> };
>>>> +bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio)
>>>> +{
>>>> + switch (ctx_prio) {
>>>> + case AMDGPU_CTX_PRIORITY_UNSET:
>>>> + case AMDGPU_CTX_PRIORITY_VERY_LOW:
>>>> + case AMDGPU_CTX_PRIORITY_LOW:
>>>> + case AMDGPU_CTX_PRIORITY_NORMAL:
>>>> + case AMDGPU_CTX_PRIORITY_HIGH:
>>>> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>>>> + return true;
>>>> + default:
>>>> + return false;
>>>> + }
>>>> +}
>>>> +
>>>> +static enum drm_sched_priority
>>>> +amdgpu_ctx_to_drm_sched_prio(int32_t ctx_prio)
>>>> +{
>>>> + switch (ctx_prio) {
>>>> + case AMDGPU_CTX_PRIORITY_UNSET:
>>>> + return DRM_SCHED_PRIORITY_UNSET;
>>>> +
>>>> + case AMDGPU_CTX_PRIORITY_VERY_LOW:
>>>> + return DRM_SCHED_PRIORITY_MIN;
>>>> +
>>>> + case AMDGPU_CTX_PRIORITY_LOW:
>>>> + return DRM_SCHED_PRIORITY_MIN;
>>>> +
>>>> + case AMDGPU_CTX_PRIORITY_NORMAL:
>>>> + return DRM_SCHED_PRIORITY_NORMAL;
>>>> +
>>>> + case AMDGPU_CTX_PRIORITY_HIGH:
>>>> + return DRM_SCHED_PRIORITY_HIGH;
>>>> +
>>>> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>>>> + return DRM_SCHED_PRIORITY_HIGH;
>>>> +
>>>> + /* This should not happen as we sanitized userspace provided
>>>> priority
>>>> + * already, WARN if this happens.
>>>> + */
>>>> + default:
>>>> + WARN(1, "Invalid context priority %d\n", ctx_prio);
>>>> + return DRM_SCHED_PRIORITY_NORMAL;
>>>> + }
>>>> +
>>>> +}
>>>> +
>>>> static int amdgpu_ctx_priority_permit(struct drm_file *filp,
>>>> - enum drm_sched_priority priority)
>>>> + int32_t priority)
>>>> {
>>>> - if (priority < 0 || priority >= DRM_SCHED_PRIORITY_COUNT)
>>>> + if (!amdgpu_ctx_priority_is_valid(priority))
>>>> return -EINVAL;
>>>> /* NORMAL and below are accessible by everyone */
>>>> - if (priority <= DRM_SCHED_PRIORITY_NORMAL)
>>>> + if (priority <= AMDGPU_CTX_PRIORITY_NORMAL)
>>>> return 0;
>>>> if (capable(CAP_SYS_NICE))
>>>> @@ -62,26 +109,35 @@ static int amdgpu_ctx_priority_permit(struct
>>>> drm_file *filp,
>>>> return -EACCES;
>>>> }
>>>> -static enum gfx_pipe_priority
>>>> amdgpu_ctx_sched_prio_to_compute_prio(enum drm_sched_priority prio)
>>>> +static enum gfx_pipe_priority
>>>> amdgpu_ctx_prio_to_compute_prio(int32_t prio)
>>>> {
>>>> switch (prio) {
>>>> - case DRM_SCHED_PRIORITY_HIGH:
>>>> - case DRM_SCHED_PRIORITY_KERNEL:
>>>> + case AMDGPU_CTX_PRIORITY_HIGH:
>>>> + case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>>>> return AMDGPU_GFX_PIPE_PRIO_HIGH;
>>>> default:
>>>> return AMDGPU_GFX_PIPE_PRIO_NORMAL;
>>>> }
>>>> }
>>>> -static unsigned int amdgpu_ctx_prio_sched_to_hw(struct
>>>> amdgpu_device *adev,
>>>> - enum drm_sched_priority prio,
>>>> - u32 hw_ip)
>>>> +static unsigned int amdgpu_ctx_get_hw_prio(struct amdgpu_ctx *ctx,
>>>> u32 hw_ip)
>>>> {
>>>> + struct amdgpu_device *adev = ctx->adev;
>>>> + int32_t ctx_prio;
>>>> unsigned int hw_prio;
>>>> - hw_prio = (hw_ip == AMDGPU_HW_IP_COMPUTE) ?
>>>> - amdgpu_ctx_sched_prio_to_compute_prio(prio) :
>>>> - AMDGPU_RING_PRIO_DEFAULT;
>>>> + ctx_prio = (ctx->override_priority ==
>>>> AMDGPU_CTX_PRIORITY_UNSET) ?
>>>> + ctx->init_priority : ctx->override_priority;
>>>> +
>>>> + switch (hw_ip) {
>>>> + case AMDGPU_HW_IP_COMPUTE:
>>>> + hw_prio = amdgpu_ctx_prio_to_compute_prio(ctx_prio);
>>>> + break;
>>>> + default:
>>>> + hw_prio = AMDGPU_RING_PRIO_DEFAULT;
>>>> + break;
>>>> + }
>>>> +
>>>> hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
>>>> if (adev->gpu_sched[hw_ip][hw_prio].num_scheds == 0)
>>>> hw_prio = AMDGPU_RING_PRIO_DEFAULT;
>>>> @@ -89,15 +145,17 @@ static unsigned int
>>>> amdgpu_ctx_prio_sched_to_hw(struct amdgpu_device *adev,
>>>> return hw_prio;
>>>> }
>>>> +
>>>> static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip,
>>>> - const u32 ring)
>>>> + const u32 ring)
>>>> {
>>>> struct amdgpu_device *adev = ctx->adev;
>>>> struct amdgpu_ctx_entity *entity;
>>>> struct drm_gpu_scheduler **scheds = NULL, *sched = NULL;
>>>> unsigned num_scheds = 0;
>>>> + int32_t ctx_prio;
>>>> unsigned int hw_prio;
>>>> - enum drm_sched_priority priority;
>>>> + enum drm_sched_priority drm_prio;
>>>> int r;
>>>> entity = kzalloc(struct_size(entity, fences,
>>>> amdgpu_sched_jobs),
>>>> @@ -105,10 +163,11 @@ static int amdgpu_ctx_init_entity(struct
>>>> amdgpu_ctx *ctx, u32 hw_ip,
>>>> if (!entity)
>>>> return -ENOMEM;
>>>> + ctx_prio = (ctx->override_priority ==
>>>> AMDGPU_CTX_PRIORITY_UNSET) ?
>>>> + ctx->init_priority : ctx->override_priority;
>>>> entity->sequence = 1;
>>>> - priority = (ctx->override_priority == DRM_SCHED_PRIORITY_UNSET) ?
>>>> - ctx->init_priority : ctx->override_priority;
>>>> - hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority, hw_ip);
>>>> + hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
>>>> + drm_prio = amdgpu_ctx_to_drm_sched_prio(ctx_prio);
>>>> hw_ip = array_index_nospec(hw_ip, AMDGPU_HW_IP_NUM);
>>>> scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
>>>> @@ -124,7 +183,7 @@ static int amdgpu_ctx_init_entity(struct
>>>> amdgpu_ctx *ctx, u32 hw_ip,
>>>> num_scheds = 1;
>>>> }
>>>> - r = drm_sched_entity_init(&entity->entity, priority, scheds,
>>>> num_scheds,
>>>> + r = drm_sched_entity_init(&entity->entity, drm_prio, scheds,
>>>> num_scheds,
>>>> &ctx->guilty);
>>>> if (r)
>>>> goto error_free_entity;
>>>> @@ -139,7 +198,7 @@ static int amdgpu_ctx_init_entity(struct
>>>> amdgpu_ctx *ctx, u32 hw_ip,
>>>> }
>>>> static int amdgpu_ctx_init(struct amdgpu_device *adev,
>>>> - enum drm_sched_priority priority,
>>>> + int32_t priority,
>>>> struct drm_file *filp,
>>>> struct amdgpu_ctx *ctx)
>>>> {
>>>> @@ -161,7 +220,7 @@ static int amdgpu_ctx_init(struct amdgpu_device
>>>> *adev,
>>>> ctx->reset_counter_query = ctx->reset_counter;
>>>> ctx->vram_lost_counter = atomic_read(&adev->vram_lost_counter);
>>>> ctx->init_priority = priority;
>>>> - ctx->override_priority = DRM_SCHED_PRIORITY_UNSET;
>>>> + ctx->override_priority = AMDGPU_CTX_PRIORITY_UNSET;
>>>> return 0;
>>>> }
>>>> @@ -234,7 +293,7 @@ int amdgpu_ctx_get_entity(struct amdgpu_ctx
>>>> *ctx, u32 hw_ip, u32 instance,
>>>> static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
>>>> struct amdgpu_fpriv *fpriv,
>>>> struct drm_file *filp,
>>>> - enum drm_sched_priority priority,
>>>> + int32_t priority,
>>>> uint32_t *id)
>>>> {
>>>> struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
>>>> @@ -397,19 +456,19 @@ int amdgpu_ctx_ioctl(struct drm_device *dev,
>>>> void *data,
>>>> {
>>>> int r;
>>>> uint32_t id;
>>>> - enum drm_sched_priority priority;
>>>> + int32_t priority;
>>>> union drm_amdgpu_ctx *args = data;
>>>> struct amdgpu_device *adev = drm_to_adev(dev);
>>>> struct amdgpu_fpriv *fpriv = filp->driver_priv;
>>>> id = args->in.ctx_id;
>>>> - r = amdgpu_to_sched_priority(args->in.priority, &priority);
>>>> + priority = args->in.priority;
>>>> /* For backwards compatibility reasons, we need to accept
>>>> * ioctls with garbage in the priority field */
>>>> - if (r == -EINVAL)
>>>> - priority = DRM_SCHED_PRIORITY_NORMAL;
>>>> + if (!amdgpu_ctx_priority_is_valid(priority))
>>>> + priority = AMDGPU_CTX_PRIORITY_NORMAL;
>>>> switch (args->in.op) {
>>>> case AMDGPU_CTX_OP_ALLOC_CTX:
>>>> @@ -515,9 +574,9 @@ struct dma_fence *amdgpu_ctx_get_fence(struct
>>>> amdgpu_ctx *ctx,
>>>> }
>>>> static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
>>>> - struct amdgpu_ctx_entity *aentity,
>>>> - int hw_ip,
>>>> - enum drm_sched_priority priority)
>>>> + struct amdgpu_ctx_entity *aentity,
>>>> + int hw_ip,
>>>> + int32_t priority)
>>>> {
>>>> struct amdgpu_device *adev = ctx->adev;
>>>> unsigned int hw_prio;
>>>> @@ -525,12 +584,12 @@ static void
>>>> amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
>>>> unsigned num_scheds;
>>>> /* set sw priority */
>>>> - drm_sched_entity_set_priority(&aentity->entity, priority);
>>>> + drm_sched_entity_set_priority(&aentity->entity,
>>>> + amdgpu_ctx_to_drm_sched_prio(priority));
>>>> /* set hw priority */
>>>> if (hw_ip == AMDGPU_HW_IP_COMPUTE) {
>>>> - hw_prio = amdgpu_ctx_prio_sched_to_hw(adev, priority,
>>>> - AMDGPU_HW_IP_COMPUTE);
>>>> + hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
>>>> hw_prio = array_index_nospec(hw_prio, AMDGPU_RING_PRIO_MAX);
>>>
>>> Not related to this patch, but this kind of logic may break some
>>> day. There is a HWIP specific priority and there is another
>>> RING_PRIO which is unmapped to HWIP priority Ex: when a new priority
>>> is added for VCN which is higher than any of the existing priorities.
>>
>> Yes, that's something I've noticed as well.
>>
>> Either we should leave the exact mapping to the engine or have a
>> global definition of possible hw priorities (the later is preferable
>> I think).
>
>
> Will this be sufficient :
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index d43fe2ed8116..937320293029 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -43,9 +43,9 @@
> #define AMDGPU_MAX_COMPUTE_QUEUES KGD_MAX_QUEUES
>
> enum gfx_pipe_priority {
> - AMDGPU_GFX_PIPE_PRIO_NORMAL = 1,
> - AMDGPU_GFX_PIPE_PRIO_HIGH,
> - AMDGPU_GFX_PIPE_PRIO_MAX
> + AMDGPU_GFX_PIPE_PRIO_NORMAL = AMDGPU_RING_PRIO_1,
> + AMDGPU_GFX_PIPE_PRIO_HIGH = AMDGPU_RING_PRIO_2,
> + AMDGPU_GFX_PIPE_PRIO_MAX = AMDGPU_RING_PRIO_3
> };
>
> /* Argument for PPSMC_MSG_GpuChangeState */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index e713d31619fe..c54539faf209 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -37,7 +37,14 @@
> #define AMDGPU_MAX_UVD_ENC_RINGS 2
>
> #define AMDGPU_RING_PRIO_DEFAULT 1
You should probably move the default value into the enum as well but
apart from that looks good to me.
Christian.
> -#define AMDGPU_RING_PRIO_MAX AMDGPU_GFX_PIPE_PRIO_MAX
> +
> +enum amdgpu_ring_priority_level {
> + AMDGPU_RING_PRIO_0,
> + AMDGPU_RING_PRIO_1,
> + AMDGPU_RING_PRIO_2,
> + AMDGPU_RING_PRIO_3,
> + AMDGPU_RING_PRIO_MAX
>
> +};
>
>
> Regards,
>
> Nirmoy
>
>>
>> Christian.
>>
>>>
>>> Thanks,
>>> Lijo
>>>
>>>> scheds = adev->gpu_sched[hw_ip][hw_prio].sched;
>>>> num_scheds = adev->gpu_sched[hw_ip][hw_prio].num_scheds;
>>>> @@ -540,14 +599,14 @@ static void
>>>> amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
>>>> }
>>>> void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
>>>> - enum drm_sched_priority priority)
>>>> + int32_t priority)
>>>> {
>>>> - enum drm_sched_priority ctx_prio;
>>>> + int32_t ctx_prio;
>>>> unsigned i, j;
>>>> ctx->override_priority = priority;
>>>> - ctx_prio = (ctx->override_priority ==
>>>> DRM_SCHED_PRIORITY_UNSET) ?
>>>> + ctx_prio = (ctx->override_priority ==
>>>> AMDGPU_CTX_PRIORITY_UNSET) ?
>>>> ctx->init_priority : ctx->override_priority;
>>>> for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
>>>> for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j) {
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>>> index 14db16bc3322..a44b8b8ed39c 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>>> @@ -47,8 +47,8 @@ struct amdgpu_ctx {
>>>> spinlock_t ring_lock;
>>>> struct amdgpu_ctx_entity
>>>> *entities[AMDGPU_HW_IP_NUM][AMDGPU_MAX_ENTITY_NUM];
>>>> bool preamble_presented;
>>>> - enum drm_sched_priority init_priority;
>>>> - enum drm_sched_priority override_priority;
>>>> + int32_t init_priority;
>>>> + int32_t override_priority;
>>>> struct mutex lock;
>>>> atomic_t guilty;
>>>> unsigned long ras_counter_ce;
>>>> @@ -75,8 +75,8 @@ void amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx,
>>>> struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
>>>> struct drm_sched_entity *entity,
>>>> uint64_t seq);
>>>> -void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx,
>>>> - enum drm_sched_priority priority);
>>>> +bool amdgpu_ctx_priority_is_valid(int32_t ctx_prio);
>>>> +void amdgpu_ctx_priority_override(struct amdgpu_ctx *ctx, int32_t
>>>> ctx_prio);
>>>> int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
>>>> struct drm_file *filp);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>>> index b7d861ed5284..e9b45089a28a 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>>> @@ -32,37 +32,9 @@
>>>> #include "amdgpu_sched.h"
>>>> #include "amdgpu_vm.h"
>>>> -int amdgpu_to_sched_priority(int amdgpu_priority,
>>>> - enum drm_sched_priority *prio)
>>>> -{
>>>> - switch (amdgpu_priority) {
>>>> - case AMDGPU_CTX_PRIORITY_VERY_HIGH:
>>>> - *prio = DRM_SCHED_PRIORITY_HIGH;
>>>> - break;
>>>> - case AMDGPU_CTX_PRIORITY_HIGH:
>>>> - *prio = DRM_SCHED_PRIORITY_HIGH;
>>>> - break;
>>>> - case AMDGPU_CTX_PRIORITY_NORMAL:
>>>> - *prio = DRM_SCHED_PRIORITY_NORMAL;
>>>> - break;
>>>> - case AMDGPU_CTX_PRIORITY_LOW:
>>>> - case AMDGPU_CTX_PRIORITY_VERY_LOW:
>>>> - *prio = DRM_SCHED_PRIORITY_MIN;
>>>> - break;
>>>> - case AMDGPU_CTX_PRIORITY_UNSET:
>>>> - *prio = DRM_SCHED_PRIORITY_UNSET;
>>>> - break;
>>>> - default:
>>>> - WARN(1, "Invalid context priority %d\n", amdgpu_priority);
>>>> - return -EINVAL;
>>>> - }
>>>> -
>>>> - return 0;
>>>> -}
>>>> -
>>>> static int amdgpu_sched_process_priority_override(struct
>>>> amdgpu_device *adev,
>>>> int fd,
>>>> - enum drm_sched_priority priority)
>>>> + int32_t priority)
>>>> {
>>>> struct fd f = fdget(fd);
>>>> struct amdgpu_fpriv *fpriv;
>>>> @@ -89,7 +61,7 @@ static int
>>>> amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
>>>> static int amdgpu_sched_context_priority_override(struct
>>>> amdgpu_device *adev,
>>>> int fd,
>>>> unsigned ctx_id,
>>>> - enum drm_sched_priority priority)
>>>> + int32_t priority)
>>>> {
>>>> struct fd f = fdget(fd);
>>>> struct amdgpu_fpriv *fpriv;
>>>> @@ -124,7 +96,6 @@ int amdgpu_sched_ioctl(struct drm_device *dev,
>>>> void *data,
>>>> {
>>>> union drm_amdgpu_sched *args = data;
>>>> struct amdgpu_device *adev = drm_to_adev(dev);
>>>> - enum drm_sched_priority priority;
>>>> int r;
>>>> /* First check the op, then the op's argument.
>>>> @@ -138,21 +109,22 @@ int amdgpu_sched_ioctl(struct drm_device
>>>> *dev, void *data,
>>>> return -EINVAL;
>>>> }
>>>> - r = amdgpu_to_sched_priority(args->in.priority, &priority);
>>>> - if (r)
>>>> - return r;
>>>> + if (!amdgpu_ctx_priority_is_valid(args->in.priority)) {
>>>> + WARN(1, "Invalid context priority %d\n", args->in.priority);
>>>> + return -EINVAL;
>>>> + }
>>>> switch (args->in.op) {
>>>> case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
>>>> r = amdgpu_sched_process_priority_override(adev,
>>>> args->in.fd,
>>>> - priority);
>>>> + args->in.priority);
>>>> break;
>>>> case AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE:
>>>> r = amdgpu_sched_context_priority_override(adev,
>>>> args->in.fd,
>>>> args->in.ctx_id,
>>>> - priority);
>>>> + args->in.priority);
>>>> break;
>>>> default:
>>>> /* Impossible.
>>>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-25 13:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-25 11:22 [PATCH 1/1] drm/amdgpu: rework context priority handling Nirmoy Das
2021-08-25 11:29 ` Christian König
2021-08-25 12:20 ` Lazar, Lijo
2021-08-25 12:29 ` Christian König
2021-08-25 13:35 ` Das, Nirmoy
2021-08-25 13:52 ` Christian König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.