Re: [PATCH v13 13/15] drm/panfrost: Avoid cache flush after perfcnt sample in fully coherent systems

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm: race condition in panfrost_gpu_irq_handler causes lost perfcnt interrupts
--

commit 54ca68cf4bae3920aff016a6d2b492d879450341
Author: Adrián Larumbe <adrian.larumbe@collabora.com>

drm/panfrost: Avoid cache flush after perfcnt sample in fully coherent systems

This commit skips the GPU cache flush after a performance counter sample
becomes available when the GPU and CPU are fully coherent. It does this by
completing the dump immediately rather than issuing a cache clean command.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index ffc77121070e2..b3f71d7fd82a9 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> @@ -52,7 +52,10 @@ void panfrost_perfcnt_clean_cache_done(struct panfrost_device *pfdev)
>  
>  void panfrost_perfcnt_sample_done(struct panfrost_device *pfdev)
>  {
> -	gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES);
> +	if (pfdev->features.selected_coherency != COHERENCY_ACE)
> +		gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES);
> +	else
> +		complete(&pfdev->perfcnt->dump_comp);

[Severity: Medium]
Since complete() is now called before the IRQ handler clears the interrupt,
could this cause a race condition where a subsequent perfcnt dump request
times out?

If the waiting process wakes up and immediately issues another perfcnt dump
request on another CPU, it might write GPU_CMD_PERFCNT_SAMPLE and trigger a
new GPU_IRQ_PERFCNT_SAMPLE_COMPLETED interrupt before the original IRQ
handler finishes.

The original IRQ handler in panfrost_gpu_irq_handler() finishes by writing
the previously read state back:

drivers/gpu/drm/panfrost/panfrost_gpu.c:panfrost_gpu_irq_handler() {
    ...
    state = gpu_read(pfdev, GPU_INT_STAT);
    ...
    if (state & GPU_IRQ_PERFCNT_SAMPLE_COMPLETED)
        panfrost_perfcnt_sample_done(pfdev);
    ...
    gpu_write(pfdev, GPU_INT_CLEAR, state);
    return IRQ_HANDLED;
}

Could this gpu_write(pfdev, GPU_INT_CLEAR, state) clear the newly asserted
interrupt, causing the second dump request to hang and return -ETIMEDOUT to
userspace?

>  }
>  
>  static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-claude-fixes-v13-0-d6a493185860@collabora.com?part=13