[Mesa-dev] [PATCH] llvmpipe: add EXT_timer_query support.
Jose Fonseca
jfonseca at vmware.com
Tue Nov 8 07:21:52 PST 2011
----- Original Message -----
> From: Dave Airlie <airlied at redhat.com>
>
> This adds timer query support, though I'm not 100% sure about the bin
> stuff
> if we have multiple queries in flight, maybe it needs a linked list,
> suggestions
> welcome.
I think that taking the time interval to the last rasterization task to complete is quite a sensible measurement. I can't think of anything better.
Note that the spec doesn't allow nested timed queries
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
> src/gallium/drivers/llvmpipe/lp_query.c | 26
> ++++++++++++++++++++------
> src/gallium/drivers/llvmpipe/lp_query.h | 2 ++
> src/gallium/drivers/llvmpipe/lp_rast.c | 11 +++++++++--
> src/gallium/drivers/llvmpipe/lp_screen.c | 2 +-
> 4 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/src/gallium/drivers/llvmpipe/lp_query.c
> b/src/gallium/drivers/llvmpipe/lp_query.c
> index 42eb856..3b1193b 100644
> --- a/src/gallium/drivers/llvmpipe/lp_query.c
> +++ b/src/gallium/drivers/llvmpipe/lp_query.c
> @@ -30,6 +30,7 @@
> * Keith Whitwell, Qicheng Christopher Li, Brian Paul
> */
>
> +#include "os/os_time.h"
> #include "draw/draw_context.h"
> #include "pipe/p_defines.h"
> #include "util/u_memory.h"
> @@ -51,10 +52,11 @@ llvmpipe_create_query(struct pipe_context *pipe,
> {
> struct llvmpipe_query *pq;
>
> - assert(type == PIPE_QUERY_OCCLUSION_COUNTER);
> + assert(type == PIPE_QUERY_OCCLUSION_COUNTER ||
> + type == PIPE_QUERY_TIME_ELAPSED);
>
> pq = CALLOC_STRUCT( llvmpipe_query );
> -
> + pq->type = type;
> return (struct pipe_query *) pq;
> }
>
> @@ -109,9 +111,18 @@ llvmpipe_get_query_result(struct pipe_context
> *pipe,
>
> /* Sum the results from each of the threads:
> */
> - *result = 0;
> - for (i = 0; i < LP_MAX_THREADS; i++) {
> - *result += pq->count[i];
> + if (pq->type == PIPE_QUERY_TIME_ELAPSED) {
I'd prefer a switch(pq->type) { .. } with an assert(0) on default case, so that the code is consistent as more queries are added.
> + *result = 0;
> + for (i = 0; i < LP_MAX_THREADS; i++) {
> + if (*result < pq->count[i])
> + *result = pq->count[i];
> + }
> + *result -= pq->start;
This will give wrong results on integer wrap around. It would be preferable to do:
*result = 0;
for (i = 0; i < LP_MAX_THREADS; i++) {
uint64_t delta = pq->count[i] - pq->start;
if (delta > *result)
*result = delta;
}
> + } else {
> + *result = 0;
> + for (i = 0; i < LP_MAX_THREADS; i++) {
> + *result += pq->count[i];
> + }
> }
>
> return TRUE;
> @@ -132,8 +143,11 @@ llvmpipe_begin_query(struct pipe_context *pipe,
> struct pipe_query *q)
> llvmpipe_finish(pipe, __FUNCTION__);
> }
>
> -
> memset(pq->count, 0, sizeof(pq->count));
> +
> + if (pq->type == PIPE_QUERY_TIME_ELAPSED)
You should flush the draw module before taking the start time, or the primitivies previous batched inside the draw module will be wrongly included.
Reading http://www.opengl.org/registry/specs/EXT/timer_query.txt it says:
What time interval is being measured?
RESOLVED: The timer starts when all commands prior to BeginQuery() have
been fully executed. At that point, everything that should be drawn by
those commands has been written to the framebuffer. The timer stops
when all commands prior to EndQuery() have been fully executed.
So it sounds like we should actually flush and wait on the idle. (This will slow down the rendering, but will give more meaningful timings).
I just realize that it's really difficult to provide a good timing definition for a pipeline.
> + pq->start = 1000ULL*os_time_get();
Let's do the 1000ULL only once. For example, when just before passing the value to the caller in llvmpipe_get_query_result().
> +
> lp_setup_begin_query(llvmpipe->setup, pq);
>
> llvmpipe->active_query_count++;
> diff --git a/src/gallium/drivers/llvmpipe/lp_query.h
> b/src/gallium/drivers/llvmpipe/lp_query.h
> index ef1bc30..fb467e4 100644
> --- a/src/gallium/drivers/llvmpipe/lp_query.h
> +++ b/src/gallium/drivers/llvmpipe/lp_query.h
> @@ -44,6 +44,8 @@ struct llvmpipe_context;
> struct llvmpipe_query {
> uint64_t count[LP_MAX_THREADS]; /**< a counter for each thread
> */
> struct lp_fence *fence; /* fence from last scene this was
> binned in */
> + unsigned type;
It's a pitty there's no enum for PIPE_QUERY_XXX...
> + uint64_t start;
> };
>
>
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast.c
> b/src/gallium/drivers/llvmpipe/lp_rast.c
> index c4560bf..a27e425 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast.c
> +++ b/src/gallium/drivers/llvmpipe/lp_rast.c
> @@ -32,6 +32,8 @@
> #include "util/u_surface.h"
> #include "util/u_pack_color.h"
>
> +#include "os/os_time.h"
> +
> #include "lp_scene_queue.h"
> #include "lp_debug.h"
> #include "lp_fence.h"
> @@ -485,7 +487,9 @@ lp_rast_begin_query(struct lp_rasterizer_task
> *task,
> struct llvmpipe_query *pq = arg.query_obj;
>
> assert(task->query == NULL);
> - task->vis_counter = 0;
> + if (pq->type == PIPE_QUERY_OCCLUSION_COUNTER)
> + task->vis_counter = 0;
> +
> task->query = pq;
> }
>
> @@ -501,7 +505,10 @@ lp_rast_end_query(struct lp_rasterizer_task
> *task,
> {
> assert(task->query);
> if (task->query) {
> - task->query->count[task->thread_index] += task->vis_counter;
> + if (task->query->type == PIPE_QUERY_OCCLUSION_COUNTER)
> + task->query->count[task->thread_index] += task->vis_counter;
> + else if (task->query->type == PIPE_QUERY_TIME_ELAPSED)
> + task->query->count[task->thread_index] = 1000ULL * os_time_get();
> task->query = NULL;
Switch statement too here please.
> }
> }
> diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c
> b/src/gallium/drivers/llvmpipe/lp_screen.c
> index c8a088c..369fd91 100644
> --- a/src/gallium/drivers/llvmpipe/lp_screen.c
> +++ b/src/gallium/drivers/llvmpipe/lp_screen.c
> @@ -123,7 +123,7 @@ llvmpipe_get_param(struct pipe_screen *screen,
> enum pipe_cap param)
> case PIPE_CAP_OCCLUSION_QUERY:
> return 1;
> case PIPE_CAP_TIMER_QUERY:
> - return 0;
> + return 1;
> case PIPE_CAP_TEXTURE_MIRROR_CLAMP:
> return 1;
> case PIPE_CAP_TEXTURE_SHADOW_MAP:
> --
> 1.7.6.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list