[Mesa-dev] [SQUASH] i965: Zero out query buffer done bit

[Kenneth Graunke]

Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
 src/mesa/drivers/dri/i965/brw_context.h   |  2 ++
 src/mesa/drivers/dri/i965/brw_queryobj.c  |  4 ++--
 src/mesa/drivers/dri/i965/gen6_queryobj.c | 30 ++++++++++++++++++++++++++----
 3 files changed, 30 insertions(+), 6 deletions(-)

Jordan,

I'm pretty sure you need to zero out the "Done?" bit in query->bo.  The BOs
we allocate aren't guaranteed to be zero...so we may end up copying garbage
to the MI_PREDICATE registers, rather than 0/1 as we wanted.

Here's some code that Eric wrote a long time ago to do that; I've updated
it a couple times now.  Feel free to squash it into your patch.

With all of my feedback addressed, you get a Reviewed-by :)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index b75eabd..3805b56 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1429,6 +1429,8 @@ void brw_query_counter(struct gl_context *ctx, struct gl_query_object *q);
 bool brw_is_query_pipelined(struct brw_query_object *query);
 
 /** gen6_queryobj.c */
+void brw_allocate_query_bo(struct brw_context *brw,
+                           struct brw_query_object *query);
 void gen6_init_queryobj_functions(struct dd_function_table *functions);
 void brw_write_timestamp(struct brw_context *brw, drm_intel_bo *bo, int idx);
 void brw_write_depth_count(struct brw_context *brw, drm_intel_bo *bo, int idx);
diff --git a/src/mesa/drivers/dri/i965/brw_queryobj.c b/src/mesa/drivers/dri/i965/brw_queryobj.c
index 81ee5ea..b7c2726 100644
--- a/src/mesa/drivers/dri/i965/brw_queryobj.c
+++ b/src/mesa/drivers/dri/i965/brw_queryobj.c
@@ -470,8 +470,8 @@ brw_query_counter(struct gl_context *ctx, struct gl_query_object *q)
 
    assert(q->Target == GL_TIMESTAMP);
 
-   drm_intel_bo_unreference(query->bo);
-   query->bo = drm_intel_bo_alloc(brw->bufmgr, "timestamp query", 4096, 4096);
+   brw_allocate_query_bo(brw, query);
+
    brw_write_timestamp(brw, query->bo, 0);
 
    query->flushed = false;
diff --git a/src/mesa/drivers/dri/i965/gen6_queryobj.c b/src/mesa/drivers/dri/i965/gen6_queryobj.c
index f95c9fc..04f58f6 100644
--- a/src/mesa/drivers/dri/i965/gen6_queryobj.c
+++ b/src/mesa/drivers/dri/i965/gen6_queryobj.c
@@ -40,6 +40,8 @@
 #include "intel_buffer_objects.h"
 #include "intel_reg.h"
 
+#define QUERY_DONE_OFFSET (2 * sizeof(uint64_t))
+
 static inline void
 set_query_availability(struct brw_context *brw, struct brw_query_object *query,
                        bool available)
@@ -51,7 +53,7 @@ set_query_availability(struct brw_context *brw, struct brw_query_object *query,
        brw_is_query_pipelined(query)) {
       brw_emit_pipe_control_write(brw,
                                   PIPE_CONTROL_WRITE_IMMEDIATE,
-                                  query->bo, 2 * sizeof(uint64_t),
+                                  query->bo, QUERY_DONE_OFFSET,
                                   available, 0);
    }
 }
@@ -140,6 +142,28 @@ emit_pipeline_stat(struct brw_context *brw, drm_intel_bo *bo,
    brw_store_register_mem64(brw, bo, reg, idx * sizeof(uint64_t));
 }
 
+void
+brw_allocate_query_bo(struct brw_context *brw, struct brw_query_object *query)
+{
+   /* Since we're starting a new query, we need to throw away previous
+    * uncollected results if there are any.
+    */
+   drm_intel_bo_unreference(query->bo);
+
+   query->bo = drm_intel_bo_alloc(brw->bufmgr, "query results", 4096, 4096);
+
+   if (ctx->Extensions.ARB_query_buffer_object) {
+      /* Clear the "done" field.
+       *
+       * We can safely use _unsynchronized here (and possibly avoid bothering
+       * the kernel for this mapping at all on a cached buffer on an LLC
+       * system), because drm_intel_bo_alloc() guarantees you an idle BO.
+       */
+      drm_intel_gem_bo_map_unsynchronized(query->bo);
+      *(uint32_t *)(query->bo->virtual + QUERY_DONE_OFFSET) = 0;
+      drm_intel_bo_unmap(query->bo);
+   }
+}
 
 /**
  * Wait on the query object's BO and calculate the final result.
@@ -256,9 +280,7 @@ gen6_begin_query(struct gl_context *ctx, struct gl_query_object *q)
    struct brw_context *brw = brw_context(ctx);
    struct brw_query_object *query = (struct brw_query_object *)q;
 
-   /* Since we're starting a new query, we need to throw away old results. */
-   drm_intel_bo_unreference(query->bo);
-   query->bo = drm_intel_bo_alloc(brw->bufmgr, "query results", 4096, 4096);
+   brw_allocate_query_bo(brw, query);
 
    /* For ARB_query_buffer_object: The result is not available */
    set_query_availability(brw, query, false);
-- 
2.8.0