Re: [RFC PATCH 2/4] drm/scheduler: implement a function to modify sched list
On 2020-03-03 14:06, Christian König wrote:
> Am 02.03.20 um 21:47 schrieb Luben Tuikov:
>> On 2020-02-28 2:47 a.m., Christian König wrote:
>>> Am 28.02.20 um 06:08 schrieb Luben Tuikov:
On 2020-02-27 4:40 p.m., Nirmoy Das wrote:
> [SNIP]
> + if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
> + return -EINVAL;
This seems unmaintainable. I'd write it in its natural form:
>>> This is probably just copy & pasted from the init function and complete
>>> overkill here.
>> It should be an easy rejection then. Statements like this make
>> the code unmaintainable. Regardless of whether it was copy-and-pasted
>> I wanted to emphasize the lack of simplification of what
>> was being done.
>
> The problem is even deeper. As you noticed as well this is checking for
> in kernel coding error and not application errors.
We shouldn't check the in-kernel implementation itself. If the kernel
did this everywhere, it'd be twice the size. The kernel shouldn't be
second-guessing itself.
Another way to see this is to ask "what would the kernel do here
if XYZ was NULL", for instance? For userspace, it's clear. For
the kernel, it shows inconsistent and incomplete implmentation--something
which should be fixed.
Another way of seeing this is, if you break out a function into separate
smaller functions, checking the input parameters in those leaf/helper
functions (code which had been part of the original big function), would
make the code larger and second-guessing itself.
That check shouldn't be there. The implementation should be consistent
and complete in order to show an elegant code.
>
> That check shouldn't have been in the init function in the first place,
> but nobody had time to look into that so far.
>
>> We need to put intention and sense into what we're doing, scrutinizing
>> every line we put into a patch. This is why I suggested
>> the simplification here:
>>
int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
struct drm_gpu_scheduler **sched_list,
unsigned int num_sched_list)
{
if (entity && sched_list && (num_sched_list == 0 || sched_list[0] !=
NULL)) {
entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
entity->num_sched_list = num_sched_list;
return 0;
} else {
return -EINVAL;
}
}
>>> Actually that's a rather bad idea. Error handling should always be in
>> I actually don't think that it is a "rather bad idea". At all.
>> I actually think that it makes this leaf function more clear to
>> understand as the conditional would read like a sentence in prose.
>
> The condition is indeed easier to read, but for the sacrifice of earlier
> return and keeping prerequisite checking out of the code.
You know the compiler can reorder such code and invert
this simple-for-the-compiler conditional. It is easier
for human readers who might need to maintain it.
For instance, when you asked "Do you guys remember why we
might not have a job here?" for amdgpu_ib_schedule()
in a recent email--if that code had been split into code for
test purposes and real IB processing code, then that question
wouldn't even be on the table.
We need to achieve a balance of breaking out code and if-else
statements. At the moment the code show everything bunched
up into a single function and a ton of if-else statements,
with the pretense "to avoid duplication". Such duplication can be
avoided architecturally by redefining structures, what's in them,
and what arguments functions take.
>
>> [SNIP]
>>> What we should do instead is just: WARN_ON(!num_sched_list || !sched_list);
>> Again, what does that *mean*? What does the check mean and what
>> does the num_sched_list == 0 or sched_list == NULL mean?
>> And how did we get into a situation like this where either or both
>> could be nil?
>
> It's an in kernel coding error to do this. The caller should at least
> always provide a list with some entries in it.
>
> A WARN_ON() is appropriate since it helps to narrows down the incorrect
> behavior following from that.
>
>> Wouldn't it be better to simplify or re-architecture this (we only recently
>> decided to hide physical rings from user-space), so that the code
>> is elegant (meaning no if-else) yet flexible and straightforward?
>
> That was not recently at all, hiding physical rings was done nearly 5
> years ago shortly after the driver was initially released.
>
Why not fix the architecture so that this is simply copied?
>>> We had that and moved away from it because the scheduler list is
>>> actually const and shouldn't be allocated with each entity (which we can
>>> easily have thousands of).
>> I think that peppering the code with if-else conditionals
>> everywhere as these patch-series into the DRM scheduler have been,
>> would make the code unmaintainable in the long run.
>
>
Re: [RFC PATCH 2/4] drm/scheduler: implement a function to modify sched list
Am 02.03.20 um 21:47 schrieb Luben Tuikov:
On 2020-02-28 2:47 a.m., Christian König wrote:
Am 28.02.20 um 06:08 schrieb Luben Tuikov:
On 2020-02-27 4:40 p.m., Nirmoy Das wrote:
[SNIP]
+ if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
+ return -EINVAL;
This seems unmaintainable. I'd write it in its natural form:
This is probably just copy & pasted from the init function and complete
overkill here.
It should be an easy rejection then. Statements like this make
the code unmaintainable. Regardless of whether it was copy-and-pasted
I wanted to emphasize the lack of simplification of what
was being done.
The problem is even deeper. As you noticed as well this is checking for
in kernel coding error and not application errors.
That check shouldn't have been in the init function in the first place,
but nobody had time to look into that so far.
We need to put intention and sense into what we're doing, scrutinizing
every line we put into a patch. This is why I suggested
the simplification here:
int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
struct drm_gpu_scheduler **sched_list,
unsigned int num_sched_list)
{
if (entity && sched_list && (num_sched_list == 0 || sched_list[0] !=
NULL)) {
entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
entity->num_sched_list = num_sched_list;
return 0;
} else {
return -EINVAL;
}
}
Actually that's a rather bad idea. Error handling should always be in
I actually don't think that it is a "rather bad idea". At all.
I actually think that it makes this leaf function more clear to
understand as the conditional would read like a sentence in prose.
The condition is indeed easier to read, but for the sacrifice of earlier
return and keeping prerequisite checking out of the code.
[SNIP]
What we should do instead is just: WARN_ON(!num_sched_list || !sched_list);
Again, what does that *mean*? What does the check mean and what
does the num_sched_list == 0 or sched_list == NULL mean?
And how did we get into a situation like this where either or both
could be nil?
It's an in kernel coding error to do this. The caller should at least
always provide a list with some entries in it.
A WARN_ON() is appropriate since it helps to narrows down the incorrect
behavior following from that.
Wouldn't it be better to simplify or re-architecture this (we only recently
decided to hide physical rings from user-space), so that the code
is elegant (meaning no if-else) yet flexible and straightforward?
That was not recently at all, hiding physical rings was done nearly 5
years ago shortly after the driver was initially released.
Why not fix the architecture so that this is simply copied?
We had that and moved away from it because the scheduler list is
actually const and shouldn't be allocated with each entity (which we can
easily have thousands of).
I think that peppering the code with if-else conditionals
everywhere as these patch-series into the DRM scheduler have been,
would make the code unmaintainable in the long run.
That's something I can agree on. Using a switch to map the priority to
the backend implementation seems like the best idea to me.
E.g. function amdgpu_to_sched_priority() should not only map the IOCTL
values to the scheduler values, but also return the array which hw rings
to use.
Regards,
Christian.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH 2/4] drm/scheduler: implement a function to modify sched list
On 2020-02-28 2:47 a.m., Christian König wrote:
> Am 28.02.20 um 06:08 schrieb Luben Tuikov:
>> On 2020-02-27 4:40 p.m., Nirmoy Das wrote:
>>> implement drm_sched_entity_modify_sched() which can modify existing
>>> sched_list with a different one. This is going to be helpful when
>>> userspace changes priority of a ctx/entity then driver can switch to
>>> corresponding hw shced list for that priority
>>>
>>> Signed-off-by: Nirmoy Das
>>> ---
>>> drivers/gpu/drm/scheduler/sched_entity.c | 24
>>> include/drm/gpu_scheduler.h | 4
>>> 2 files changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
>>> b/drivers/gpu/drm/scheduler/sched_entity.c
>>> index 63bccd201b97..711e9d504bcb 100644
>>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>>> @@ -83,6 +83,30 @@ int drm_sched_entity_init(struct drm_sched_entity
>>> *entity,
>>> }
>>> EXPORT_SYMBOL(drm_sched_entity_init);
>>>
>>> +/**
>>> + * drm_sched_entity_modify_sched - Modify sched of an entity
>>> + *
>>> + * @entity: scheduler entity to init
>>> + * @sched_list: the list of new drm scheds which will replace
>>> + * existing entity->sched_list
>>> + * @num_sched_list: number of drm sched in sched_list
>>> + *
>>> + * Returns 0 on success or a negative error code on failure.
>>> + */
>>> +int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
>>> + struct drm_gpu_scheduler **sched_list,
>>> + unsigned int num_sched_list)
>>> +{
>>> + if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
>>> + return -EINVAL;
>> This seems unmaintainable. I'd write it in its natural form:
>
> This is probably just copy & pasted from the init function and complete
> overkill here.
It should be an easy rejection then. Statements like this make
the code unmaintainable. Regardless of whether it was copy-and-pasted
I wanted to emphasize the lack of simplification of what
was being done.
We need to put intention and sense into what we're doing, scrutinizing
every line we put into a patch. This is why I suggested
the simplification here:
>
>>
>> int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
>>struct drm_gpu_scheduler **sched_list,
>>unsigned int num_sched_list)
>> {
>> if (entity && sched_list && (num_sched_list == 0 || sched_list[0] !=
>> NULL)) {
>> entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
>> entity->num_sched_list = num_sched_list;
>> return 0;
>> } else {
>> return -EINVAL;
>> }
>> }
>
> Actually that's a rather bad idea. Error handling should always be in
I actually don't think that it is a "rather bad idea". At all.
I actually think that it makes this leaf function more clear to
understand as the conditional would read like a sentence in prose.
> the form of:
>
> if (check_error || missing_prerequisite)
> return_or_goto_cleanup;
I don't think we should generalize across the board. We should be
more flexible in order to create clear and maintainable code.
>
>> That's too heavy. Can we improve the architecture
>> so we don't have to check for this in leaf functions like this one?
>> We can just return a parameterization.
>>
>> Why would this be called with entity being NULL?
>> Or with sched_list being NULL? Or num_sched_list being not zero
>> yet sched_list[0] being NULL? Why not make sure that sched_list[0] is
>> never NULL and that num_sched_list is greater than 0 always?
>>
>> Does this make it to user space?
>> Why would the state of execution be one such that this is true/false
>> for the code to return -EINVAL?
>> From patch 3/4 it seems that an error is printed inside
>> amdgpu_ctx_priority_override()
>> and the result is not propagated up the stack.
>>
>> I think we should improve the code where here this condition above
>> is never true. Then we can use parameterization for those two
>> statements below:
>>
>>> +
>>> + entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
>> So if we're here, we know from the top conditional that
>> either num_sched_list is 0 or that sched_list[0] not NULL
>> or both.
>>
>> So if num_sched_list is 0 or 1 we return NULL?
>> And if num_sched_list is 2 or greater we return sched_list
>> of which sched_list[0] could be NULL?
>
> This is also copy from the init code and completely wrong here.
So even more reasons to reject this patch.
>
> What we should do instead is just: WARN_ON(!num_sched_list || !sched_list);
Again, what does that *mean*? What does the check mean and what
does the num_sched_list == 0 or sched_list == NULL mean?
And how did we get into a situation like this where either or both
could be nil?
Wouldn't it be better to simplify or re-architecture this (we only recently
decided
Re: [RFC PATCH 2/4] drm/scheduler: implement a function to modify sched list
On 2/28/20 8:47 AM, Christian König wrote:
Am 28.02.20 um 06:08 schrieb Luben Tuikov:
On 2020-02-27 4:40 p.m., Nirmoy Das wrote:
implement drm_sched_entity_modify_sched() which can modify existing
sched_list with a different one. This is going to be helpful when
userspace changes priority of a ctx/entity then driver can switch to
corresponding hw shced list for that priority
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/scheduler/sched_entity.c | 24
include/drm/gpu_scheduler.h | 4
2 files changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
b/drivers/gpu/drm/scheduler/sched_entity.c
index 63bccd201b97..711e9d504bcb 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -83,6 +83,30 @@ int drm_sched_entity_init(struct drm_sched_entity
*entity,
}
EXPORT_SYMBOL(drm_sched_entity_init);
+/**
+ * drm_sched_entity_modify_sched - Modify sched of an entity
+ *
+ * @entity: scheduler entity to init
+ * @sched_list: the list of new drm scheds which will replace
+ * existing entity->sched_list
+ * @num_sched_list: number of drm sched in sched_list
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
+ struct drm_gpu_scheduler **sched_list,
+ unsigned int num_sched_list)
+{
+ if (!(entity && sched_list && (num_sched_list == 0 ||
sched_list[0])))
+ return -EINVAL;
This seems unmaintainable. I'd write it in its natural form:
This is probably just copy & pasted from the init function and
complete overkill here.
Was indeed lame copy paste from my side.
So if num_sched_list is 0 or 1 we return NULL?
And if num_sched_list is 2 or greater we return sched_list
of which sched_list[0] could be NULL?
This is also copy from the init code and completely wrong here.
What we should do instead is just: WARN_ON(!num_sched_list ||
!sched_list);
Thanks, this will make it much simple. I will have this in next version.
Nirmoy
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH 2/4] drm/scheduler: implement a function to modify sched list
Am 28.02.20 um 06:08 schrieb Luben Tuikov:
On 2020-02-27 4:40 p.m., Nirmoy Das wrote:
implement drm_sched_entity_modify_sched() which can modify existing
sched_list with a different one. This is going to be helpful when
userspace changes priority of a ctx/entity then driver can switch to
corresponding hw shced list for that priority
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/scheduler/sched_entity.c | 24
include/drm/gpu_scheduler.h | 4
2 files changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
b/drivers/gpu/drm/scheduler/sched_entity.c
index 63bccd201b97..711e9d504bcb 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -83,6 +83,30 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
}
EXPORT_SYMBOL(drm_sched_entity_init);
+/**
+ * drm_sched_entity_modify_sched - Modify sched of an entity
+ *
+ * @entity: scheduler entity to init
+ * @sched_list: the list of new drm scheds which will replace
+ * existing entity->sched_list
+ * @num_sched_list: number of drm sched in sched_list
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
+ struct drm_gpu_scheduler **sched_list,
+ unsigned int num_sched_list)
+{
+ if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
+ return -EINVAL;
This seems unmaintainable. I'd write it in its natural form:
This is probably just copy & pasted from the init function and complete
overkill here.
int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
struct drm_gpu_scheduler **sched_list,
unsigned int num_sched_list)
{
if (entity && sched_list && (num_sched_list == 0 || sched_list[0] !=
NULL)) {
entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
entity->num_sched_list = num_sched_list;
return 0;
} else {
return -EINVAL;
}
}
Actually that's a rather bad idea. Error handling should always be in
the form of:
if (check_error || missing_prerequisite)
return_or_goto_cleanup;
That's too heavy. Can we improve the architecture
so we don't have to check for this in leaf functions like this one?
We can just return a parameterization.
Why would this be called with entity being NULL?
Or with sched_list being NULL? Or num_sched_list being not zero
yet sched_list[0] being NULL? Why not make sure that sched_list[0] is
never NULL and that num_sched_list is greater than 0 always?
Does this make it to user space?
Why would the state of execution be one such that this is true/false
for the code to return -EINVAL?
From patch 3/4 it seems that an error is printed inside
amdgpu_ctx_priority_override()
and the result is not propagated up the stack.
I think we should improve the code where here this condition above
is never true. Then we can use parameterization for those two
statements below:
+
+ entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
So if we're here, we know from the top conditional that
either num_sched_list is 0 or that sched_list[0] not NULL
or both.
So if num_sched_list is 0 or 1 we return NULL?
And if num_sched_list is 2 or greater we return sched_list
of which sched_list[0] could be NULL?
This is also copy from the init code and completely wrong here.
What we should do instead is just: WARN_ON(!num_sched_list || !sched_list);
And to the checking for not keeping around the scheduler list in the
init function.
Why not fix the architecture so that this is simply copied?
We had that and moved away from it because the scheduler list is
actually const and shouldn't be allocated with each entity (which we can
easily have thousands of).
Regards,
Christian.
(in which case
we can simply index-parameterize it and simply copy it.
Why are there so many checks everywhere?
+ entity->num_sched_list = num_sched_list;
+
I mean, all we're doing in this function is initializing
entity->sched_list and entity->num_sched_list. Why does this
function have to be so complex and do so many checks?
Can we improve/fix the architecture instead?
Regards,
Luben
+ return 0;
+}
+EXPORT_SYMBOL(drm_sched_entity_modify_sched);
+
/**
* drm_sched_entity_is_idle - Check if entity is idle
*
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 589be851f8a1..0c164a96d51b 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -297,6 +297,10 @@ void drm_sched_fini(struct drm_gpu_scheduler *sched);
int drm_sched_job_init(struct drm_sched_job *job,
struct drm_sched_entity *entity,
void *owner);
+int
Re: [RFC PATCH 2/4] drm/scheduler: implement a function to modify sched list
On 2020-02-27 4:40 p.m., Nirmoy Das wrote:
> implement drm_sched_entity_modify_sched() which can modify existing
> sched_list with a different one. This is going to be helpful when
> userspace changes priority of a ctx/entity then driver can switch to
> corresponding hw shced list for that priority
>
> Signed-off-by: Nirmoy Das
> ---
> drivers/gpu/drm/scheduler/sched_entity.c | 24
> include/drm/gpu_scheduler.h | 4
> 2 files changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
> b/drivers/gpu/drm/scheduler/sched_entity.c
> index 63bccd201b97..711e9d504bcb 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -83,6 +83,30 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
> }
> EXPORT_SYMBOL(drm_sched_entity_init);
>
> +/**
> + * drm_sched_entity_modify_sched - Modify sched of an entity
> + *
> + * @entity: scheduler entity to init
> + * @sched_list: the list of new drm scheds which will replace
> + * existing entity->sched_list
> + * @num_sched_list: number of drm sched in sched_list
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
> + struct drm_gpu_scheduler **sched_list,
> + unsigned int num_sched_list)
> +{
> + if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
> + return -EINVAL;
This seems unmaintainable. I'd write it in its natural form:
int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
struct drm_gpu_scheduler **sched_list,
unsigned int num_sched_list)
{
if (entity && sched_list && (num_sched_list == 0 || sched_list[0] !=
NULL)) {
entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
entity->num_sched_list = num_sched_list;
return 0;
} else {
return -EINVAL;
}
}
That's too heavy. Can we improve the architecture
so we don't have to check for this in leaf functions like this one?
We can just return a parameterization.
Why would this be called with entity being NULL?
Or with sched_list being NULL? Or num_sched_list being not zero
yet sched_list[0] being NULL? Why not make sure that sched_list[0] is
never NULL and that num_sched_list is greater than 0 always?
Does this make it to user space?
Why would the state of execution be one such that this is true/false
for the code to return -EINVAL?
>From patch 3/4 it seems that an error is printed inside
>amdgpu_ctx_priority_override()
and the result is not propagated up the stack.
I think we should improve the code where here this condition above
is never true. Then we can use parameterization for those two
statements below:
> +
> + entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
So if we're here, we know from the top conditional that
either num_sched_list is 0 or that sched_list[0] not NULL
or both.
So if num_sched_list is 0 or 1 we return NULL?
And if num_sched_list is 2 or greater we return sched_list
of which sched_list[0] could be NULL?
Why not fix the architecture so that this is simply copied? (in which case
we can simply index-parameterize it and simply copy it.
Why are there so many checks everywhere?
> + entity->num_sched_list = num_sched_list;
> +
I mean, all we're doing in this function is initializing
entity->sched_list and entity->num_sched_list. Why does this
function have to be so complex and do so many checks?
Can we improve/fix the architecture instead?
Regards,
Luben
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_sched_entity_modify_sched);
> +
> /**
> * drm_sched_entity_is_idle - Check if entity is idle
> *
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 589be851f8a1..0c164a96d51b 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -297,6 +297,10 @@ void drm_sched_fini(struct drm_gpu_scheduler *sched);
> int drm_sched_job_init(struct drm_sched_job *job,
> struct drm_sched_entity *entity,
> void *owner);
> +int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
> + struct drm_gpu_scheduler **sched_list,
> + unsigned int num_sched_list);
> +
> void drm_sched_job_cleanup(struct drm_sched_job *job);
> void drm_sched_wakeup(struct drm_gpu_scheduler *sched);
> void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job
> *bad);
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[RFC PATCH 2/4] drm/scheduler: implement a function to modify sched list
implement drm_sched_entity_modify_sched() which can modify existing
sched_list with a different one. This is going to be helpful when
userspace changes priority of a ctx/entity then driver can switch to
corresponding hw shced list for that priority
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/scheduler/sched_entity.c | 24
include/drm/gpu_scheduler.h | 4
2 files changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
b/drivers/gpu/drm/scheduler/sched_entity.c
index 63bccd201b97..711e9d504bcb 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -83,6 +83,30 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
}
EXPORT_SYMBOL(drm_sched_entity_init);
+/**
+ * drm_sched_entity_modify_sched - Modify sched of an entity
+ *
+ * @entity: scheduler entity to init
+ * @sched_list: the list of new drm scheds which will replace
+ * existing entity->sched_list
+ * @num_sched_list: number of drm sched in sched_list
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
+ struct drm_gpu_scheduler **sched_list,
+ unsigned int num_sched_list)
+{
+ if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
+ return -EINVAL;
+
+ entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
+ entity->num_sched_list = num_sched_list;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_sched_entity_modify_sched);
+
/**
* drm_sched_entity_is_idle - Check if entity is idle
*
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 589be851f8a1..0c164a96d51b 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -297,6 +297,10 @@ void drm_sched_fini(struct drm_gpu_scheduler *sched);
int drm_sched_job_init(struct drm_sched_job *job,
struct drm_sched_entity *entity,
void *owner);
+int drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
+ struct drm_gpu_scheduler **sched_list,
+ unsigned int num_sched_list);
+
void drm_sched_job_cleanup(struct drm_sched_job *job);
void drm_sched_wakeup(struct drm_gpu_scheduler *sched);
void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job
*bad);
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
