On Tue, 18 Aug 2026 18:20:58 +0100 Liviu Dudau <[email protected]> wrote:
> On Tue, Aug 11, 2026 at 01:23:43PM +0200, Boris Brezillon wrote: > > Make the sched_process_csg_irq_locked() call part of > > group_unbind_locked() so we don't have to manually call it in > > tick_ctx_apply()/panthor_sched_suspend(). > > > > This implies moving group_[un]bind_locked() around to avoid a > > forward declaration. > > Not sure why group_bind_locked() had to move. Aestethics? Yeah, I tend to keep do/undo helpers close to each other in general. I can make the code motion a separate commit if you prefer. > > Makes the patch a bit harder to review as one function is copied > verbatim while the other gets a line added. Minor sigh in the > end, so please ignore. > > Reviewed-by: Liviu Dudau <[email protected]> > > Best regards, > Liviu > > > > > Reviewed-by: Steven Price <[email protected]> > > Reviewed-by: Chia-I Wu <[email protected]> > > Signed-off-by: Boris Brezillon <[email protected]> > > --- > > drivers/gpu/drm/panthor/panthor_sched.c | 182 > > +++++++++++++++----------------- > > 1 file changed, 84 insertions(+), 98 deletions(-) > > > > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c > > b/drivers/gpu/drm/panthor/panthor_sched.c > > index 8016b0a55173..57308cd90e96 100644 > > --- a/drivers/gpu/drm/panthor/panthor_sched.c > > +++ b/drivers/gpu/drm/panthor/panthor_sched.c > > @@ -989,87 +989,6 @@ group_get(struct panthor_group *group) > > return group; > > } > > > > -/** > > - * group_bind_locked() - Bind a group to a group slot > > - * @group: Group. > > - * @csg_id: Slot. > > - * > > - * Return: 0 on success, a negative error code otherwise. > > - */ > > -static int > > -group_bind_locked(struct panthor_group *group, u32 csg_id) > > -{ > > - struct panthor_device *ptdev = group->ptdev; > > - int ret; > > - > > - lockdep_assert_held(&ptdev->scheduler->lock); > > - > > - if (drm_WARN_ON(&ptdev->base, group->csg_id != -1 || csg_id >= MAX_CSGS > > || > > - ptdev->scheduler->csg_slots[csg_id].group)) > > - return -EINVAL; > > - > > - ret = panthor_vm_active(group->vm); > > - if (ret) > > - return ret; > > - > > - group_get(group); > > - > > - /* Dummy doorbell allocation: doorbell is assigned to the group and > > - * all queues use the same doorbell. > > - * > > - * TODO: Implement LRU-based doorbell assignment, so the most often > > - * updated queues get their own doorbell, thus avoiding useless checks > > - * on queues belonging to the same group that are rarely updated. > > - */ > > - for (u32 i = 0; i < group->queue_count; i++) > > - group->queues[i]->doorbell_id = csg_id + 1; > > - > > - scoped_guard(spinlock, &ptdev->scheduler->events_lock) { > > - ptdev->scheduler->csg_slots[csg_id].group = group; > > - group->csg_id = csg_id; > > - } > > - > > - return 0; > > -} > > - > > -/** > > - * group_unbind_locked() - Unbind a group from a slot. > > - * @group: Group to unbind. > > - * > > - * Return: 0 on success, a negative error code otherwise. > > - */ > > -static int > > -group_unbind_locked(struct panthor_group *group) > > -{ > > - struct panthor_device *ptdev = group->ptdev; > > - > > - lockdep_assert_held(&ptdev->scheduler->lock); > > - > > - if (drm_WARN_ON(&ptdev->base, group->csg_id < 0 || group->csg_id >= > > MAX_CSGS)) > > - return -EINVAL; > > - > > - if (drm_WARN_ON(&ptdev->base, group->state == PANTHOR_CS_GROUP_ACTIVE)) > > - return -EINVAL; > > - > > - scoped_guard(spinlock, &ptdev->scheduler->events_lock) { > > - ptdev->scheduler->csg_slots[group->csg_id].group = NULL; > > - group->csg_id = -1; > > - } > > - > > - panthor_vm_idle(group->vm); > > - > > - /* Tiler OOM events will be re-issued next time the group is scheduled. > > */ > > - atomic_set(&group->tiler_oom, 0); > > - if (cancel_work(&group->tiler_oom_work)) > > - group_put(group); > > - > > - for (u32 i = 0; i < group->queue_count; i++) > > - group->queues[i]->doorbell_id = -1; > > - > > - group_put(group); > > - return 0; > > -} > > - > > static bool > > group_is_idle(struct panthor_group *group) > > { > > @@ -1979,6 +1898,89 @@ void panthor_sched_report_fw_events(struct > > panthor_device *ptdev, u32 events) > > } > > } > > > > +/** > > + * group_bind_locked() - Bind a group to a group slot > > + * @group: Group. > > + * @csg_id: Slot. > > + * > > + * Return: 0 on success, a negative error code otherwise. > > + */ > > +static int > > +group_bind_locked(struct panthor_group *group, u32 csg_id) > > +{ > > + struct panthor_device *ptdev = group->ptdev; > > + int ret; > > + > > + lockdep_assert_held(&ptdev->scheduler->lock); > > + > > + if (drm_WARN_ON(&ptdev->base, group->csg_id != -1 || csg_id >= MAX_CSGS > > || > > + ptdev->scheduler->csg_slots[csg_id].group)) > > + return -EINVAL; > > + > > + ret = panthor_vm_active(group->vm); > > + if (ret) > > + return ret; > > + > > + group_get(group); > > + > > + /* Dummy doorbell allocation: doorbell is assigned to the group and > > + * all queues use the same doorbell. > > + * > > + * TODO: Implement LRU-based doorbell assignment, so the most often > > + * updated queues get their own doorbell, thus avoiding useless checks > > + * on queues belonging to the same group that are rarely updated. > > + */ > > + for (u32 i = 0; i < group->queue_count; i++) > > + group->queues[i]->doorbell_id = csg_id + 1; > > + > > + scoped_guard(spinlock, &ptdev->scheduler->events_lock) { > > + ptdev->scheduler->csg_slots[csg_id].group = group; > > + group->csg_id = csg_id; > > + } > > + > > + return 0; > > +} > > + > > +/** > > + * group_unbind_locked() - Unbind a group from a slot. > > + * @group: Group to unbind. > > + * > > + * Return: 0 on success, a negative error code otherwise. > > + */ > > +static int > > +group_unbind_locked(struct panthor_group *group) > > +{ > > + struct panthor_device *ptdev = group->ptdev; > > + > > + lockdep_assert_held(&ptdev->scheduler->lock); > > + > > + if (drm_WARN_ON(&ptdev->base, group->csg_id < 0 || group->csg_id >= > > MAX_CSGS)) > > + return -EINVAL; > > + > > + if (drm_WARN_ON(&ptdev->base, group->state == PANTHOR_CS_GROUP_ACTIVE)) > > + return -EINVAL; > > + > > + scoped_guard(spinlock, &ptdev->scheduler->events_lock) { > > + /* Process all pending IRQs before returning the slot. */ > > + sched_process_csg_irq_locked(ptdev, group->csg_id); > > + ptdev->scheduler->csg_slots[group->csg_id].group = NULL; > > + group->csg_id = -1; > > + } > > + > > + panthor_vm_idle(group->vm); > > + > > + /* Tiler OOM events will be re-issued next time the group is scheduled. > > */ > > + atomic_set(&group->tiler_oom, 0); > > + if (cancel_work(&group->tiler_oom_work)) > > + group_put(group); > > + > > + for (u32 i = 0; i < group->queue_count; i++) > > + group->queues[i]->doorbell_id = -1; > > + > > + group_put(group); > > + return 0; > > +} > > + > > static const char *fence_get_driver_name(struct dma_fence *fence) > > { > > return "panthor"; > > @@ -2405,18 +2407,8 @@ tick_ctx_apply(struct panthor_scheduler *sched, > > struct panthor_sched_tick_ctx *c > > > > /* Unbind evicted groups. */ > > for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) { > > - list_for_each_entry(group, &ctx->old_groups[prio], run_node) { > > - /* This group is gone. Process interrupts to clear > > - * any pending interrupts before we start the new > > - * group. > > - */ > > - if (group->csg_id >= 0) { > > - guard(spinlock)(&sched->events_lock); > > - sched_process_csg_irq_locked(ptdev, > > group->csg_id); > > - } > > - > > + list_for_each_entry(group, &ctx->old_groups[prio], run_node) > > group_unbind_locked(group); > > - } > > } > > > > for (i = 0; i < sched->csg_slot_count; i++) { > > @@ -3010,12 +3002,6 @@ void panthor_sched_suspend(struct panthor_device > > *ptdev) > > continue; > > > > group_get(group); > > - > > - if (group->csg_id >= 0) { > > - guard(spinlock)(&sched->events_lock); > > - sched_process_csg_irq_locked(ptdev, group->csg_id); > > - } > > - > > group_unbind_locked(group); > > > > drm_WARN_ON(&group->ptdev->base, !list_empty(&group->run_node)); > > > > -- > > 2.55.0 > > >
