On Saturday, December 13, 2014 08:21:35 PM Ben Widawsky wrote:
> On Fri, Dec 12, 2014 at 11:15:38PM -0800, Kenneth Graunke wrote:
> > q->Ready means that the results are in, and core Mesa is free to return
> > them to the application. gen6_queryobj_get_results() is a natural place
> > to set that flag; doing so means callers don't have to.
> >
> > The older non-hardware-context aware code couldn't do this, because we
> > had to call brw_queryobj_get_results() to gather intermediate results
> > when we ran out of space for snapshots in the query buffer. We only
> > gather complete results in the Gen6+ code, however.
> >
> > 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/gen6_queryobj.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/gen6_queryobj.c
> > b/src/mesa/drivers/dri/i965/gen6_queryobj.c
> > index 130236e..3013513 100644
> > --- a/src/mesa/drivers/dri/i965/gen6_queryobj.c
> > +++ b/src/mesa/drivers/dri/i965/gen6_queryobj.c
> > @@ -195,6 +195,8 @@ gen6_queryobj_get_results(struct gl_context *ctx,
> > */
> > drm_intel_bo_unreference(query->bo);
> > query->bo = NULL;
> > +
> > + query->Base.Ready = true;
> > }
> >
> > /**
> > @@ -305,7 +307,6 @@ static void gen6_wait_query(struct gl_context *ctx,
> > struct gl_query_object *q)
> > struct brw_query_object *query = (struct brw_query_object *)q;
> >
> > gen6_queryobj_get_results(ctx, query);
> > - query->Base.Ready = true;
> > }
> >
> > /**
> > @@ -331,7 +332,6 @@ static void gen6_check_query(struct gl_context *ctx,
> > struct gl_query_object *q)
> >
> > if (query->bo == NULL || !drm_intel_bo_busy(query->bo)) {
> > gen6_queryobj_get_results(ctx, query);
> > - query->Base.Ready = true;
> > }
> > }
> The behavior changes here if query->bo == NULL (after the patch Ready is never
> set). However, looking through the code it seems query->bo won't ever be NULL.
> AFAICT therefore, this is fine, but maybe you can take a look and make sure as
> well.You're right, but I think it's fine. The lifetime is: 1. BeginQuery(): - Creates a new BO. (bo != NULL) - q->Ready = False - q->Active = True When Active, asking about the result raises an error; mesa/main/queryobj.c won't ever call the CheckQuery/WaitQuery driver hooks. 2. EndQuery() writes the final snapshot (bo != NULL still), and marks the query as inactive (q->Active = False). Ready is still False. 3. Now you can CheckQuery() or WaitQuery(). If the result is ready, they map query->bo, obtain the results, and free the buffer (bo = NULL). Then it is marked Ready. So...bo == NULL means that some earlier CheckQuery/WaitQuery call already fetched the results, freed the buffer, and set the Ready flag. Setting Ready again would be redundant, so it doesn't matter. Technically, bo == NULL can also happen if you ask for the results but haven't ever called BeginQuery(). This is handled by setting Ready = True when creating new query objects. > Also, I notice an extraneous flush in this path, but I think you address it > in a > later patch. Hmm. Not seeing that, but yeah, it's probably killed by the next patch. > Assuming that's verified (and perhaps commit message updated to explain), this > is > Reviewed-by: Ben Widawsky <b...@bwidawsk.net> Thanks Ben!
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
