[Mesa-dev] [PATCH 5/5] i965/query: Cache whether the batch references the query BO.

[Kenneth Graunke]

Chris Wilson noted that repeated calls to CheckQuery() would call
drm_intel_bo_references(brw->batch.bo, query->bo) on each invocation,
which is expensive.  Once we've flushed, we know that future batches
won't reference query->bo, so there's no point in asking more than once.

This patch adds a brw_query_object::flushed flag, which is a
conservative estimate of whether the batch has been flushed.

On the first call to CheckQuery() or WaitQuery(), we check if the
batch references query->bo.  If not, it must have been flushed for
some reason (such as being full).  We record that it was flushed.
If it does reference query->bo, we explicitly flush, and record that
we did so.

Any subsequent checks will simply see that query->flushed is set,
and skip the drm_intel_bo_references() call.

Inspired by a patch from Chris Wilson.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86969
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Eero Tamminen <eero.t.tamminen at intel.com>
---
 src/mesa/drivers/dri/i965/brw_context.h   |  3 +++
 src/mesa/drivers/dri/i965/gen6_queryobj.c | 27 +++++++++++++++++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

ickle's patch in bug #86969 eliminated all calls to drm_intel_bo_references
in the query object code.  One consequence is that the first call to
CheckQuery or WaitQuery would always flush the batch, even if it wasn't
necessary (say, the batch had been flushed).

Mine is a bit different - it still uses bo_references on the first call to
CheckQuery/WaitQuery - but then records that the batch has been flushed, so
we never have to call it again.

Either approach should greatly reduce the overhead from repeatedly calling
drm_intel_bo_references.

Eero, would you be able to test this and see if it solves the issue you
were seeing?  If so, could you provide some simple statistics showing the
benefit?  Thanks!

This is available as the 'query-references' branch in my tree (~kwg/mesa).

diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index 1b8f0bb..a63c483 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -831,6 +831,9 @@ struct brw_query_object {
 
    /** Last index in bo with query data for this object. */
    int last_index;
+
+   /** True if we know the batch has been flushed since we ended the query. */
+   bool flushed;
 };
 
 struct intel_sync_object {
diff --git a/src/mesa/drivers/dri/i965/gen6_queryobj.c b/src/mesa/drivers/dri/i965/gen6_queryobj.c
index 0d51390..de71bb5 100644
--- a/src/mesa/drivers/dri/i965/gen6_queryobj.c
+++ b/src/mesa/drivers/dri/i965/gen6_queryobj.c
@@ -281,6 +281,27 @@ gen6_end_query(struct gl_context *ctx, struct gl_query_object *q)
    default:
       unreachable("Unrecognized query target in brw_end_query()");
    }
+
+   /* The current batch contains the commands to handle EndQuery(),
+    * but they won't actually execute until it is flushed.
+    */
+   query->flushed = false;
+}
+
+/**
+ * Flush the batch if it still references the query object BO.
+ */
+static void
+flush_batch_if_needed(struct brw_context *brw, struct brw_query_object *query)
+{
+   /* If the batch doesn't reference the BO, it must have been flushed
+    * (for example, due to being full).  Record that it's been flushed.
+    */
+   query->flushed = query->flushed ||
+      !drm_intel_bo_references(brw->batch.bo, query->bo);
+
+   if (!query->flushed)
+      intel_batchbuffer_flush(brw);
 }
 
 /**
@@ -298,8 +319,7 @@ static void gen6_wait_query(struct gl_context *ctx, struct gl_query_object *q)
     * still contributing to it, flush it now to finish that work so the
     * result will become available (eventually).
     */
-   if (drm_intel_bo_references(brw->batch.bo, query->bo))
-      intel_batchbuffer_flush(brw);
+   flush_batch_if_needed(brw, query);
 
    gen6_queryobj_get_results(ctx, query);
 }
@@ -328,8 +348,7 @@ static void gen6_check_query(struct gl_context *ctx, struct gl_query_object *q)
     *      not ready yet on the first time it is queried.  This ensures that
     *      the async query will return true in finite time.
     */
-   if (drm_intel_bo_references(brw->batch.bo, query->bo))
-      intel_batchbuffer_flush(brw);
+   flush_batch_if_needed(brw, query);
 
    if (!drm_intel_bo_busy(query->bo)) {
       gen6_queryobj_get_results(ctx, query);
-- 
2.1.3