On Thu, 6 Nov 2025 16:18:13 +0000
Steven Price <[email protected]> wrote:
> On 06/11/2025 14:46, Boris Brezillon wrote:
> > It's the group item that's supposed to be inserted before other_group,
> > not the other way around.
> >
> > Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> > Signed-off-by: Boris Brezillon <[email protected]>
>
> Looks correct to me (but then I think I thought that about the old
> code... ;) ).
Actually, I realize I forgot to update the commit message, because
there's way more than the insertion order in the list that's
problematic.
>
> Reviewed-by: Steven Price <[email protected]>
>
> > ---
> > drivers/gpu/drm/panthor/panthor_sched.c | 46 +++++++++++++++----------
> > 1 file changed, 28 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
> > b/drivers/gpu/drm/panthor/panthor_sched.c
> > index 94514d464e64..69cc1b4c23f2 100644
> > --- a/drivers/gpu/drm/panthor/panthor_sched.c
> > +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> > @@ -1960,31 +1960,22 @@ tick_ctx_pick_groups_from_list(const struct
> > panthor_scheduler *sched,
> > static void
> > tick_ctx_insert_old_group(struct panthor_scheduler *sched,
> > struct panthor_sched_tick_ctx *ctx,
> > - struct panthor_group *group,
> > - bool full_tick)
> > + struct panthor_group *group)
> > {
> > struct panthor_csg_slot *csg_slot = &sched->csg_slots[group->csg_id];
> > struct panthor_group *other_group;
> >
> > - if (!full_tick) {
> > - list_add_tail(&group->run_node,
> > &ctx->old_groups[group->priority]);
> > - return;
> > - }
> > -
> > - /* Rotate to make sure groups with lower CSG slot
> > - * priorities have a chance to get a higher CSG slot
> > - * priority next time they get picked. This priority
> > - * has an impact on resource request ordering, so it's
> > - * important to make sure we don't let one group starve
> > - * all other groups with the same group priority.
> > - */
> > + /* Class groups in descending priority order so we can easily rotate. */
> > list_for_each_entry(other_group,
> > &ctx->old_groups[csg_slot->group->priority],
> > run_node) {
> > struct panthor_csg_slot *other_csg_slot =
> > &sched->csg_slots[other_group->csg_id];
> >
> > - if (other_csg_slot->priority > csg_slot->priority) {
> > - list_add_tail(&csg_slot->group->run_node,
> > &other_group->run_node);
> > + /* Our group has a higher prio than the one we're testing
> > against,
> > + * place it just before.
> > + */
> > + if (csg_slot->priority > other_csg_slot->priority) {
> > + list_add_tail(&group->run_node, &other_group->run_node);
> > return;
> > }
> > }
> > @@ -2033,7 +2024,7 @@ tick_ctx_init(struct panthor_scheduler *sched,
> > group->fatal_queues |=
> > GENMASK(group->queue_count - 1, 0);
> > }
> >
> > - tick_ctx_insert_old_group(sched, ctx, group, full_tick);
> > + tick_ctx_insert_old_group(sched, ctx, group);
> > csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, i,
> > csg_iface->output->ack ^
> > CSG_STATUS_UPDATE,
> > CSG_STATUS_UPDATE);
> > @@ -2399,9 +2390,28 @@ static void tick_work(struct work_struct *work)
> > for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1;
> > prio >= 0 && !tick_ctx_is_full(sched, &ctx);
> > prio--) {
> > + struct panthor_group *old_highest_prio_group =
> > + list_first_entry_or_null(&ctx.old_groups[prio],
> > + struct panthor_group,
> > run_node);
> > +
> > + /* Pull out the group with the highest prio for rotation. */
> > + if (old_highest_prio_group)
> > + list_del(&old_highest_prio_group->run_node);
> > +
> > + /* Re-insert old active groups so they get a chance to run with
> > higher prio. */
> > + tick_ctx_pick_groups_from_list(sched, &ctx,
> > &ctx.old_groups[prio], true, true);
> > +
> > + /* Fill the remaining slots with runnable groups. */
> > tick_ctx_pick_groups_from_list(sched, &ctx,
> > &sched->groups.runnable[prio],
> > true, false);
> > - tick_ctx_pick_groups_from_list(sched, &ctx,
> > &ctx.old_groups[prio], true, true);
> > +
> > + /* Re-insert the old group with the highest prio, and give it a
> > chance to be
> > + * scheduled again (but with a lower prio) if there's room left.
> > + */
> > + if (old_highest_prio_group) {
> > + list_add_tail(&old_highest_prio_group->run_node,
> > &ctx.old_groups[prio]);
> > + tick_ctx_pick_groups_from_list(sched, &ctx,
> > &ctx.old_groups[prio], true, true);
> > + }
> > }
> >
> > /* If we have free CSG slots left, pick idle groups */
>
