On Monday, December 15, 2014 01:38:47 PM Ben Widawsky wrote: > On Mon, Dec 15, 2014 at 01:28:52PM -0800, Ian Romanick wrote: > > On 12/14/2014 04:39 PM, Ben Widawsky wrote: > > > On Fri, Dec 12, 2014 at 11:15:42PM -0800, Kenneth Graunke wrote: > > >> 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 <kenn...@whitecape.org> > > >> Cc: Chris Wilson <ch...@chris-wilson.co.uk> > > >> Cc: Eero Tamminen <eero.t.tammi...@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; > > >> +} > > > > > > I think it makes more sense to set the flushed state at BeginQuery, and to > > > remove this hunk. I think that's feasible. Am I correct that this is the > > > generalized regex for the callchain? > > > BeginQuery->(flush_batch_if_needed)*->EndQuery->(flush_batch_if_needed)*->EndQuery > > > > > > No. It's BeginQuery->(flush_batch_if_needed)*->EndQuery. The > > application cannot ask for the query results until after the EndQuery. > > I think the way Ken has it is correct. > > > > It seems like until we implement QueryCounter that's not true. (I mentioned > this below)
You're right, I neglected to explain QueryCounter(). Most queries take two counter snapshots (at BeginQuery and EndQuery time), and we subtract the two to produce a proper value. GL_TIMESTAMP is a unique counter - instead of a delta, you only get a single value. You don't use glBeginQuery and glEndQuery to access it. Instead, you use glQueryCounter(q, GL_TIMESTAMP). So, the workflow is: glQueryCounter -> CheckQuery or WaitQuery (which may flush the batch). In other words, QueryCounter replaces BeginQuery/EndQuery. The rest is the same.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
