On Mon, 5 Oct 2026 17:05:14 +0100 Steven Price <[email protected]> wrote:
> On 05/10/2026 16:37, Boris Brezillon wrote: > > On Mon, 5 Oct 2026 16:06:02 +0100 > > Steven Price <[email protected]> wrote: > > > >> On 05/10/2026 09:38, Boris Brezillon wrote: > >>> On Fri, 2 Oct 2026 16:14:38 +0100 > >>> Steven Price <[email protected]> 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.
