[Mesa-dev] [PATCH v2 02/10] i965: Track initial CPU domain for mappings

[Chris Wilson]

Quoting Kenneth Graunke (2017-06-20 00:33:35)
> On Monday, June 19, 2017 3:55:01 AM PDT Chris Wilson wrote:
> > If we need to force a cache domain transition (e.g. a buffer was in the
> > CPU domain and we want to access it via WC) then we need to trigger a
> > clflush. This overrides the use of MAP_ASYNC as we call into the kernel
> > to change domains on the whole object.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Kenneth Graunke <kenneth at whitecape.org>
> > Cc: Matt Turner <mattst88 at gmail.com>
> > ---
> >  src/mesa/drivers/dri/i965/brw_bufmgr.c        | 14 ++++++++++----
> >  src/mesa/drivers/dri/i965/brw_bufmgr.h        | 10 ++++++++++
> >  src/mesa/drivers/dri/i965/intel_batchbuffer.c |  1 +
> >  3 files changed, 21 insertions(+), 4 deletions(-)
> 
> This patch doesn't work.  For example, on Broxton,
> 
> $ piglit/bin/vbo-map-unsync -auto -fbo

Ah, what's happening here is that we have

        /* Map second half of vertex buffer */
        verts_map = glMapBufferRange(GL_ARRAY_BUFFER_ARB,
                                     sizeof(verts), sizeof(verts),
                                     GL_MAP_WRITE_BIT |
                                     GL_MAP_INVALIDATE_RANGE_BIT |
                                     GL_MAP_UNSYNCHRONIZED_BIT);
        elems_map = glMapBufferARB(GL_ELEMENT_ARRAY_BUFFER_ARB,
                                   GL_WRITE_ONLY_ARB

(following a similar invocation for the first half)

We see the ASYNC flag for verts_map and see that we are still streaming
into the current CPU bo and so return the cpu mapping from the first
half. However, the elems_map overwrites the previous data causing a
batch flush which invalidates the correctness of using the cpu map.

And stalls!

diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index bb2f0d9..8afcde1 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -2814,7 +2814,7 @@ get_map_buffer_access_flags(struct gl_context *ctx, GLenum access,
       *flags = GL_MAP_READ_BIT;
       return _mesa_is_desktop_gl(ctx);
    case GL_WRITE_ONLY_ARB:
-      *flags = GL_MAP_WRITE_BIT;
+      *flags = GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_BUFFER_BIT;
       return true;
    case GL_READ_WRITE_ARB:
       *flags = GL_MAP_READ_BIT | GL_MAP_WRITE_BIT;

is sufficient to prevent the implicit flush and stall (and failure) but
is not a very general solution.

Since we cannot prevent implicit flushes, I think we have to simply
forbid ASYNC access to the cpu mapping (when not cache coherent).
-Chris