On Mon, 17 Aug 2026 16:29:42 +0100 Steven Price <[email protected]> wrote:
> On 11/08/2026 12:23, Boris Brezillon wrote: > > Call group_check_job_completion() directly from > > csg_slot_sync_update_locked() instead of deferring it. > > > > Signed-off-by: Boris Brezillon <[email protected]> > > --- > > drivers/gpu/drm/panthor/panthor_sched.c | 182 > > +++++++++++++++----------------- > > 1 file changed, 87 insertions(+), 95 deletions(-) > > > > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c > > b/drivers/gpu/drm/panthor/panthor_sched.c > > index 794105e239e6..1ef9da55030c 100644 > > --- a/drivers/gpu/drm/panthor/panthor_sched.c > > +++ b/drivers/gpu/drm/panthor/panthor_sched.c > > @@ -697,9 +697,6 @@ struct panthor_group { > > */ > > struct panthor_kernel_bo *protm_suspend_buf; > > > > - /** @sync_upd_work: Work used to check/signal job fences. */ > > - struct work_struct sync_upd_work; > > - > > /** @tiler_oom_work: Work used to process tiler OOM events happening on > > this group. */ > > struct work_struct tiler_oom_work; > > > > @@ -1762,6 +1759,92 @@ static void > > csg_slot_process_idle_event_locked(struct panthor_device *ptdev, u32 > > sched_queue_delayed_work(sched, tick, 0); > > } > > > > +static void update_fdinfo_stats(struct panthor_job *job) > > +{ > > + struct panthor_group *group = job->group; > > + struct panthor_queue *queue = group->queues[job->queue_idx]; > > + struct panthor_gpu_usage *fdinfo = &group->fdinfo.data; > > + struct panthor_job_profiling_data *slots = queue->profiling.slots->kmap; > > + struct panthor_job_profiling_data *data = &slots[job->profiling.slot]; > > + > > + scoped_guard(spinlock_irqsave, &group->fdinfo.lock) { > > You've switched to spinlock_irqsave here, but > panthor_fdinfo_gather_group_samples() is still using plain spinlock. I > don't think we need the _irqsave as things stand here (we're in a > threaded IRQ handler). Oops, that's a leftover from the non-irqsafe -> irqsafe transition that was done in the previous revisions of this patchset. Will fix that. > > With that fixed: > > Reviewed-by: Steven Price <[email protected]> > > Thanks, > Steve > > > + if (job->profiling.mask & PANTHOR_DEVICE_PROFILING_CYCLES) > > + fdinfo->cycles += data->cycles.after - > > data->cycles.before; > > + if (job->profiling.mask & PANTHOR_DEVICE_PROFILING_TIMESTAMP) > > + fdinfo->time += data->time.after - data->time.before; > > + } > > +} > > + > > +static bool queue_check_job_completion(struct panthor_queue *queue) > > +{ > > + struct panthor_syncobj_64b *syncobj = NULL; > > + struct panthor_job *job, *job_tmp; > > + bool cookie, progress = false; > > + LIST_HEAD(done_jobs); > > + > > + cookie = dma_fence_begin_signalling(); > > + scoped_guard(spinlock_irqsave, &queue->fence_ctx.lock) { > > + list_for_each_entry_safe(job, job_tmp, > > &queue->fence_ctx.in_flight_jobs, node) { > > + if (!syncobj) { > > + struct panthor_group *group = job->group; > > + > > + syncobj = group->syncobjs->kmap + > > + (job->queue_idx * sizeof(*syncobj)); > > + } > > + > > + if (syncobj->seqno < job->done_fence->seqno) > > + break; > > + > > + list_move_tail(&job->node, &done_jobs); > > + dma_fence_signal_locked(job->done_fence); > > + } > > + > > + if (list_empty(&queue->fence_ctx.in_flight_jobs)) { > > + /* If we have no job left, we cancel the timer, and > > reset remaining > > + * time to its default so it can be restarted next time > > + * queue_resume_timeout() is called. > > + */ > > + queue_suspend_timeout_locked(queue); > > + > > + /* If there's no job pending, we consider it progress > > to avoid a > > + * spurious timeout if the timeout handler and the sync > > update > > + * handler raced. > > + */ > > + progress = true; > > + } else if (!list_empty(&done_jobs)) { > > + queue_reset_timeout_locked(queue); > > + progress = true; > > + } > > + } > > + dma_fence_end_signalling(cookie); > > + > > + list_for_each_entry_safe(job, job_tmp, &done_jobs, node) { > > + if (job->profiling.mask) > > + update_fdinfo_stats(job); > > + list_del_init(&job->node); > > + panthor_job_put(&job->base); > > + } > > + > > + return progress; > > +} > > + > > +static void group_check_job_completion(struct panthor_group *group) > > +{ > > + u32 queue_idx; > > + bool cookie; > > + > > + cookie = dma_fence_begin_signalling(); > > + for (queue_idx = 0; queue_idx < group->queue_count; queue_idx++) { > > + struct panthor_queue *queue = group->queues[queue_idx]; > > + > > + if (!queue) > > + continue; > > + > > + queue_check_job_completion(queue); > > + } > > + dma_fence_end_signalling(cookie); > > +} > > + > > static void csg_slot_sync_update_locked(struct panthor_device *ptdev, > > u32 csg_id) > > { > > @@ -1771,7 +1854,7 @@ static void csg_slot_sync_update_locked(struct > > panthor_device *ptdev, > > lockdep_assert_held(&ptdev->scheduler->events_lock); > > > > if (group) > > - group_queue_work(group, sync_upd); > > + group_check_job_completion(group); > > > > sched_queue_work(ptdev->scheduler, sync_upd); > > } > > @@ -3043,22 +3126,6 @@ void panthor_sched_post_reset(struct panthor_device > > *ptdev, bool reset_failed) > > } > > } > > > > -static void update_fdinfo_stats(struct panthor_job *job) > > -{ > > - struct panthor_group *group = job->group; > > - struct panthor_queue *queue = group->queues[job->queue_idx]; > > - struct panthor_gpu_usage *fdinfo = &group->fdinfo.data; > > - struct panthor_job_profiling_data *slots = queue->profiling.slots->kmap; > > - struct panthor_job_profiling_data *data = &slots[job->profiling.slot]; > > - > > - scoped_guard(spinlock, &group->fdinfo.lock) { > > - if (job->profiling.mask & PANTHOR_DEVICE_PROFILING_CYCLES) > > - fdinfo->cycles += data->cycles.after - > > data->cycles.before; > > - if (job->profiling.mask & PANTHOR_DEVICE_PROFILING_TIMESTAMP) > > - fdinfo->time += data->time.after - data->time.before; > > - } > > -} > > - > > void panthor_fdinfo_gather_group_samples(struct panthor_file *pfile) > > { > > struct panthor_group_pool *gpool = pfile->groups; > > @@ -3079,80 +3146,6 @@ void panthor_fdinfo_gather_group_samples(struct > > panthor_file *pfile) > > xa_unlock(&gpool->xa); > > } > > > > -static bool queue_check_job_completion(struct panthor_queue *queue) > > -{ > > - struct panthor_syncobj_64b *syncobj = NULL; > > - struct panthor_job *job, *job_tmp; > > - bool cookie, progress = false; > > - LIST_HEAD(done_jobs); > > - > > - cookie = dma_fence_begin_signalling(); > > - scoped_guard(spinlock_irqsave, &queue->fence_ctx.lock) { > > - list_for_each_entry_safe(job, job_tmp, > > &queue->fence_ctx.in_flight_jobs, node) { > > - if (!syncobj) { > > - struct panthor_group *group = job->group; > > - > > - syncobj = group->syncobjs->kmap + > > - (job->queue_idx * sizeof(*syncobj)); > > - } > > - > > - if (syncobj->seqno < job->done_fence->seqno) > > - break; > > - > > - list_move_tail(&job->node, &done_jobs); > > - dma_fence_signal_locked(job->done_fence); > > - } > > - > > - if (list_empty(&queue->fence_ctx.in_flight_jobs)) { > > - /* If we have no job left, we cancel the timer, and > > reset remaining > > - * time to its default so it can be restarted next time > > - * queue_resume_timeout() is called. > > - */ > > - queue_suspend_timeout_locked(queue); > > - > > - /* If there's no job pending, we consider it progress > > to avoid a > > - * spurious timeout if the timeout handler and the sync > > update > > - * handler raced. > > - */ > > - progress = true; > > - } else if (!list_empty(&done_jobs)) { > > - queue_reset_timeout_locked(queue); > > - progress = true; > > - } > > - } > > - dma_fence_end_signalling(cookie); > > - > > - list_for_each_entry_safe(job, job_tmp, &done_jobs, node) { > > - if (job->profiling.mask) > > - update_fdinfo_stats(job); > > - list_del_init(&job->node); > > - panthor_job_put(&job->base); > > - } > > - > > - return progress; > > -} > > - > > -static void group_sync_upd_work(struct work_struct *work) > > -{ > > - struct panthor_group *group = > > - container_of(work, struct panthor_group, sync_upd_work); > > - u32 queue_idx; > > - bool cookie; > > - > > - cookie = dma_fence_begin_signalling(); > > - for (queue_idx = 0; queue_idx < group->queue_count; queue_idx++) { > > - struct panthor_queue *queue = group->queues[queue_idx]; > > - > > - if (!queue) > > - continue; > > - > > - queue_check_job_completion(queue); > > - } > > - dma_fence_end_signalling(cookie); > > - > > - group_put(group); > > -} > > - > > struct panthor_job_ringbuf_instrs { > > u64 buffer[MAX_INSTRS_PER_JOB]; > > u32 count; > > @@ -3718,7 +3711,6 @@ int panthor_group_create(struct panthor_file *pfile, > > INIT_LIST_HEAD(&group->wait_node); > > INIT_LIST_HEAD(&group->run_node); > > INIT_WORK(&group->term_work, group_term_work); > > - INIT_WORK(&group->sync_upd_work, group_sync_upd_work); > > INIT_WORK(&group->tiler_oom_work, group_tiler_oom_work); > > INIT_WORK(&group->release_work, group_release_work); > > > > >
