[Mesa-dev] [PATCH 03/53] i965: Make sure we don't use CPU maps for the scanout buffer.

[Chris Wilson]

On Tue, Apr 04, 2017 at 05:09:53PM -0700, Kenneth Graunke wrote:
> Using an incoherent CPU map on the active scanout buffer is really
> sketchy - we may need extra flushing via GEM_SW_FINISH, or using
> drmModeDirtyFB() and kernel commit a6a7cc4b7db6d (4.10+).
> 
> Chris suggests "never ever do that", which seems like a wise plan!
> 
> intel_miptree_map_raw() uses CPU maps on linear buffers.
> 
> Having a linear scanout buffer should be really rare, and mapping the
> front buffer should be similarly rare.  Together, it should basically
> never happen.  But, in case it does somehow...make sure that mapping
> the scanout buffer always goes through an uncached GTT map.
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 467ada5079b..272eb49867e 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -2460,7 +2460,7 @@ intel_miptree_map_raw(struct brw_context *brw, struct intel_mipmap_tree *mt)
>     if (drm_intel_bo_references(brw->batch.bo, bo))
>        intel_batchbuffer_flush(brw);
>  
> -   if (mt->tiling != I915_TILING_NONE)
> +   if (mt->tiling != I915_TILING_NONE || mt->is_scanout)
>        brw_bo_map_gtt(brw, bo, "miptree");
>     else
>        brw_bo_map(brw, bo, true, "miptree");

If bo map remains the CPU only variant, can you put an assert in it that
it never sees a a scanout? Hmm. I guess you don't track scanout buffers
at that level yet?

A good starting point might be a comment instead, brw_bo_map:

/*
 * brw_bo_map() uses a WB mmaping of the buffer's backing storage. It
 * will utilize the CPU cache even if the buffer is incoherent with the
 * GPU (i.e. any writes will be stored in the cache and not flushed to
 * memory and so will be invisible to the GPU or display engine). This
 * is the majority of buffers on a !llc machine, but even on a llc
 * almost all scanouts are incoherent with the CPU. A WB write into the
 * backing storage of the current scanout will not be immediately
 * visible on the screen. The transfer from cache to screen is slow and
 * indeterministic causing visible glitching on the screen. Never use
 * this WB mapping for writes to an active scanout (reads are fine, so
 * long as cache consistency is maintained).
 */

Another caveat it will be extremely useful if we can drop the guarantee
that all backing storage (bo) will be CPU accessible. A wayland
compositor + gbm could allocate all framebuffers from stolen -- it's
an issue of protocol, both parties must be aware that certain bo are not
first class.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre