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

[Boris Brezillon]

On Mon, 5 Oct 2026 17:05:14 +0100
Steven Price <steven.price@xxxxxxx> wrote:

> On 05/10/2026 16:37, Boris Brezillon wrote:
> > On Mon, 5 Oct 2026 16:06:02 +0100
> > Steven Price <steven.price@xxxxxxx> wrote:
> >   
> >> On 05/10/2026 09:38, Boris Brezillon wrote:  
> >>> On Fri, 2 Oct 2026 16:14:38 +0100
> >>> Steven Price <steven.price@xxxxxxx> wrote:
> >>>     
> >>>> 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.    
> >>>
> >>> I don't think this can happen though, because _enable_locked() is
> >>> creating a BO (and its GPU mapping) just before enabling the perfcnt
> >>> block, meaning the buffer is known to have no dirty cacheline pointing
> >>> to it until the first dump happens. And we do flush and invalidate GPU
> >>> caches after each dump, so again, we should be covered.    
> >>
> >> The situation isn't actually a dirty cache line at the start, but a
> >> stale one. We start off with the memory matching a clean line in the
> >> GPU's cache. But because we don't have coherency the clean line can stay
> >> even if it's inconsistent with everything else.
> >>
> >> CPU		|	GPU		| Memory
> >> ----------------+-----------------------+--------------------
> >>                 | clean line            | matches GPU cache
> >> ----------------+-----------------------+--------------------
> >>    CPU allocates new buffer and writes zeros
> >> ----------------+-----------------------+--------------------
> >> Dirty cache line| stale clean line      | unknown (cache line
> >>                 |                       | might be evicted)
> >> ----------------+-----------------------+--------------------
> >>    CPU cleans its own cache to memory
> >> ----------------+-----------------------+--------------------
> >> Potential clean | stale clean line      | Matches CPU
> >> cache line      |                       |
> >> ----------------+-----------------------+--------------------
> >>    Start dump without invaliding GPU
> >> ----------------+-----------------------+--------------------
> >> Potential clean | GPU writes data, and  | Unknown
> >> cache line      | hits in the clean line|
> >>                 | even though it's stale|
> >> ----------------+-----------------------+--------------------
> >>    CPU flushes the GPU's cache and invalidates it's own
> >> ----------------+-----------------------+--------------------
> >> No-cache line   | writes out clean line | Matches GPU
> >>
> >>
> >> Of course for the GPU to have ended up with that stale clean cache line
> >> means that the physical memory was previously used for something else on
> >> the GPU, so the newly allocated BO has to reuse memory from a previous
> >> BO that the GPU has accessed. And it's all "unlikely" due to the small
> >> size of the GPU's cache.  
> > 
> > Hm, I'd say it's actually impossible because every unmap operation is
> > followed by a flush+inval of the GPU L2 and LSC, so for this stale
> > clean line to exist on the GPU side when a physical page is GPU-mapped
> > again, it would take a bug in the unmap logic or in the MMU HW, I
> > think. Am I missing something?  
> 
> Ah, yes that's true :) Although there's no need for us to do an
> invalidate on the unmap path...

My bad, I'm confusing Panfrost and Panthor here. There's indeed no L2
flush+inval on unmap in Panfrost. We do have an L2 flush+inval on map
(FLUSH_PT) that should cover this case though.