Re: [PATCH 09/10] drm/panthor: Process FW events in IRQ context

Steven Price
2026-05-01T14:38:56Z

On 29/04/2026 10:38, Boris Brezillon wrote:
> Now that everything is set to allow processing FW events in IRQ context,
> go for it. This should reduce the dma_fence signaling latency.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Another AI-found locking bug...

With this change there's a callpath:

panthor_job_irq_raw_handler()
panthor_job_irq_handler()
panthor_sched_report_fw_events()
sched_process_csg_irq_locked()
if (csg_events & CSG_SYNC_UPDATE)
csg_slot_sync_update_locked()
group_check_job_completion()
queue_check_job_completion()
if (job->profiling.mask)
update_fdinfo_stats()
spin_lock(&group->fdinfo.lock)

However group->fdinfo.lock is also held in process context via:

panthor_gpu_show_fdinfo()
panthor_fdinfo_gather_group_samples()
guard(spinlock)(&group->fdinfo.lock);

So panthor_fdinfo_gather_group_samples() will need to use
spinlock_irqsave to be safe.

Thanks,
Steve

> ---
> drivers/gpu/drm/panthor/panthor_fw.c | 33 +++++++++++++++++++++++++++++++--
> 1 file changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index f5e0ceca4130..05c632913359 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -1087,9 +1087,38 @@ static void panthor_job_irq_handler(struct panthor_irq *pirq, u32 status)
> }
> }
>
> +static irqreturn_t panthor_job_irq_raw_handler(int irq, void *data)
> +{
> + struct panthor_irq *pirq = data;
> +
> + if (!gpu_read(pirq->iomem, INT_STAT))
> + return IRQ_NONE;
> +
> + scoped_guard(spinlock_irqsave, &pirq->mask_lock) {
> + if (pirq->state != PANTHOR_IRQ_STATE_ACTIVE)
> + return IRQ_NONE;
> +
> + pirq->state = PANTHOR_IRQ_STATE_PROCESSING;
> + }
> +
> + panthor_job_irq_handler(pirq, gpu_read(pirq->iomem, INT_RAWSTAT));
> +
> + scoped_guard(spinlock_irqsave, &pirq->mask_lock) {
> + if (pirq->state == PANTHOR_IRQ_STATE_PROCESSING)
> + pirq->state = PANTHOR_IRQ_STATE_ACTIVE;
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> static irqreturn_t panthor_job_irq_threaded_handler(int irq, void *data)
> {
> - return panthor_irq_default_threaded_handler(data, panthor_job_irq_handler);
> + struct panthor_irq *pirq = data;
> +
> + /* We never return IRQ_WAKE_THREAD, so we're not supposed to be called. */
> + drm_WARN_ON_ONCE(&pirq->ptdev->base,
> + "threaded IRQ handler should never be called.");
> + return IRQ_NONE;
> }
>
> static int panthor_fw_start(struct panthor_device *ptdev)
> @@ -1489,7 +1518,7 @@ int panthor_fw_init(struct panthor_device *ptdev)
>
> ret = panthor_irq_request(ptdev, &fw->irq, irq, 0,
> ptdev->iomem + JOB_INT_BASE, "job",
> - panthor_irq_default_raw_handler,
> + panthor_job_irq_raw_handler,
> panthor_job_irq_threaded_handler);
> if (ret) {
> drm_err(&ptdev->base, "failed to request job irq");
>