public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: Chia-I Wu <olvaffe@gmail.com>
To: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Steven Price <steven.price@arm.com>,
	Liviu Dudau <liviu.dudau@arm.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 06/11] drm/panthor: Prepare the scheduler logic for FW events in IRQ context
Date: Tue, 12 May 2026 14:04:43 -0700	[thread overview]
Message-ID: <CAPaKu7RjHvRAYZDehSF9R_8T-uTrC9-NfsAPSOX0n=-2phunpg@mail.gmail.com> (raw)
In-Reply-To: <20260512-panthor-signal-from-irq-v2-6-95c614a739cb@collabora.com>

On Tue, May 12, 2026 at 5:14 AM Boris Brezillon
<boris.brezillon@collabora.com> wrote:
>
> Add a specific spinlock for events processing, and force processing
> of events in the panthor_sched_report_fw_events() path rather than
> deferring it to a work item. We also fast-track fence signalling by
> making the job completion logic IRQ-safe.
>
> Note that it requires changing a couple spin_lock() into
> spin_lock_irqsave() when those are taken inside a events_lock section.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_sched.c | 332 +++++++++++++++-----------------
>  1 file changed, 155 insertions(+), 177 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 5b34032deff8..fbf76b59b7ef 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -177,18 +177,6 @@ struct panthor_scheduler {
>          */
>         struct work_struct sync_upd_work;
>
> -       /**
> -        * @fw_events_work: Work used to process FW events outside the interrupt path.
> -        *
> -        * Even if the interrupt is threaded, we need any event processing
> -        * that require taking the panthor_scheduler::lock to be processed
> -        * outside the interrupt path so we don't block the tick logic when
> -        * it calls panthor_fw_{csg,wait}_wait_acks(). Since most of the
> -        * event processing requires taking this lock, we just delegate all
> -        * FW event processing to the scheduler workqueue.
> -        */
> -       struct work_struct fw_events_work;
> -
>         /**
>          * @fw_events: Bitmask encoding pending FW events.
>          */
If we process all fw events in the irq context, we can remove
fw_events as well. More on this below.
> @@ -254,6 +242,15 @@ struct panthor_scheduler {
>                 struct list_head waiting;
>         } groups;
>
> +       /**
> +        * @events_lock: Lock taken when processing events.
> +        *
> +        * This also needs to be taken when csg_slots are updated, to make sure
> +        * the event processing logic doesn't touch groups that have left the CSG
> +        * slot.
> +        */
> +       spinlock_t events_lock;
> +
>         /**
>          * @csg_slots: FW command stream group slots.
It looks like read access can use either lock (process context) or
events_lock (irq context), while write access must use events_lock
(process context). Can we put that into the comment, or if makes
sense, enforce that with accessor functions?


>          */
> @@ -676,9 +673,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;
> -
Can we make this a preparatory commit, where group_sync_upd_work is
replaced by group_check_job_completion?

Multiple things happen in this commit. I try to identify things that
can be separate commits. If this does not make sense, feel free to
ignore.

>         /** @tiler_oom_work: Work used to process tiler OOM events happening on this group. */
>         struct work_struct tiler_oom_work;
>
> @@ -999,7 +993,6 @@ static int
>  group_bind_locked(struct panthor_group *group, u32 csg_id)
>  {
>         struct panthor_device *ptdev = group->ptdev;
> -       struct panthor_csg_slot *csg_slot;
>         int ret;
>
>         lockdep_assert_held(&ptdev->scheduler->lock);
> @@ -1012,9 +1005,7 @@ group_bind_locked(struct panthor_group *group, u32 csg_id)
>         if (ret)
>                 return ret;
>
> -       csg_slot = &ptdev->scheduler->csg_slots[csg_id];
>         group_get(group);
> -       group->csg_id = csg_id;
>
>         /* Dummy doorbell allocation: doorbell is assigned to the group and
>          * all queues use the same doorbell.
> @@ -1026,7 +1017,10 @@ group_bind_locked(struct panthor_group *group, u32 csg_id)
>         for (u32 i = 0; i < group->queue_count; i++)
>                 group->queues[i]->doorbell_id = csg_id + 1;
>
> -       csg_slot->group = group;
> +       scoped_guard(spinlock_irqsave, &ptdev->scheduler->events_lock) {
> +               ptdev->scheduler->csg_slots[csg_id].group = group;
> +               group->csg_id = csg_id;
> +       }
>
>         return 0;
>  }
> @@ -1041,7 +1035,6 @@ static int
>  group_unbind_locked(struct panthor_group *group)
>  {
>         struct panthor_device *ptdev = group->ptdev;
> -       struct panthor_csg_slot *slot;
>
>         lockdep_assert_held(&ptdev->scheduler->lock);
>
> @@ -1051,9 +1044,12 @@ group_unbind_locked(struct panthor_group *group)
>         if (drm_WARN_ON(&ptdev->base, group->state == PANTHOR_CS_GROUP_ACTIVE))
>                 return -EINVAL;
>
> -       slot = &ptdev->scheduler->csg_slots[group->csg_id];
> +       scoped_guard(spinlock_irqsave, &ptdev->scheduler->events_lock) {
> +               ptdev->scheduler->csg_slots[group->csg_id].group = NULL;
> +               group->csg_id = -1;
> +       }
> +
>         panthor_vm_idle(group->vm);
> -       group->csg_id = -1;
>
>         /* Tiler OOM events will be re-issued next time the group is scheduled. */
>         atomic_set(&group->tiler_oom, 0);
> @@ -1062,8 +1058,6 @@ group_unbind_locked(struct panthor_group *group)
>         for (u32 i = 0; i < group->queue_count; i++)
>                 group->queues[i]->doorbell_id = -1;
>
> -       slot->group = NULL;
> -
>         group_put(group);
>         return 0;
>  }
> @@ -1151,16 +1145,14 @@ queue_suspend_timeout_locked(struct panthor_queue *queue)
>  static void
>  queue_suspend_timeout(struct panthor_queue *queue)
>  {
> -       spin_lock(&queue->fence_ctx.lock);
> +       guard(spinlock_irqsave)(&queue->fence_ctx.lock);
>         queue_suspend_timeout_locked(queue);
> -       spin_unlock(&queue->fence_ctx.lock);
>  }
>
>  static void
>  queue_resume_timeout(struct panthor_queue *queue)
>  {
> -       spin_lock(&queue->fence_ctx.lock);
> -
> +       guard(spinlock_irqsave)(&queue->fence_ctx.lock);
>         if (queue_timeout_is_suspended(queue)) {
>                 mod_delayed_work(queue->scheduler.timeout_wq,
>                                  &queue->timeout.work,
> @@ -1168,8 +1160,6 @@ queue_resume_timeout(struct panthor_queue *queue)
>
>                 queue->timeout.remaining = MAX_SCHEDULE_TIMEOUT;
>         }
> -
> -       spin_unlock(&queue->fence_ctx.lock);
>  }
>
>  /**
> @@ -1484,7 +1474,7 @@ cs_slot_process_fatal_event_locked(struct panthor_device *ptdev,
>         u32 fatal;
>         u64 info;
>
> -       lockdep_assert_held(&sched->lock);
> +       lockdep_assert_held(&sched->events_lock);
>
>         cs_iface = panthor_fw_get_cs_iface(ptdev, csg_id, cs_id);
>         fatal = cs_iface->output->fatal;
> @@ -1532,7 +1522,7 @@ cs_slot_process_fault_event_locked(struct panthor_device *ptdev,
>         u32 fault;
>         u64 info;
>
> -       lockdep_assert_held(&sched->lock);
> +       lockdep_assert_held(&sched->events_lock);
>
>         cs_iface = panthor_fw_get_cs_iface(ptdev, csg_id, cs_id);
>         fault = cs_iface->output->fault;
> @@ -1542,7 +1532,7 @@ cs_slot_process_fault_event_locked(struct panthor_device *ptdev,
>                 u64 cs_extract = queue->iface.output->extract;
>                 struct panthor_job *job;
>
> -               spin_lock(&queue->fence_ctx.lock);
> +               guard(spinlock_irqsave)(&queue->fence_ctx.lock);
>                 list_for_each_entry(job, &queue->fence_ctx.in_flight_jobs, node) {
>                         if (cs_extract >= job->ringbuf.end)
>                                 continue;
> @@ -1552,7 +1542,6 @@ cs_slot_process_fault_event_locked(struct panthor_device *ptdev,
>
>                         dma_fence_set_error(job->done_fence, -EINVAL);
>                 }
> -               spin_unlock(&queue->fence_ctx.lock);
>         }
>
>         if (group) {
> @@ -1682,7 +1671,7 @@ cs_slot_process_tiler_oom_event_locked(struct panthor_device *ptdev,
>         struct panthor_csg_slot *csg_slot = &sched->csg_slots[csg_id];
>         struct panthor_group *group = csg_slot->group;
>
> -       lockdep_assert_held(&sched->lock);
> +       lockdep_assert_held(&sched->events_lock);
>
>         if (drm_WARN_ON(&ptdev->base, !group))
>                 return;
> @@ -1703,7 +1692,7 @@ static bool cs_slot_process_irq_locked(struct panthor_device *ptdev,
>         struct panthor_fw_cs_iface *cs_iface;
>         u32 req, ack, events;
>
> -       lockdep_assert_held(&ptdev->scheduler->lock);
> +       lockdep_assert_held(&ptdev->scheduler->events_lock);
>
>         cs_iface = panthor_fw_get_cs_iface(ptdev, csg_id, cs_id);
>         req = cs_iface->input->req;
> @@ -1731,7 +1720,7 @@ static void csg_slot_process_idle_event_locked(struct panthor_device *ptdev, u32
>  {
>         struct panthor_scheduler *sched = ptdev->scheduler;
>
> -       lockdep_assert_held(&sched->lock);
> +       lockdep_assert_held(&sched->events_lock);
>
>         sched->might_have_idle_groups = true;
>
> @@ -1742,16 +1731,102 @@ 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) {
> +               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)
> +{
> +       bool cookie;
> +       u32 queue_idx;
> +
> +       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)
>  {
>         struct panthor_csg_slot *csg_slot = &ptdev->scheduler->csg_slots[csg_id];
>         struct panthor_group *group = csg_slot->group;
>
> -       lockdep_assert_held(&ptdev->scheduler->lock);
> +       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);
>  }
> @@ -1763,7 +1838,7 @@ csg_slot_process_progress_timer_event_locked(struct panthor_device *ptdev, u32 c
>         struct panthor_csg_slot *csg_slot = &sched->csg_slots[csg_id];
>         struct panthor_group *group = csg_slot->group;
>
> -       lockdep_assert_held(&sched->lock);
> +       lockdep_assert_held(&sched->events_lock);
>
>         group = csg_slot->group;
>         if (!drm_WARN_ON(&ptdev->base, !group)) {
> @@ -1784,7 +1859,7 @@ static void sched_process_csg_irq_locked(struct panthor_device *ptdev, u32 csg_i
>         struct panthor_fw_csg_iface *csg_iface;
>         u32 ring_cs_db_mask = 0;
>
> -       lockdep_assert_held(&ptdev->scheduler->lock);
> +       lockdep_assert_held(&ptdev->scheduler->events_lock);
>
>         if (drm_WARN_ON(&ptdev->base, csg_id >= ptdev->scheduler->csg_slot_count))
>                 return;
> @@ -1842,7 +1917,7 @@ static void sched_process_idle_event_locked(struct panthor_device *ptdev)
>  {
>         struct panthor_fw_global_iface *glb_iface = panthor_fw_get_glb_iface(ptdev);
>
> -       lockdep_assert_held(&ptdev->scheduler->lock);
> +       lockdep_assert_held(&ptdev->scheduler->events_lock);
>
>         /* Acknowledge the idle event and schedule a tick. */
>         panthor_fw_update_reqs(glb_iface, req, glb_iface->output->ack, GLB_IDLE);
> @@ -1858,7 +1933,7 @@ static void sched_process_global_irq_locked(struct panthor_device *ptdev)
>         struct panthor_fw_global_iface *glb_iface = panthor_fw_get_glb_iface(ptdev);
>         u32 req, ack, evts;
>
> -       lockdep_assert_held(&ptdev->scheduler->lock);
> +       lockdep_assert_held(&ptdev->scheduler->events_lock);
>
>         req = READ_ONCE(glb_iface->input->req);
>         ack = READ_ONCE(glb_iface->output->ack);
> @@ -1868,30 +1943,6 @@ static void sched_process_global_irq_locked(struct panthor_device *ptdev)
>                 sched_process_idle_event_locked(ptdev);
>  }
>
> -static void process_fw_events_work(struct work_struct *work)
> -{
> -       struct panthor_scheduler *sched = container_of(work, struct panthor_scheduler,
> -                                                     fw_events_work);
> -       u32 events = atomic_xchg(&sched->fw_events, 0);
> -       struct panthor_device *ptdev = sched->ptdev;
> -
> -       mutex_lock(&sched->lock);
> -
> -       if (events & JOB_INT_GLOBAL_IF) {
> -               sched_process_global_irq_locked(ptdev);
> -               events &= ~JOB_INT_GLOBAL_IF;
> -       }
> -
> -       while (events) {
> -               u32 csg_id = ffs(events) - 1;
> -
> -               sched_process_csg_irq_locked(ptdev, csg_id);
> -               events &= ~BIT(csg_id);
> -       }
> -
> -       mutex_unlock(&sched->lock);
> -}
> -
>  /**
>   * panthor_sched_report_fw_events() - Report FW events to the scheduler.
>   * @ptdev: Device.
> @@ -1902,8 +1953,19 @@ void panthor_sched_report_fw_events(struct panthor_device *ptdev, u32 events)
This can be renamed to panthor_sched_handle_fw_events.

>         if (!ptdev->scheduler)
>                 return;
>
> -       atomic_or(events, &ptdev->scheduler->fw_events);
> -       sched_queue_work(ptdev->scheduler, fw_events);
> +       guard(spinlock_irqsave)(&ptdev->scheduler->events_lock);
> +
> +       if (events & JOB_INT_GLOBAL_IF) {
> +               sched_process_global_irq_locked(ptdev);
> +               events &= ~JOB_INT_GLOBAL_IF;
> +       }
> +
> +       while (events) {
> +               u32 csg_id = ffs(events) - 1;
> +
> +               sched_process_csg_irq_locked(ptdev, csg_id);
> +               events &= ~BIT(csg_id);
> +       }
This handles all fw events in the irq context. Are there concerns that
it may take too long? I might be wrong, but it seems possible to
handle only CSG_SYNC_UPDATE and defer the rest as before.

>  }
>
>  static const char *fence_get_driver_name(struct dma_fence *fence)
> @@ -2136,7 +2198,9 @@ tick_ctx_init(struct panthor_scheduler *sched,
>                  * CSG IRQs, so we can flag the faulty queue.
>                  */
>                 if (panthor_vm_has_unhandled_faults(group->vm)) {
> -                       sched_process_csg_irq_locked(ptdev, i);
> +                       scoped_guard(spinlock_irqsave, &sched->events_lock) {
> +                               sched_process_csg_irq_locked(ptdev, i);
> +                       }
>
>                         /* No fatal fault reported, flag all queues as faulty. */
>                         if (!group->fatal_queues)
> @@ -2183,13 +2247,13 @@ group_term_post_processing(struct panthor_group *group)
>                 if (!queue)
>                         continue;
>
> -               spin_lock(&queue->fence_ctx.lock);
> -               list_for_each_entry_safe(job, tmp, &queue->fence_ctx.in_flight_jobs, node) {
> -                       list_move_tail(&job->node, &faulty_jobs);
> -                       dma_fence_set_error(job->done_fence, err);
> -                       dma_fence_signal_locked(job->done_fence);
> +               scoped_guard(spinlock_irqsave, &queue->fence_ctx.lock) {
> +                       list_for_each_entry_safe(job, tmp, &queue->fence_ctx.in_flight_jobs, node) {
> +                               list_move_tail(&job->node, &faulty_jobs);
> +                               dma_fence_set_error(job->done_fence, err);
> +                               dma_fence_signal_locked(job->done_fence);
> +                       }
>                 }
> -               spin_unlock(&queue->fence_ctx.lock);
>
>                 /* Manually update the syncobj seqno to unblock waiters. */
>                 syncobj = group->syncobjs->kmap + (i * sizeof(*syncobj));
> @@ -2336,8 +2400,10 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
>                          * any pending interrupts before we start the new
>                          * group.
>                          */
> -                       if (group->csg_id >= 0)
> +                       if (group->csg_id >= 0) {
> +                               guard(spinlock_irqsave)(&sched->events_lock);
>                                 sched_process_csg_irq_locked(ptdev, group->csg_id);
> +                       }
>
>                         group_unbind_locked(group);
>                 }
> @@ -2902,10 +2968,12 @@ void panthor_sched_suspend(struct panthor_device *ptdev)
>                         u32 csg_id = ffs(slot_mask) - 1;
>                         struct panthor_csg_slot *csg_slot = &sched->csg_slots[csg_id];
>
> -                       if (flush_caches_failed)
> +                       if (flush_caches_failed) {
>                                 csg_slot->group->state = PANTHOR_CS_GROUP_TERMINATED;
> -                       else
> +                       } else {
> +                               guard(spinlock_irqsave)(&sched->events_lock);
>                                 csg_slot_sync_update_locked(ptdev, csg_id);
> +                       }
>
>                         slot_mask &= ~BIT(csg_id);
>                 }
> @@ -2920,8 +2988,10 @@ void panthor_sched_suspend(struct panthor_device *ptdev)
>
>                 group_get(group);
>
> -               if (group->csg_id >= 0)
> +               if (group->csg_id >= 0) {
> +                       guard(spinlock_irqsave)(&sched->events_lock);
>                         sched_process_csg_irq_locked(ptdev, group->csg_id);
> +               }
>
>                 group_unbind_locked(group);
>
> @@ -3005,22 +3075,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;
> @@ -3032,7 +3086,7 @@ void panthor_fdinfo_gather_group_samples(struct panthor_file *pfile)
>
>         xa_lock(&gpool->xa);
>         xa_for_each_marked(&gpool->xa, i, group, GROUP_REGISTERED) {
> -               guard(spinlock)(&group->fdinfo.lock);
> +               guard(spinlock_irqsave)(&group->fdinfo.lock);
>                 pfile->stats.cycles += group->fdinfo.data.cycles;
>                 pfile->stats.time += group->fdinfo.data.time;
>                 group->fdinfo.data.cycles = 0;
> @@ -3041,80 +3095,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();
> -       spin_lock(&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;
> -       }
> -       spin_unlock(&queue->fence_ctx.lock);
> -       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;
> @@ -3346,9 +3326,8 @@ queue_run_job(struct drm_sched_job *sched_job)
>         job->ringbuf.end = job->ringbuf.start + (instrs.count * sizeof(u64));
>
>         panthor_job_get(&job->base);
> -       spin_lock(&queue->fence_ctx.lock);
> -       list_add_tail(&job->node, &queue->fence_ctx.in_flight_jobs);
> -       spin_unlock(&queue->fence_ctx.lock);
> +       scoped_guard(spinlock_irqsave, &queue->fence_ctx.lock)
> +               list_add_tail(&job->node, &queue->fence_ctx.in_flight_jobs);
>
>         /* Make sure the ring buffer is updated before the INSERT
>          * register.
> @@ -3683,7 +3662,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);
>
> @@ -4054,7 +4032,6 @@ void panthor_sched_unplug(struct panthor_device *ptdev)
>         struct panthor_scheduler *sched = ptdev->scheduler;
>
>         disable_delayed_work_sync(&sched->tick_work);
> -       disable_work_sync(&sched->fw_events_work);
>         disable_work_sync(&sched->sync_upd_work);
>
>         mutex_lock(&sched->lock);
> @@ -4139,7 +4116,8 @@ int panthor_sched_init(struct panthor_device *ptdev)
>         sched->tick_period = msecs_to_jiffies(10);
>         INIT_DELAYED_WORK(&sched->tick_work, tick_work);
>         INIT_WORK(&sched->sync_upd_work, sync_upd_work);
> -       INIT_WORK(&sched->fw_events_work, process_fw_events_work);
> +
> +       spin_lock_init(&sched->events_lock);
>
>         ret = drmm_mutex_init(&ptdev->base, &sched->lock);
>         if (ret)
>
> --
> 2.54.0
>

  reply	other threads:[~2026-05-12 21:04 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12 11:37 [PATCH v2 00/11] drm/panthor: Reduce dma_fence signalling latency Boris Brezillon
2026-05-12 11:37 ` [PATCH v2 01/11] drm/panthor: Make panthor_irq::state a non-atomic field Boris Brezillon
2026-05-12 18:40   ` Chia-I Wu
2026-05-16  3:32   ` Claude review: " Claude Code Review Bot
2026-05-12 11:37 ` [PATCH v2 02/11] drm/panthor: Move the register accessors before the IRQ helpers Boris Brezillon
2026-05-12 18:41   ` Chia-I Wu
2026-05-16  3:32   ` Claude review: " Claude Code Review Bot
2026-05-12 11:37 ` [PATCH v2 03/11] drm/panthor: Replace the panthor_irq macro machinery by inline helpers Boris Brezillon
2026-05-12 18:58   ` Chia-I Wu
2026-05-13  8:03     ` Boris Brezillon
2026-05-13 16:46       ` Chia-I Wu
2026-05-16  3:32   ` Claude review: " Claude Code Review Bot
2026-05-12 11:37 ` [PATCH v2 04/11] drm/panthor: Extend the IRQ logic to allow fast/hard IRQ handlers Boris Brezillon
2026-05-12 19:11   ` Chia-I Wu
2026-05-13  8:09     ` Boris Brezillon
2026-05-13 17:06       ` Chia-I Wu
2026-05-13 17:30         ` Boris Brezillon
2026-05-13 18:17           ` Chia-I Wu
2026-05-16  3:32   ` Claude review: " Claude Code Review Bot
2026-05-12 11:37 ` [PATCH v2 05/11] drm/panthor: Make panthor_fw_{update,toggle}_reqs() callable from IRQ context Boris Brezillon
2026-05-12 19:29   ` [PATCH v2 05/11] drm/panthor: Make panthor_fw_{update, toggle}_reqs() " Chia-I Wu
2026-05-16  3:32   ` Claude review: drm/panthor: Make panthor_fw_{update,toggle}_reqs() " Claude Code Review Bot
2026-05-12 11:37 ` [PATCH v2 06/11] drm/panthor: Prepare the scheduler logic for FW events in " Boris Brezillon
2026-05-12 21:04   ` Chia-I Wu [this message]
2026-05-13  8:29     ` Boris Brezillon
2026-05-13 17:47       ` Chia-I Wu
2026-05-16  3:32   ` Claude review: " Claude Code Review Bot
2026-05-12 11:37 ` [PATCH v2 07/11] drm/panthor: Automate CSG IRQ processing at group unbind time Boris Brezillon
2026-05-12 21:16   ` Chia-I Wu
2026-05-14 14:17   ` Steven Price
2026-05-16  3:32   ` Claude review: " Claude Code Review Bot
2026-05-12 11:37 ` [PATCH v2 08/11] drm/panthor: Automatically enable interrupts in panthor_fw_wait_acks() Boris Brezillon
2026-05-12 21:55   ` Chia-I Wu
2026-05-13  8:42     ` Boris Brezillon
2026-05-13 17:14       ` Chia-I Wu
2026-05-14 14:25   ` Steven Price
2026-05-16  3:32   ` Claude review: " Claude Code Review Bot
2026-05-12 11:37 ` [PATCH v2 09/11] drm/panthor: Process FW events in IRQ context Boris Brezillon
2026-05-12 22:05   ` Chia-I Wu
2026-05-12 22:09     ` Chia-I Wu
2026-05-13  8:44       ` Boris Brezillon
2026-05-14 15:23   ` Steven Price
2026-05-16  3:32   ` Claude review: " Claude Code Review Bot
2026-05-12 11:37 ` [PATCH v2 10/11] drm/panthor: Use the irqsave variant of spin_lock in panthor_gpu_irq_handler() Boris Brezillon
2026-05-14 15:26   ` Steven Price
2026-05-16  3:32   ` Claude review: " Claude Code Review Bot
2026-05-12 11:37 ` [PATCH v2 11/11] drm/panthor: Process GPU events in IRQ context Boris Brezillon
2026-05-12 11:50   ` Boris Brezillon
2026-05-12 22:40     ` Chia-I Wu
2026-05-13  8:54       ` Boris Brezillon
2026-05-13 18:07         ` Chia-I Wu
2026-05-16  3:32   ` Claude review: " Claude Code Review Bot
2026-05-16  3:32 ` Claude review: drm/panthor: Reduce dma_fence signalling latency Claude Code Review Bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPaKu7RjHvRAYZDehSF9R_8T-uTrC9-NfsAPSOX0n=-2phunpg@mail.gmail.com' \
    --to=olvaffe@gmail.com \
    --cc=airlied@gmail.com \
    --cc=boris.brezillon@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liviu.dudau@arm.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=steven.price@arm.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox