On Wednesday, June 14, 2017 3:50:12 PM PDT Kenneth Graunke wrote:
> On Friday, June 9, 2017 6:01:37 AM PDT Chris Wilson wrote:
> > If we map the bo upon creation, we can avoid the latency of mmapping it
> > when querying, and later use the asynchronous, persistent map of the
> > predicate to do a quick query.
> >
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > Cc: Kenneth Graunke <kenn...@whitecape.org>
> > Cc: Matt Turner <matts...@gmail.com>
> > ---
> > src/mesa/drivers/dri/i965/brw_bufmgr.c | 15 +++++++++++++
> > src/mesa/drivers/dri/i965/brw_bufmgr.h | 2 ++
> > src/mesa/drivers/dri/i965/brw_context.h | 1 +
> > src/mesa/drivers/dri/i965/gen6_queryobj.c | 37
> > ++++++++++++++++++++++---------
> > 4 files changed, 44 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> > b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> > index 01590a0b0a..9028b538c6 100644
> > --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> > +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> > @@ -775,6 +775,21 @@ brw_bo_map(struct brw_context *brw, struct brw_bo *bo,
> > unsigned flags)
> > return brw_bo_map_gtt(brw, bo, flags);
> > }
> >
> > +void
> > +brw_bo_map_sync(struct brw_context *brw, struct brw_bo *bo, unsigned flags)
> > +{
> > + unsigned domain;
> > +
> > + if (bo->tiling_mode != I915_TILING_NONE && !(flags & MAP_RAW))
> > + domain = I915_GEM_DOMAIN_GTT;
> > + else if (can_map_cpu(bo, flags))
> > + domain = I915_GEM_DOMAIN_CPU;
> > + else
> > + domain = I915_GEM_DOMAIN_GTT;
> > +
> > + set_domain(brw, __func__, bo, domain, flags & MAP_WRITE ? domain : 0);
> > +}
> > +
> > int
> > brw_bo_unmap(struct brw_bo *bo)
> > {
> > diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> > b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> > index 3a397be695..214b75bf1a 100644
> > --- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
> > +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
> > @@ -196,6 +196,8 @@ void brw_bo_unreference(struct brw_bo *bo);
> > */
> > MUST_CHECK void *brw_bo_map(struct brw_context *brw, struct brw_bo *bo,
> > unsigned flags);
> >
> > +void brw_bo_map_sync(struct brw_context *brw, struct brw_bo *bo, unsigned
> > flags);
> > +
> > /**
> > * Reduces the refcount on the userspace mapping of the buffer
> > * object.
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> > b/src/mesa/drivers/dri/i965/brw_context.h
> > index c5acb83ad0..117b1ecdca 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.h
> > +++ b/src/mesa/drivers/dri/i965/brw_context.h
> > @@ -419,6 +419,7 @@ struct brw_query_object {
> >
> > /** Last query BO associated with this query. */
> > struct brw_bo *bo;
> > + uint64_t *results;
> >
> > /** Last index in bo with query data for this object. */
> > int last_index;
> > diff --git a/src/mesa/drivers/dri/i965/gen6_queryobj.c
> > b/src/mesa/drivers/dri/i965/gen6_queryobj.c
> > index f913f986ae..18af608166 100644
> > --- a/src/mesa/drivers/dri/i965/gen6_queryobj.c
> > +++ b/src/mesa/drivers/dri/i965/gen6_queryobj.c
> > @@ -221,7 +221,9 @@ gen6_queryobj_get_results(struct gl_context *ctx,
> > if (query->bo == NULL)
> > return;
> >
> > - uint64_t *results = brw_bo_map(brw, query->bo, MAP_READ);
> > + brw_bo_map_sync(brw, query->bo, MAP_READ | MAP_COHERENT);
> > + uint64_t *results = query->results;
> > +
> > switch (query->Base.Target) {
> > case GL_TIME_ELAPSED:
> > /* The query BO contains the starting and ending timestamps.
> > @@ -296,7 +298,6 @@ gen6_queryobj_get_results(struct gl_context *ctx,
> > default:
> > unreachable("Unrecognized query target in
> > brw_queryobj_get_results()");
> > }
> > - brw_bo_unmap(query->bo);
> >
> > /* Now that we've processed the data stored in the query's buffer
> > object,
> > * we can release it.
> > @@ -307,6 +308,23 @@ gen6_queryobj_get_results(struct gl_context *ctx,
> > query->Base.Ready = true;
> > }
> >
> > +static int gen6_alloc_query(struct brw_context *brw,
> > + struct brw_query_object *query)
> > +{
> > + /* Since we're starting a new query, we need to throw away old results.
> > */
> > + if (query->bo)
> > + brw_bo_unreference(query->bo);
> > +
> > + query->bo = brw_bo_alloc(brw->bufmgr, "query results", 4096, 4096);
> > + query->results = brw_bo_map(brw, query->bo,
> > + MAP_READ | MAP_COHERENT | MAP_ASYNC);
>
> I don't understand why you're using MAP_ASYNC here. We're allocating a new
> BO here, and not using the BO_ALLOC_FOR_RENDER flag, so it will be idle.
> (brw_bufmgr.c:297 should ensure we never get a busy BO - if the cached BOs
> are busy, it will just allocate us a new one.)
>
> So, MAP_ASYNC shouldn't avoid a stall. It does, however, skip the
> SET_DOMAIN call, which means that it may not have the right domain
> for our new coherent mapping. Hence, you need to whack it later with
> your new brw_bo_map_sync() helper.
>
> I think you can drop MAP_ASYNC, and drop brw_bo_map_sync() entirely,
> with no ill-effects. Or am I wrong?D'oh, in fact I am wrong. brw_bo_map_sync() is what stalls, allowing WaitQuery() to wait until the counter snapshots actually land in the buffer. That's pretty darn important. Now I'm just confused by all the domains.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
