* [PATCH] drm/amdgpu: Add out parameters to drm_amdgpu_sched
@ 2022-04-30 15:14 Arunpravin Paneer Selvam
2022-04-30 19:29 ` Christian König
0 siblings, 1 reply; 5+ messages in thread
From: Arunpravin Paneer Selvam @ 2022-04-30 15:14 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arunpravin Paneer Selvam
- Add pipe and queue as out parameters to support high priority
queue test enabled in libdrm basic test suite.
- Fetch amdgpu_ring pointer and pass sched info to userspace
- Improve amdgpu_sched_ioctl() function
The related merge request for enabling high priority test case are
libdrm - https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/235
mesa - https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16262
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 114 ++++++++--------------
include/uapi/drm/amdgpu_drm.h | 14 ++-
2 files changed, 53 insertions(+), 75 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
index e9b45089a28a..fc2864b612d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
@@ -32,106 +32,72 @@
#include "amdgpu_sched.h"
#include "amdgpu_vm.h"
-static int amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
- int fd,
- int32_t priority)
+int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp)
{
- struct fd f = fdget(fd);
+ union drm_amdgpu_sched *args = data;
+ struct amdgpu_ctx *ctx, *ctx_ptr;
+ struct drm_sched_entity *entity;
struct amdgpu_fpriv *fpriv;
- struct amdgpu_ctx *ctx;
- uint32_t id;
- int r;
-
- if (!f.file)
- return -EINVAL;
-
- r = amdgpu_file_to_fpriv(f.file, &fpriv);
- if (r) {
- fdput(f);
- return r;
- }
-
- idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx, id)
- amdgpu_ctx_priority_override(ctx, priority);
-
- fdput(f);
- return 0;
-}
-
-static int amdgpu_sched_context_priority_override(struct amdgpu_device *adev,
- int fd,
- unsigned ctx_id,
- int32_t priority)
-{
+ struct amdgpu_ring *ring;
+ u32 fd = args->in.fd;
struct fd f = fdget(fd);
- struct amdgpu_fpriv *fpriv;
- struct amdgpu_ctx *ctx;
+ u32 id;
int r;
if (!f.file)
return -EINVAL;
r = amdgpu_file_to_fpriv(f.file, &fpriv);
- if (r) {
- fdput(f);
- return r;
- }
+ if (r)
+ goto out_fd;
- ctx = amdgpu_ctx_get(fpriv, ctx_id);
+ ctx = amdgpu_ctx_get(fpriv, args->in.ctx_id);
if (!ctx) {
- fdput(f);
- return -EINVAL;
- }
-
- amdgpu_ctx_priority_override(ctx, priority);
- amdgpu_ctx_put(ctx);
- fdput(f);
-
- return 0;
-}
-
-int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
- struct drm_file *filp)
-{
- union drm_amdgpu_sched *args = data;
- struct amdgpu_device *adev = drm_to_adev(dev);
- int r;
-
- /* First check the op, then the op's argument.
- */
- switch (args->in.op) {
- case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
- case AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE:
- break;
- default:
- DRM_ERROR("Invalid sched op specified: %d\n", args->in.op);
- return -EINVAL;
+ r = -EINVAL;
+ goto out_fd;
}
if (!amdgpu_ctx_priority_is_valid(args->in.priority)) {
WARN(1, "Invalid context priority %d\n", args->in.priority);
- return -EINVAL;
+ r = -EINVAL;
+ goto out_ctx;
}
switch (args->in.op) {
case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
- r = amdgpu_sched_process_priority_override(adev,
- args->in.fd,
- args->in.priority);
+ /* Retrieve all ctx handles and override priority */
+ idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx_ptr, id)
+ amdgpu_ctx_priority_override(ctx_ptr, 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,
- args->in.priority);
+ /* Override priority for a given context */
+ amdgpu_ctx_priority_override(ctx, args->in.priority);
break;
default:
- /* Impossible.
- */
+ DRM_ERROR("Invalid sched op specified: %d\n", args->in.op);
r = -EINVAL;
- break;
+ goto out_ctx;
}
+ r = amdgpu_ctx_get_entity(ctx, args->in.ip_type, 0, args->in.ring,
+ &entity);
+ if (r)
+ goto out_ctx;
+
+ /* Fetch amdgpu_ring pointer */
+ ring = to_amdgpu_ring(entity->rq->sched);
+
+ /* Pass sched info to userspace */
+ memset(args, 0, sizeof(*args));
+ args->out.info.pipe = ring->pipe;
+ args->out.info.queue = ring->queue;
+
+out_ctx:
+ amdgpu_ctx_put(ctx);
+out_fd:
+ fdput(f);
+
return r;
}
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 1d65c1fbc4ec..e93f1b6c4561 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -70,7 +70,7 @@ extern "C" {
#define DRM_IOCTL_AMDGPU_WAIT_FENCES DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_WAIT_FENCES, union drm_amdgpu_wait_fences)
#define DRM_IOCTL_AMDGPU_VM DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_VM, union drm_amdgpu_vm)
#define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
-#define DRM_IOCTL_AMDGPU_SCHED DRM_IOW(DRM_COMMAND_BASE + DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
+#define DRM_IOCTL_AMDGPU_SCHED DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
/**
* DOC: memory domains
@@ -308,6 +308,11 @@ union drm_amdgpu_vm {
#define AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE 1
#define AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE 2
+struct drm_amdgpu_sched_info {
+ __u32 pipe;
+ __u32 queue;
+};
+
struct drm_amdgpu_sched_in {
/* AMDGPU_SCHED_OP_* */
__u32 op;
@@ -315,10 +320,17 @@ struct drm_amdgpu_sched_in {
/** AMDGPU_CTX_PRIORITY_* */
__s32 priority;
__u32 ctx_id;
+ __u32 ip_type;
+ __u32 ring;
+};
+
+struct drm_amdgpu_sched_out {
+ struct drm_amdgpu_sched_info info;
};
union drm_amdgpu_sched {
struct drm_amdgpu_sched_in in;
+ struct drm_amdgpu_sched_out out;
};
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: Add out parameters to drm_amdgpu_sched
2022-04-30 15:14 [PATCH] drm/amdgpu: Add out parameters to drm_amdgpu_sched Arunpravin Paneer Selvam
@ 2022-04-30 19:29 ` Christian König
2022-05-01 19:45 ` Arunpravin Paneer Selvam
0 siblings, 1 reply; 5+ messages in thread
From: Christian König @ 2022-04-30 19:29 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher, christian.koenig
Am 30.04.22 um 17:14 schrieb Arunpravin Paneer Selvam:
> - Add pipe and queue as out parameters to support high priority
> queue test enabled in libdrm basic test suite.
>
> - Fetch amdgpu_ring pointer and pass sched info to userspace
>
> - Improve amdgpu_sched_ioctl() function
>
> The related merge request for enabling high priority test case are
> libdrm - https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/235
> mesa - https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16262
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Well that is certainly a NAK since as far as I can see this breaks the
UAPI in a not backward compatible way.
Then we absolutely should not pass scheduler info to userspace. That
should be completely transparent to userspace.
So can you summarize once more why that should be necessary?
Regards,
Christian.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 114 ++++++++--------------
> include/uapi/drm/amdgpu_drm.h | 14 ++-
> 2 files changed, 53 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> index e9b45089a28a..fc2864b612d9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> @@ -32,106 +32,72 @@
> #include "amdgpu_sched.h"
> #include "amdgpu_vm.h"
>
> -static int amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
> - int fd,
> - int32_t priority)
> +int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp)
> {
> - struct fd f = fdget(fd);
> + union drm_amdgpu_sched *args = data;
> + struct amdgpu_ctx *ctx, *ctx_ptr;
> + struct drm_sched_entity *entity;
> struct amdgpu_fpriv *fpriv;
> - struct amdgpu_ctx *ctx;
> - uint32_t id;
> - int r;
> -
> - if (!f.file)
> - return -EINVAL;
> -
> - r = amdgpu_file_to_fpriv(f.file, &fpriv);
> - if (r) {
> - fdput(f);
> - return r;
> - }
> -
> - idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx, id)
> - amdgpu_ctx_priority_override(ctx, priority);
> -
> - fdput(f);
> - return 0;
> -}
> -
> -static int amdgpu_sched_context_priority_override(struct amdgpu_device *adev,
> - int fd,
> - unsigned ctx_id,
> - int32_t priority)
> -{
> + struct amdgpu_ring *ring;
> + u32 fd = args->in.fd;
> struct fd f = fdget(fd);
> - struct amdgpu_fpriv *fpriv;
> - struct amdgpu_ctx *ctx;
> + u32 id;
> int r;
>
> if (!f.file)
> return -EINVAL;
>
> r = amdgpu_file_to_fpriv(f.file, &fpriv);
> - if (r) {
> - fdput(f);
> - return r;
> - }
> + if (r)
> + goto out_fd;
>
> - ctx = amdgpu_ctx_get(fpriv, ctx_id);
> + ctx = amdgpu_ctx_get(fpriv, args->in.ctx_id);
>
> if (!ctx) {
> - fdput(f);
> - return -EINVAL;
> - }
> -
> - amdgpu_ctx_priority_override(ctx, priority);
> - amdgpu_ctx_put(ctx);
> - fdput(f);
> -
> - return 0;
> -}
> -
> -int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
> - struct drm_file *filp)
> -{
> - union drm_amdgpu_sched *args = data;
> - struct amdgpu_device *adev = drm_to_adev(dev);
> - int r;
> -
> - /* First check the op, then the op's argument.
> - */
> - switch (args->in.op) {
> - case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
> - case AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE:
> - break;
> - default:
> - DRM_ERROR("Invalid sched op specified: %d\n", args->in.op);
> - return -EINVAL;
> + r = -EINVAL;
> + goto out_fd;
> }
>
> if (!amdgpu_ctx_priority_is_valid(args->in.priority)) {
> WARN(1, "Invalid context priority %d\n", args->in.priority);
> - return -EINVAL;
> + r = -EINVAL;
> + goto out_ctx;
> }
>
> switch (args->in.op) {
> case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
> - r = amdgpu_sched_process_priority_override(adev,
> - args->in.fd,
> - args->in.priority);
> + /* Retrieve all ctx handles and override priority */
> + idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx_ptr, id)
> + amdgpu_ctx_priority_override(ctx_ptr, 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,
> - args->in.priority);
> + /* Override priority for a given context */
> + amdgpu_ctx_priority_override(ctx, args->in.priority);
> break;
> default:
> - /* Impossible.
> - */
> + DRM_ERROR("Invalid sched op specified: %d\n", args->in.op);
> r = -EINVAL;
> - break;
> + goto out_ctx;
> }
>
> + r = amdgpu_ctx_get_entity(ctx, args->in.ip_type, 0, args->in.ring,
> + &entity);
> + if (r)
> + goto out_ctx;
> +
> + /* Fetch amdgpu_ring pointer */
> + ring = to_amdgpu_ring(entity->rq->sched);
> +
> + /* Pass sched info to userspace */
> + memset(args, 0, sizeof(*args));
> + args->out.info.pipe = ring->pipe;
> + args->out.info.queue = ring->queue;
> +
> +out_ctx:
> + amdgpu_ctx_put(ctx);
> +out_fd:
> + fdput(f);
> +
> return r;
> }
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 1d65c1fbc4ec..e93f1b6c4561 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -70,7 +70,7 @@ extern "C" {
> #define DRM_IOCTL_AMDGPU_WAIT_FENCES DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_WAIT_FENCES, union drm_amdgpu_wait_fences)
> #define DRM_IOCTL_AMDGPU_VM DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_VM, union drm_amdgpu_vm)
> #define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
> -#define DRM_IOCTL_AMDGPU_SCHED DRM_IOW(DRM_COMMAND_BASE + DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
> +#define DRM_IOCTL_AMDGPU_SCHED DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
>
> /**
> * DOC: memory domains
> @@ -308,6 +308,11 @@ union drm_amdgpu_vm {
> #define AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE 1
> #define AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE 2
>
> +struct drm_amdgpu_sched_info {
> + __u32 pipe;
> + __u32 queue;
> +};
> +
> struct drm_amdgpu_sched_in {
> /* AMDGPU_SCHED_OP_* */
> __u32 op;
> @@ -315,10 +320,17 @@ struct drm_amdgpu_sched_in {
> /** AMDGPU_CTX_PRIORITY_* */
> __s32 priority;
> __u32 ctx_id;
> + __u32 ip_type;
> + __u32 ring;
> +};
> +
> +struct drm_amdgpu_sched_out {
> + struct drm_amdgpu_sched_info info;
> };
>
> union drm_amdgpu_sched {
> struct drm_amdgpu_sched_in in;
> + struct drm_amdgpu_sched_out out;
> };
>
> /*
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: Add out parameters to drm_amdgpu_sched
2022-04-30 19:29 ` Christian König
@ 2022-05-01 19:45 ` Arunpravin Paneer Selvam
2022-05-02 5:56 ` Christian König
0 siblings, 1 reply; 5+ messages in thread
From: Arunpravin Paneer Selvam @ 2022-05-01 19:45 UTC (permalink / raw)
To: Christian König, amd-gfx; +Cc: alexander.deucher, christian.koenig
[-- Attachment #1: Type: text/plain, Size: 9837 bytes --]
On 5/1/2022 12:59 AM, Christian König wrote:
> Am 30.04.22 um 17:14 schrieb Arunpravin Paneer Selvam:
>> - Add pipe and queue as out parameters to support high priority
>> queue test enabled in libdrm basic test suite.
>>
>> - Fetch amdgpu_ring pointer and pass sched info to userspace
>>
>> - Improve amdgpu_sched_ioctl() function
>>
>> The related merge request for enabling high priority test case are
>> libdrm -
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fdrm%2F-%2Fmerge_requests%2F235&data=05%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Ce7a876029be4439b742808da2adfb82a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637869437601696844%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=jXIBtdA4seXl%2BYKsY2SDu%2B9tPoH6tfvUUXIRl4IosJc%3D&reserved=0
>> mesa -
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fmerge_requests%2F16262&data=05%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Ce7a876029be4439b742808da2adfb82a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637869437601696844%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2F%2FwMJ86SXxP7K9STA%2B1x5rox1F5CPEy3JIhFMCjqwiY%3D&reserved=0
>>
>> Signed-off-by: Arunpravin Paneer Selvam
>> <Arunpravin.PaneerSelvam@amd.com>
>
> Well that is certainly a NAK since as far as I can see this breaks the
> UAPI in a not backward compatible way.
>
> Then we absolutely should not pass scheduler info to userspace. That
> should be completely transparent to userspace.
>
> So can you summarize once more why that should be necessary?
I added a new test case named high priority queue test in libdrm basic
test suite to validate the kernel feature patch named
high priority gfx pipe. In the test case, we are creating a context and
overriding the priority as HIGH, submitting a simple NOP
command to test the job scheduled on high priority pipe / queue. This we
have manually verified using the sysfs entry
amdgpu_fence_info where fence signaled on gfx high priority pipe
(gfx_0.1.0). To ensure this in a unit test case, we require
pipe and queue info.
Thanks,
Arun
>
> Regards,
> Christian.
>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 114 ++++++++--------------
>> include/uapi/drm/amdgpu_drm.h | 14 ++-
>> 2 files changed, 53 insertions(+), 75 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>> index e9b45089a28a..fc2864b612d9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>> @@ -32,106 +32,72 @@
>> #include "amdgpu_sched.h"
>> #include "amdgpu_vm.h"
>> -static int amdgpu_sched_process_priority_override(struct
>> amdgpu_device *adev,
>> - int fd,
>> - int32_t priority)
>> +int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
>> + struct drm_file *filp)
>> {
>> - struct fd f = fdget(fd);
>> + union drm_amdgpu_sched *args = data;
>> + struct amdgpu_ctx *ctx, *ctx_ptr;
>> + struct drm_sched_entity *entity;
>> struct amdgpu_fpriv *fpriv;
>> - struct amdgpu_ctx *ctx;
>> - uint32_t id;
>> - int r;
>> -
>> - if (!f.file)
>> - return -EINVAL;
>> -
>> - r = amdgpu_file_to_fpriv(f.file, &fpriv);
>> - if (r) {
>> - fdput(f);
>> - return r;
>> - }
>> -
>> - idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx, id)
>> - amdgpu_ctx_priority_override(ctx, priority);
>> -
>> - fdput(f);
>> - return 0;
>> -}
>> -
>> -static int amdgpu_sched_context_priority_override(struct
>> amdgpu_device *adev,
>> - int fd,
>> - unsigned ctx_id,
>> - int32_t priority)
>> -{
>> + struct amdgpu_ring *ring;
>> + u32 fd = args->in.fd;
>> struct fd f = fdget(fd);
>> - struct amdgpu_fpriv *fpriv;
>> - struct amdgpu_ctx *ctx;
>> + u32 id;
>> int r;
>> if (!f.file)
>> return -EINVAL;
>> r = amdgpu_file_to_fpriv(f.file, &fpriv);
>> - if (r) {
>> - fdput(f);
>> - return r;
>> - }
>> + if (r)
>> + goto out_fd;
>> - ctx = amdgpu_ctx_get(fpriv, ctx_id);
>> + ctx = amdgpu_ctx_get(fpriv, args->in.ctx_id);
>> if (!ctx) {
>> - fdput(f);
>> - return -EINVAL;
>> - }
>> -
>> - amdgpu_ctx_priority_override(ctx, priority);
>> - amdgpu_ctx_put(ctx);
>> - fdput(f);
>> -
>> - return 0;
>> -}
>> -
>> -int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
>> - struct drm_file *filp)
>> -{
>> - union drm_amdgpu_sched *args = data;
>> - struct amdgpu_device *adev = drm_to_adev(dev);
>> - int r;
>> -
>> - /* First check the op, then the op's argument.
>> - */
>> - switch (args->in.op) {
>> - case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
>> - case AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE:
>> - break;
>> - default:
>> - DRM_ERROR("Invalid sched op specified: %d\n", args->in.op);
>> - return -EINVAL;
>> + r = -EINVAL;
>> + goto out_fd;
>> }
>> if (!amdgpu_ctx_priority_is_valid(args->in.priority)) {
>> WARN(1, "Invalid context priority %d\n", args->in.priority);
>> - return -EINVAL;
>> + r = -EINVAL;
>> + goto out_ctx;
>> }
>> switch (args->in.op) {
>> case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
>> - r = amdgpu_sched_process_priority_override(adev,
>> - args->in.fd,
>> - args->in.priority);
>> + /* Retrieve all ctx handles and override priority */
>> + idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx_ptr, id)
>> + amdgpu_ctx_priority_override(ctx_ptr, 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,
>> - args->in.priority);
>> + /* Override priority for a given context */
>> + amdgpu_ctx_priority_override(ctx, args->in.priority);
>> break;
>> default:
>> - /* Impossible.
>> - */
>> + DRM_ERROR("Invalid sched op specified: %d\n", args->in.op);
>> r = -EINVAL;
>> - break;
>> + goto out_ctx;
>> }
>> + r = amdgpu_ctx_get_entity(ctx, args->in.ip_type, 0,
>> args->in.ring,
>> + &entity);
>> + if (r)
>> + goto out_ctx;
>> +
>> + /* Fetch amdgpu_ring pointer */
>> + ring = to_amdgpu_ring(entity->rq->sched);
>> +
>> + /* Pass sched info to userspace */
>> + memset(args, 0, sizeof(*args));
>> + args->out.info.pipe = ring->pipe;
>> + args->out.info.queue = ring->queue;
>> +
>> +out_ctx:
>> + amdgpu_ctx_put(ctx);
>> +out_fd:
>> + fdput(f);
>> +
>> return r;
>> }
>> diff --git a/include/uapi/drm/amdgpu_drm.h
>> b/include/uapi/drm/amdgpu_drm.h
>> index 1d65c1fbc4ec..e93f1b6c4561 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -70,7 +70,7 @@ extern "C" {
>> #define DRM_IOCTL_AMDGPU_WAIT_FENCES DRM_IOWR(DRM_COMMAND_BASE +
>> DRM_AMDGPU_WAIT_FENCES, union drm_amdgpu_wait_fences)
>> #define DRM_IOCTL_AMDGPU_VM DRM_IOWR(DRM_COMMAND_BASE +
>> DRM_AMDGPU_VM, union drm_amdgpu_vm)
>> #define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE
>> + DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
>> -#define DRM_IOCTL_AMDGPU_SCHED DRM_IOW(DRM_COMMAND_BASE +
>> DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
>> +#define DRM_IOCTL_AMDGPU_SCHED DRM_IOWR(DRM_COMMAND_BASE +
>> DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
>> /**
>> * DOC: memory domains
>> @@ -308,6 +308,11 @@ union drm_amdgpu_vm {
>> #define AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE 1
>> #define AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE 2
>> +struct drm_amdgpu_sched_info {
>> + __u32 pipe;
>> + __u32 queue;
>> +};
>> +
>> struct drm_amdgpu_sched_in {
>> /* AMDGPU_SCHED_OP_* */
>> __u32 op;
>> @@ -315,10 +320,17 @@ struct drm_amdgpu_sched_in {
>> /** AMDGPU_CTX_PRIORITY_* */
>> __s32 priority;
>> __u32 ctx_id;
>> + __u32 ip_type;
>> + __u32 ring;
>> +};
>> +
>> +struct drm_amdgpu_sched_out {
>> + struct drm_amdgpu_sched_info info;
>> };
>> union drm_amdgpu_sched {
>> struct drm_amdgpu_sched_in in;
>> + struct drm_amdgpu_sched_out out;
>> };
>> /*
>
[-- Attachment #2: Type: text/html, Size: 19810 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: Add out parameters to drm_amdgpu_sched
2022-05-01 19:45 ` Arunpravin Paneer Selvam
@ 2022-05-02 5:56 ` Christian König
2022-05-02 9:51 ` Arunpravin Paneer Selvam
0 siblings, 1 reply; 5+ messages in thread
From: Christian König @ 2022-05-02 5:56 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, Christian König, amd-gfx; +Cc: alexander.deucher
[-- Attachment #1: Type: text/plain, Size: 10505 bytes --]
Am 01.05.22 um 21:45 schrieb Arunpravin Paneer Selvam:
>
> On 5/1/2022 12:59 AM, Christian König wrote:
>> Am 30.04.22 um 17:14 schrieb Arunpravin Paneer Selvam:
>>> - Add pipe and queue as out parameters to support high priority
>>> queue test enabled in libdrm basic test suite.
>>>
>>> - Fetch amdgpu_ring pointer and pass sched info to userspace
>>>
>>> - Improve amdgpu_sched_ioctl() function
>>>
>>> The related merge request for enabling high priority test case are
>>> libdrm -
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fdrm%2F-%2Fmerge_requests%2F235&data=05%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Ce7a876029be4439b742808da2adfb82a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637869437601696844%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=jXIBtdA4seXl%2BYKsY2SDu%2B9tPoH6tfvUUXIRl4IosJc%3D&reserved=0
>>> mesa -
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fmerge_requests%2F16262&data=05%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Ce7a876029be4439b742808da2adfb82a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637869437601696844%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2F%2FwMJ86SXxP7K9STA%2B1x5rox1F5CPEy3JIhFMCjqwiY%3D&reserved=0
>>>
>>> Signed-off-by: Arunpravin Paneer Selvam
>>> <Arunpravin.PaneerSelvam@amd.com>
>>
>> Well that is certainly a NAK since as far as I can see this breaks
>> the UAPI in a not backward compatible way.
>>
>> Then we absolutely should not pass scheduler info to userspace. That
>> should be completely transparent to userspace.
>>
>> So can you summarize once more why that should be necessary?
> I added a new test case named high priority queue test in libdrm basic
> test suite to validate the kernel feature patch named
> high priority gfx pipe. In the test case, we are creating a context
> and overriding the priority as HIGH, submitting a simple NOP
> command to test the job scheduled on high priority pipe / queue. This
> we have manually verified using the sysfs entry
> amdgpu_fence_info where fence signaled on gfx high priority pipe
> (gfx_0.1.0). To ensure this in a unit test case, we require
> pipe and queue info.
Completely drop that requirement, it's not even remotely valid
justification for an UAPI change.
The IOCTLs are for supporting userspace APIs like OpenGL, Vulkan, VA-API
etc.. and *not* unit tests.
If you need additional information for unit tests we need to add that to
debugfs instead and as you wrote we already have that in the form of the
amdgpu_fence_info file.
Regards,
Christian.
>
> Thanks,
> Arun
>>
>> Regards,
>> Christian.
>>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 114
>>> ++++++++--------------
>>> include/uapi/drm/amdgpu_drm.h | 14 ++-
>>> 2 files changed, 53 insertions(+), 75 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>> index e9b45089a28a..fc2864b612d9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>> @@ -32,106 +32,72 @@
>>> #include "amdgpu_sched.h"
>>> #include "amdgpu_vm.h"
>>> -static int amdgpu_sched_process_priority_override(struct
>>> amdgpu_device *adev,
>>> - int fd,
>>> - int32_t priority)
>>> +int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
>>> + struct drm_file *filp)
>>> {
>>> - struct fd f = fdget(fd);
>>> + union drm_amdgpu_sched *args = data;
>>> + struct amdgpu_ctx *ctx, *ctx_ptr;
>>> + struct drm_sched_entity *entity;
>>> struct amdgpu_fpriv *fpriv;
>>> - struct amdgpu_ctx *ctx;
>>> - uint32_t id;
>>> - int r;
>>> -
>>> - if (!f.file)
>>> - return -EINVAL;
>>> -
>>> - r = amdgpu_file_to_fpriv(f.file, &fpriv);
>>> - if (r) {
>>> - fdput(f);
>>> - return r;
>>> - }
>>> -
>>> - idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx, id)
>>> - amdgpu_ctx_priority_override(ctx, priority);
>>> -
>>> - fdput(f);
>>> - return 0;
>>> -}
>>> -
>>> -static int amdgpu_sched_context_priority_override(struct
>>> amdgpu_device *adev,
>>> - int fd,
>>> - unsigned ctx_id,
>>> - int32_t priority)
>>> -{
>>> + struct amdgpu_ring *ring;
>>> + u32 fd = args->in.fd;
>>> struct fd f = fdget(fd);
>>> - struct amdgpu_fpriv *fpriv;
>>> - struct amdgpu_ctx *ctx;
>>> + u32 id;
>>> int r;
>>> if (!f.file)
>>> return -EINVAL;
>>> r = amdgpu_file_to_fpriv(f.file, &fpriv);
>>> - if (r) {
>>> - fdput(f);
>>> - return r;
>>> - }
>>> + if (r)
>>> + goto out_fd;
>>> - ctx = amdgpu_ctx_get(fpriv, ctx_id);
>>> + ctx = amdgpu_ctx_get(fpriv, args->in.ctx_id);
>>> if (!ctx) {
>>> - fdput(f);
>>> - return -EINVAL;
>>> - }
>>> -
>>> - amdgpu_ctx_priority_override(ctx, priority);
>>> - amdgpu_ctx_put(ctx);
>>> - fdput(f);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
>>> - struct drm_file *filp)
>>> -{
>>> - union drm_amdgpu_sched *args = data;
>>> - struct amdgpu_device *adev = drm_to_adev(dev);
>>> - int r;
>>> -
>>> - /* First check the op, then the op's argument.
>>> - */
>>> - switch (args->in.op) {
>>> - case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
>>> - case AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE:
>>> - break;
>>> - default:
>>> - DRM_ERROR("Invalid sched op specified: %d\n", args->in.op);
>>> - return -EINVAL;
>>> + r = -EINVAL;
>>> + goto out_fd;
>>> }
>>> if (!amdgpu_ctx_priority_is_valid(args->in.priority)) {
>>> WARN(1, "Invalid context priority %d\n", args->in.priority);
>>> - return -EINVAL;
>>> + r = -EINVAL;
>>> + goto out_ctx;
>>> }
>>> switch (args->in.op) {
>>> case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
>>> - r = amdgpu_sched_process_priority_override(adev,
>>> - args->in.fd,
>>> - args->in.priority);
>>> + /* Retrieve all ctx handles and override priority */
>>> + idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx_ptr, id)
>>> + amdgpu_ctx_priority_override(ctx_ptr, 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,
>>> - args->in.priority);
>>> + /* Override priority for a given context */
>>> + amdgpu_ctx_priority_override(ctx, args->in.priority);
>>> break;
>>> default:
>>> - /* Impossible.
>>> - */
>>> + DRM_ERROR("Invalid sched op specified: %d\n", args->in.op);
>>> r = -EINVAL;
>>> - break;
>>> + goto out_ctx;
>>> }
>>> + r = amdgpu_ctx_get_entity(ctx, args->in.ip_type, 0,
>>> args->in.ring,
>>> + &entity);
>>> + if (r)
>>> + goto out_ctx;
>>> +
>>> + /* Fetch amdgpu_ring pointer */
>>> + ring = to_amdgpu_ring(entity->rq->sched);
>>> +
>>> + /* Pass sched info to userspace */
>>> + memset(args, 0, sizeof(*args));
>>> + args->out.info.pipe = ring->pipe;
>>> + args->out.info.queue = ring->queue;
>>> +
>>> +out_ctx:
>>> + amdgpu_ctx_put(ctx);
>>> +out_fd:
>>> + fdput(f);
>>> +
>>> return r;
>>> }
>>> diff --git a/include/uapi/drm/amdgpu_drm.h
>>> b/include/uapi/drm/amdgpu_drm.h
>>> index 1d65c1fbc4ec..e93f1b6c4561 100644
>>> --- a/include/uapi/drm/amdgpu_drm.h
>>> +++ b/include/uapi/drm/amdgpu_drm.h
>>> @@ -70,7 +70,7 @@ extern "C" {
>>> #define DRM_IOCTL_AMDGPU_WAIT_FENCES DRM_IOWR(DRM_COMMAND_BASE +
>>> DRM_AMDGPU_WAIT_FENCES, union drm_amdgpu_wait_fences)
>>> #define DRM_IOCTL_AMDGPU_VM DRM_IOWR(DRM_COMMAND_BASE +
>>> DRM_AMDGPU_VM, union drm_amdgpu_vm)
>>> #define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE
>>> + DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
>>> -#define DRM_IOCTL_AMDGPU_SCHED DRM_IOW(DRM_COMMAND_BASE +
>>> DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
>>> +#define DRM_IOCTL_AMDGPU_SCHED DRM_IOWR(DRM_COMMAND_BASE +
>>> DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
>>> /**
>>> * DOC: memory domains
>>> @@ -308,6 +308,11 @@ union drm_amdgpu_vm {
>>> #define AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE 1
>>> #define AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE 2
>>> +struct drm_amdgpu_sched_info {
>>> + __u32 pipe;
>>> + __u32 queue;
>>> +};
>>> +
>>> struct drm_amdgpu_sched_in {
>>> /* AMDGPU_SCHED_OP_* */
>>> __u32 op;
>>> @@ -315,10 +320,17 @@ struct drm_amdgpu_sched_in {
>>> /** AMDGPU_CTX_PRIORITY_* */
>>> __s32 priority;
>>> __u32 ctx_id;
>>> + __u32 ip_type;
>>> + __u32 ring;
>>> +};
>>> +
>>> +struct drm_amdgpu_sched_out {
>>> + struct drm_amdgpu_sched_info info;
>>> };
>>> union drm_amdgpu_sched {
>>> struct drm_amdgpu_sched_in in;
>>> + struct drm_amdgpu_sched_out out;
>>> };
>>> /*
>>
>
[-- Attachment #2: Type: text/html, Size: 19113 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: Add out parameters to drm_amdgpu_sched
2022-05-02 5:56 ` Christian König
@ 2022-05-02 9:51 ` Arunpravin Paneer Selvam
0 siblings, 0 replies; 5+ messages in thread
From: Arunpravin Paneer Selvam @ 2022-05-02 9:51 UTC (permalink / raw)
To: Christian König, Christian König, amd-gfx, alexander.deucher
[-- Attachment #1: Type: text/plain, Size: 11070 bytes --]
On 5/2/2022 11:26 AM, Christian König wrote:
> Am 01.05.22 um 21:45 schrieb Arunpravin Paneer Selvam:
>>
>> On 5/1/2022 12:59 AM, Christian König wrote:
>>> Am 30.04.22 um 17:14 schrieb Arunpravin Paneer Selvam:
>>>> - Add pipe and queue as out parameters to support high priority
>>>> queue test enabled in libdrm basic test suite.
>>>>
>>>> - Fetch amdgpu_ring pointer and pass sched info to userspace
>>>>
>>>> - Improve amdgpu_sched_ioctl() function
>>>>
>>>> The related merge request for enabling high priority test case are
>>>> libdrm -
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fdrm%2F-%2Fmerge_requests%2F235&data=05%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Ce7a876029be4439b742808da2adfb82a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637869437601696844%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=jXIBtdA4seXl%2BYKsY2SDu%2B9tPoH6tfvUUXIRl4IosJc%3D&reserved=0
>>>> mesa -
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fmerge_requests%2F16262&data=05%7C01%7CArunpravin.PaneerSelvam%40amd.com%7Ce7a876029be4439b742808da2adfb82a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637869437601696844%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2F%2FwMJ86SXxP7K9STA%2B1x5rox1F5CPEy3JIhFMCjqwiY%3D&reserved=0
>>>>
>>>> Signed-off-by: Arunpravin Paneer Selvam
>>>> <Arunpravin.PaneerSelvam@amd.com>
>>>
>>> Well that is certainly a NAK since as far as I can see this breaks
>>> the UAPI in a not backward compatible way.
>>>
>>> Then we absolutely should not pass scheduler info to userspace. That
>>> should be completely transparent to userspace.
>>>
>>> So can you summarize once more why that should be necessary?
>> I added a new test case named high priority queue test in libdrm
>> basic test suite to validate the kernel feature patch named
>> high priority gfx pipe. In the test case, we are creating a context
>> and overriding the priority as HIGH, submitting a simple NOP
>> command to test the job scheduled on high priority pipe / queue. This
>> we have manually verified using the sysfs entry
>> amdgpu_fence_info where fence signaled on gfx high priority pipe
>> (gfx_0.1.0). To ensure this in a unit test case, we require
>> pipe and queue info.
>
> Completely drop that requirement, it's not even remotely valid
> justification for an UAPI change.
>
> The IOCTLs are for supporting userspace APIs like OpenGL, Vulkan,
> VA-API etc.. and *not* unit tests.
>
> If you need additional information for unit tests we need to add that
> to debugfs instead and as you wrote we already have that in the form
> of the amdgpu_fence_info file.
sure, we will drop this requirement.
Hi Alex,
I verified the sysfs entry amdgpu_fence_info, a submitted high priority
job fence signaled on gfx_0.1.0, shall we push the kernel patch named
Enable high priority gfx queue
into staging branch.
Thanks,
Arun.
>
> Regards,
> Christian.
>
>>
>> Thanks,
>> Arun
>>>
>>> Regards,
>>> Christian.
>>>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 114
>>>> ++++++++--------------
>>>> include/uapi/drm/amdgpu_drm.h | 14 ++-
>>>> 2 files changed, 53 insertions(+), 75 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>>> index e9b45089a28a..fc2864b612d9 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
>>>> @@ -32,106 +32,72 @@
>>>> #include "amdgpu_sched.h"
>>>> #include "amdgpu_vm.h"
>>>> -static int amdgpu_sched_process_priority_override(struct
>>>> amdgpu_device *adev,
>>>> - int fd,
>>>> - int32_t priority)
>>>> +int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
>>>> + struct drm_file *filp)
>>>> {
>>>> - struct fd f = fdget(fd);
>>>> + union drm_amdgpu_sched *args = data;
>>>> + struct amdgpu_ctx *ctx, *ctx_ptr;
>>>> + struct drm_sched_entity *entity;
>>>> struct amdgpu_fpriv *fpriv;
>>>> - struct amdgpu_ctx *ctx;
>>>> - uint32_t id;
>>>> - int r;
>>>> -
>>>> - if (!f.file)
>>>> - return -EINVAL;
>>>> -
>>>> - r = amdgpu_file_to_fpriv(f.file, &fpriv);
>>>> - if (r) {
>>>> - fdput(f);
>>>> - return r;
>>>> - }
>>>> -
>>>> - idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx, id)
>>>> - amdgpu_ctx_priority_override(ctx, priority);
>>>> -
>>>> - fdput(f);
>>>> - return 0;
>>>> -}
>>>> -
>>>> -static int amdgpu_sched_context_priority_override(struct
>>>> amdgpu_device *adev,
>>>> - int fd,
>>>> - unsigned ctx_id,
>>>> - int32_t priority)
>>>> -{
>>>> + struct amdgpu_ring *ring;
>>>> + u32 fd = args->in.fd;
>>>> struct fd f = fdget(fd);
>>>> - struct amdgpu_fpriv *fpriv;
>>>> - struct amdgpu_ctx *ctx;
>>>> + u32 id;
>>>> int r;
>>>> if (!f.file)
>>>> return -EINVAL;
>>>> r = amdgpu_file_to_fpriv(f.file, &fpriv);
>>>> - if (r) {
>>>> - fdput(f);
>>>> - return r;
>>>> - }
>>>> + if (r)
>>>> + goto out_fd;
>>>> - ctx = amdgpu_ctx_get(fpriv, ctx_id);
>>>> + ctx = amdgpu_ctx_get(fpriv, args->in.ctx_id);
>>>> if (!ctx) {
>>>> - fdput(f);
>>>> - return -EINVAL;
>>>> - }
>>>> -
>>>> - amdgpu_ctx_priority_override(ctx, priority);
>>>> - amdgpu_ctx_put(ctx);
>>>> - fdput(f);
>>>> -
>>>> - return 0;
>>>> -}
>>>> -
>>>> -int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
>>>> - struct drm_file *filp)
>>>> -{
>>>> - union drm_amdgpu_sched *args = data;
>>>> - struct amdgpu_device *adev = drm_to_adev(dev);
>>>> - int r;
>>>> -
>>>> - /* First check the op, then the op's argument.
>>>> - */
>>>> - switch (args->in.op) {
>>>> - case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
>>>> - case AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE:
>>>> - break;
>>>> - default:
>>>> - DRM_ERROR("Invalid sched op specified: %d\n", args->in.op);
>>>> - return -EINVAL;
>>>> + r = -EINVAL;
>>>> + goto out_fd;
>>>> }
>>>> if (!amdgpu_ctx_priority_is_valid(args->in.priority)) {
>>>> WARN(1, "Invalid context priority %d\n", args->in.priority);
>>>> - return -EINVAL;
>>>> + r = -EINVAL;
>>>> + goto out_ctx;
>>>> }
>>>> switch (args->in.op) {
>>>> case AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE:
>>>> - r = amdgpu_sched_process_priority_override(adev,
>>>> - args->in.fd,
>>>> - args->in.priority);
>>>> + /* Retrieve all ctx handles and override priority */
>>>> + idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx_ptr, id)
>>>> + amdgpu_ctx_priority_override(ctx_ptr, 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,
>>>> - args->in.priority);
>>>> + /* Override priority for a given context */
>>>> + amdgpu_ctx_priority_override(ctx, args->in.priority);
>>>> break;
>>>> default:
>>>> - /* Impossible.
>>>> - */
>>>> + DRM_ERROR("Invalid sched op specified: %d\n", args->in.op);
>>>> r = -EINVAL;
>>>> - break;
>>>> + goto out_ctx;
>>>> }
>>>> + r = amdgpu_ctx_get_entity(ctx, args->in.ip_type, 0,
>>>> args->in.ring,
>>>> + &entity);
>>>> + if (r)
>>>> + goto out_ctx;
>>>> +
>>>> + /* Fetch amdgpu_ring pointer */
>>>> + ring = to_amdgpu_ring(entity->rq->sched);
>>>> +
>>>> + /* Pass sched info to userspace */
>>>> + memset(args, 0, sizeof(*args));
>>>> + args->out.info.pipe = ring->pipe;
>>>> + args->out.info.queue = ring->queue;
>>>> +
>>>> +out_ctx:
>>>> + amdgpu_ctx_put(ctx);
>>>> +out_fd:
>>>> + fdput(f);
>>>> +
>>>> return r;
>>>> }
>>>> diff --git a/include/uapi/drm/amdgpu_drm.h
>>>> b/include/uapi/drm/amdgpu_drm.h
>>>> index 1d65c1fbc4ec..e93f1b6c4561 100644
>>>> --- a/include/uapi/drm/amdgpu_drm.h
>>>> +++ b/include/uapi/drm/amdgpu_drm.h
>>>> @@ -70,7 +70,7 @@ extern "C" {
>>>> #define DRM_IOCTL_AMDGPU_WAIT_FENCES DRM_IOWR(DRM_COMMAND_BASE +
>>>> DRM_AMDGPU_WAIT_FENCES, union drm_amdgpu_wait_fences)
>>>> #define DRM_IOCTL_AMDGPU_VM DRM_IOWR(DRM_COMMAND_BASE +
>>>> DRM_AMDGPU_VM, union drm_amdgpu_vm)
>>>> #define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE
>>>> DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_FENCE_TO_HANDLE, union
>>>> drm_amdgpu_fence_to_handle)
>>>> -#define DRM_IOCTL_AMDGPU_SCHED DRM_IOW(DRM_COMMAND_BASE +
>>>> DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
>>>> +#define DRM_IOCTL_AMDGPU_SCHED DRM_IOWR(DRM_COMMAND_BASE +
>>>> DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
>>>> /**
>>>> * DOC: memory domains
>>>> @@ -308,6 +308,11 @@ union drm_amdgpu_vm {
>>>> #define AMDGPU_SCHED_OP_PROCESS_PRIORITY_OVERRIDE 1
>>>> #define AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE 2
>>>> +struct drm_amdgpu_sched_info {
>>>> + __u32 pipe;
>>>> + __u32 queue;
>>>> +};
>>>> +
>>>> struct drm_amdgpu_sched_in {
>>>> /* AMDGPU_SCHED_OP_* */
>>>> __u32 op;
>>>> @@ -315,10 +320,17 @@ struct drm_amdgpu_sched_in {
>>>> /** AMDGPU_CTX_PRIORITY_* */
>>>> __s32 priority;
>>>> __u32 ctx_id;
>>>> + __u32 ip_type;
>>>> + __u32 ring;
>>>> +};
>>>> +
>>>> +struct drm_amdgpu_sched_out {
>>>> + struct drm_amdgpu_sched_info info;
>>>> };
>>>> union drm_amdgpu_sched {
>>>> struct drm_amdgpu_sched_in in;
>>>> + struct drm_amdgpu_sched_out out;
>>>> };
>>>> /*
>>>
>>
>
[-- Attachment #2: Type: text/html, Size: 20360 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-02 9:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-30 15:14 [PATCH] drm/amdgpu: Add out parameters to drm_amdgpu_sched Arunpravin Paneer Selvam
2022-04-30 19:29 ` Christian König
2022-05-01 19:45 ` Arunpravin Paneer Selvam
2022-05-02 5:56 ` Christian König
2022-05-02 9:51 ` Arunpravin Paneer Selvam
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.