Re: [PATCH v9 13/16] 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] Waking up the user thread before clearing the GPU interrupt introduces a race condition that can clear the interrupt of a subsequent performance counter dump, causing it to time out.
--

commit f36520c5a42efb4216abf8dab5756d0a6134f2a4
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 when the system is fully coherent (ACE).
Instead of sending a cache clean command, it immediately signals completion
of the performance counter sample.

> 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

[Severity: Medium]
Does waking up the user thread before clearing the GPU interrupt introduce
a race condition that can drop subsequent sample interrupts?

If panfrost_perfcnt_sample_done() calls complete() and wakes up the user
thread, the thread can return to userspace and immediately issue a new
PERFCNT_DUMP ioctl, starting a new hardware sample.

If the IRQ handler on the original CPU is delayed or preempted (e.g., by
a hypervisor or NMI) before it writes to the clear register:

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

Could it write the original state (which has the completion bit set) to
GPU_INT_CLEAR after the new sample has already started? This would clear
the interrupt for the newly submitted sample, causing the new dump's wait
to time out with -ETIMEDOUT.

Prior to this patch, the CLEAN_CACHES step prevented this race because the
delayed completion meant the interrupt clear could not hit a new sample.

> +		complete(&pfdev->perfcnt->dump_comp);
>  }
>  
>  static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912-claude-fixes-v9-0-e588feaa61ef@collabora.com?part=13