On 12/01/2026 12:51, Christian König wrote:
On 1/12/26 13:49, Philipp Stanner wrote:
On Mon, 2026-01-12 at 10:29 +0000, Tvrtko Ursulin wrote:
On 08/01/2026 13:54, Philipp Stanner wrote:
What's the merge plan for this series? Christian?
It sounds that staged merge would be safest. First two patches could go
to amd-next and if everything will look fine, I would follow up by
sending the DRM scheduler patch once amdgpu patches land to drm-next.
Works for me.
Sounds like a plan to me as well.
Would you be able to test the first two patches with the AMD test suite?
Or put it into amd-staging-next if it would be automatically validated
there?
Or even all three patches.
On the overall I am keen to get a clear picture whether last patch will
be a go or no-go because my fair scheduler rewrite conflicts with it.
Regards,
Tvrtko
Or if DRM scheduler maintainers are happy for the DRM scheduler patch to
also go via amd-next that is another option.
> On Wed, 2026-01-07 at 12:43 +0000, Tvrtko Ursulin wrote:
Since we have removed the case where amdgpu was initializing entitites
with either no schedulers on the list, or with a single NULL scheduler,
and there appears no other drivers which rely on this, we can simplify the
scheduler by explictly rejecting that early.
Signed-off-by: Tvrtko Ursulin <[email protected]>
Cc: Christian König <[email protected]>
Cc: Danilo Krummrich <[email protected]>
Cc: Matthew Brost <[email protected]>
Cc: Philipp Stanner <[email protected]>
---
drivers/gpu/drm/scheduler/sched_entity.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
b/drivers/gpu/drm/scheduler/sched_entity.c
index fe174a4857be..bb7e5fc47f99 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -61,32 +61,27 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
unsigned int num_sched_list,
atomic_t *guilty)
{
- if (!(entity && sched_list && (num_sched_list == 0 || sched_list[0])))
+ if (!entity || !sched_list || !num_sched_list || !sched_list[0])
I personally am a fan of checking integers explicitly against a number,
which would make the diff a bit more straightforward, too. But I accept
that like that is common kernel practice.
return -EINVAL;
memset(entity, 0, sizeof(struct drm_sched_entity));
INIT_LIST_HEAD(&entity->list);
entity->rq = NULL;
entity->guilty = guilty;
- entity->num_sched_list = num_sched_list;
entity->priority = priority;
entity->last_user = current->group_leader;
- /*
- * It's perfectly valid to initialize an entity without having a valid
- * scheduler attached. It's just not valid to use the scheduler before
it
- * is initialized itself.
- */
+ entity->num_sched_list = num_sched_list;
Why do you move that line downwards? Just leave it where it was?
num_sched_list isn't changed or anything, so I don't see a logical
connection to the line below so that grouping would make sense.
It looks completely logical to me to have both lines dealing with the
same scheduler list, accessing the same input parameter even, next to
each other:
entity->num_sched_list = num_sched_list;
entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
No? In other words, I can respin if you insist but I don't see the need.
Fine by me. Though a little sentence about that cosmetical change in
the commit message would have made that clearer.
Greetings
P.
Regards,
Tvrtko
With that:
Acked-by: Philipp Stanner <[email protected]>
P.
entity->sched_list = num_sched_list > 1 ? sched_list : NULL;
RCU_INIT_POINTER(entity->last_scheduled, NULL);
RB_CLEAR_NODE(&entity->rb_tree_node);
- if (num_sched_list && !sched_list[0]->sched_rq) {
+ if (!sched_list[0]->sched_rq) {
/* Since every entry covered by num_sched_list
* should be non-NULL and therefore we warn drivers
* not to do this and to fix their DRM calling order.
*/
pr_warn("%s: called with uninitialized scheduler\n", __func__);
- } else if (num_sched_list) {
+ } else {
/* The "priority" of an entity cannot exceed the number of
run-queues of a
* scheduler. Protect against num_rqs being 0, by converting to
signed. Choose
* the lowest priority available.
