On 01.09.2026 15:32, Boris Brezillon wrote: > On Fri, 28 Aug 2026 21:56:53 +0100 > Adrián Larumbe <[email protected]> wrote: > > > The GPU cache flush/invalidate operation is unnecessary, because at present > > all counters are always enabled for all sessions, and there are no plans to > > expand the uAPI so as to let UM select specific counter groups. > > > > If the above was ever implemented, then different counter groups being > > selected between sessions might leave stale data in the GPU caches, so > > the flush/invalidate would have to be reintroduced. > > This explanation confuses me. To me, the reason we don't need a > cache-flush has nothing to do with the fact we always enable all > counters, it's just that, either the perfcnt buffer has been freshly > allocated/mapped and the cache has no entry pointing to it, or it's > been written already, and the flush should have happened after the > dump. For the case where the enable happens after a RESET (which you > introduce in one of the remaining commits, I guess), there's no flush > needed either, because the RESET clears all the caches.
This is something I wrote after a chat with Steven about whether removing this flush/invalidate was fine. If memory serves me right, he expressed his concern that flushing GPU caches after a sample might write disabled counter data into system memory, but in that case UM should know better, and I guess reading off disabled counter data is seen as undefined behaviour in the TRM. > > > > Signed-off-by: Adrián Larumbe <[email protected]> > > --- > > 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) > > Adrian Larumbe
