[Mesa-dev] [PATCH 04/10] i965: Add a pile of comments to brw_queryobj.c.

[Eric Anholt]

Kenneth Graunke <kenneth at whitecape.org> writes:

> This code was really difficult to follow, for a number of reasons:
>
> - Queries were handled in four different ways (TIMESTAMP writes a single
>   value, TIME_ELAPSED writes a single pair of values, occlusion queries
>   write pairs of values for the start and end of each batch, and other
>   queries are done entirely in software.  It turns out that there are
>   very good reasons each query is handled the way it is, but
>   insufficient comments explaining the rationale.
>
> - It wasn't immediately obvious which functions were driver hooks
>   and which were helper functions.  For example, brw_query_begin() is
>   a driver hook that implements glBeginQuery() for all query types, but
>   the similarly named brw_emit_query_begin() is a helper function that's
>   only relevant for occlusion queries.
>
> Extra explanatory comments should save me and others from constantly
> having to ask how this code works and why various query types are
> handled differently.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_queryobj.c | 159 +++++++++++++++++++++++++++----
>  1 file changed, 143 insertions(+), 16 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_queryobj.c b/src/mesa/drivers/dri/i965/brw_queryobj.c
> index c6ed8e2..f02d051 100644
> --- a/src/mesa/drivers/dri/i965/brw_queryobj.c
> +++ b/src/mesa/drivers/dri/i965/brw_queryobj.c

> -/** Waits on the query object's BO and totals the results for this query */
> +/**
> + * Wait on the query object's BO and calculate the final result.
> + */
>  static void
>  brw_queryobj_get_results(struct gl_context *ctx,
>  			 struct brw_query_object *query)
> @@ -142,6 +145,10 @@ brw_queryobj_get_results(struct gl_context *ctx,
>     if (query->bo == NULL)
>        return;
>  
> +   /* If the application has requested the query result, but this batch is
> +    * still contributing to it, flush it now so it'll become available as
> +    * soon as possible.
> +    */

It's not really about "as soon as possible", it's "so those results will
be present when mapped" -- mapping doesn't batch flush on its own, since
libdrm is unaware of batchbuffers.

Other than that, patches up to this are:

Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130228/e22f0c49/attachment.pgp>