Re: [PATCH v12 12/15] drm/panfrost: Skip cache flush/invalidate when enabling perfcnt

[Steven Price <steven.price@arm.com>]

On 29/09/2026 04:44, Adrián Larumbe wrote:
> The GPU cache flush/invalidate operation is unnecessary. First off, the
> GPU doesn't read off the perfcnt sample buffer, only writes into it, so

I don't think this is entirely true. The GPU performance counter unit
only writes the counters that are enabled, counters that share a cache
line but are not enabled are not written by the performance counter
unit, but if the L2 contains that cache line then the write can hit in
the L2 and dirty the entire line including stale data where the
unwritten cache line is.

The upshot is that if the CPU has cleared a block of memory which the
GPU happens to have cached, then the "unused" counters may end up
showing the old data before the CPU cleared it (if they share a cache
line with an active counter).

I have to admit it's probably somewhat academic given that Panfrost
doesn't expose the ability to control which counters are enabled...

Is there a good reason for this patch (i.e. have you seen a performance
problem with doing the invalidate)? Otherwise I'd prefer we keep to the
safe route rather than trying to over optimise cache maintenance.

Obviously in the fully coherent case the invalidate could be skipped (as
in the next patch).

Thanks,
Steve

> an invalidate doesn't make a difference. Then flushing GPU caches after
> each sample has been written is enough for the CPU to see updated values.
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
>  drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index f71534e741b6..ffc77121070e 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> @@ -124,21 +124,10 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
>  	panfrost_gem_internal_set_label(&bo->base, "Perfcnt sample buffer");
>  
>  	/*
> -	 * Invalidate the cache and clear the counters to start from a fresh
> -	 * state.
> +	 * Clear the counters to start from a fresh state.
>  	 */
> -	reinit_completion(&pfdev->perfcnt->dump_comp);
> -	gpu_write(pfdev, GPU_INT_CLEAR,
> -		  GPU_IRQ_CLEAN_CACHES_COMPLETED |
> -		  GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> +	gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
>  	gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_CLEAR);
> -	gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_INV_CACHES);
> -	ret = wait_for_completion_timeout(&pfdev->perfcnt->dump_comp,
> -					  msecs_to_jiffies(1000));
> -	if (!ret) {
> -		ret = -ETIMEDOUT;
> -		goto err_vunmap;
> -	}
>  
>  	ret = panfrost_mmu_as_get(pfdev, perfcnt->mapping->mmu);
>  	if (ret < 0)
>