----- Original Message -----
> From: Dave Airlie <airl...@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 <airl...@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to