On Fri, 1 Oct 2021 16:13:42 +0100 Steven Price steven.price@arm.com wrote:
On 01/10/2021 15:34, Boris Brezillon wrote:
This lets the driver reduce shareability domain of the MMU mapping, which can in turn reduce access time and save power on cache-coherent systems.
Signed-off-by: Boris Brezillon boris.brezillon@collabora.com
This seems reasonable to me - it matches the kbase BASE_MEM_COHERENT_SYSTEM (only backwards obviously) and it worked reasonably well for the blob.
But I'm wondering if we need to do anything special to deal with the fact we will now have some non-coherent mappings on an otherwise coherent device.
There are certainly some oddities around how these buffers will be mapped into user space if requested, e.g. panfrost_gem_create_object() sets 'map_wc' based on pfdev->coherent which is arguably wrong for GPUONLY. So there are two things we could consider:
a) Actually prevent user space mapping GPUONLY flagged buffers. Which matches the intention of the name.
I intended to do that, just forgot to add wrappers around drm_gem_shmem_{mmap,vmap}() to forbid CPU-mappings on gpuonly buffers.
b) Attempt to provide user space with the tools to safely interact with the buffers (this is the kbase approach). This does have the benefit of allowing *mostly* GPU access. An example here is the tiler heap where the CPU could zero out as necessary but mostly the GPU has ownership and the CPU never reads the contents. GPUONLY/DEVONLY might not be the best name in that case.
Uh, right, I forgot we had to zero the tiler heap on Midgard (most of the time done with a WRITE_VALUE job, but there's an exception on some old Midgard GPUs IIRC).