[PATCH] drm/gem_shmem: Use a writecombine mapping for ->vaddr

[Daniel Vetter]

On Mon, Jun 3, 2019 at 5:43 PM Eric Anholt <eric at anholt.net> wrote:
>
> Daniel Vetter <daniel at ffwll.ch> writes:
>
> > On Fri, May 31, 2019 at 08:46:58AM -0700, Eric Anholt wrote:
> >> Boris Brezillon <boris.brezillon at collabora.com> writes:
> >>
> >> > Right now, the BO is mapped as a cached region when ->vmap() is called
> >> > and the underlying object is not a dmabuf.
> >> > Doing that makes cache management a bit more complicated (you'd need
> >> > to call dma_map/unmap_sg() on the ->sgt field everytime the BO is about
> >> > to be passed to the GPU/CPU), so let's map the BO with writecombine
> >> > attributes instead (as done in most drivers).
> >> >
> >> > Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> >> > ---
> >> > Found this issue while working on panfrost perfcnt where the GPU dumps
> >> > perf counter values in memory and the CPU reads them back in
> >> > kernel-space. This patch seems to solve the unpredictable behavior I
> >> > had.
> >> >
> >> > I can also go for the other option (call dma_map/unmap/_sg() when
> >> > needed) if you think that's more appropriate.
> >>
> >> writecombined was the intent, and this makes kernel vmap match the
> >> userspace mmap path.
> >
> > Since I missed that obviously: Where do the shmem helpers set write
> > combined mode for userspace mmap?
>
> That was the trick when I asked the question, too.  drm_gem.c is what
> sets WC by default.

TIL. And looks like this has been the case forever, it laned in

commit a2c0a97b784f837300f7b0869c82ab712c600952
Author: Jesse Barnes <jbarnes at virtuousgeek.org>
Date:   Wed Nov 5 10:31:53 2008 -0800

    drm: GEM mmap support

I'll retract my concern, making this wc everywhere is the right thing to do.

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch