[Mesa-dev] [PATCH 8/9] gallium/hud: add support for batch queries

Samuel Pitoiset samuel.pitoiset at gmail.com
Fri Nov 13 09:35:12 PST 2015


Some comments below.

On 11/13/2015 04:57 PM, Nicolai Hähnle wrote:
> ---
>   src/gallium/auxiliary/hud/hud_context.c      |  24 ++-
>   src/gallium/auxiliary/hud/hud_driver_query.c | 248 +++++++++++++++++++++++----
>   src/gallium/auxiliary/hud/hud_private.h      |  13 +-
>   3 files changed, 240 insertions(+), 45 deletions(-)
>
> diff --git a/src/gallium/auxiliary/hud/hud_context.c b/src/gallium/auxiliary/hud/hud_context.c
> index ffe30b8..bcef701 100644
> --- a/src/gallium/auxiliary/hud/hud_context.c
> +++ b/src/gallium/auxiliary/hud/hud_context.c
> @@ -57,6 +57,7 @@ struct hud_context {
>      struct cso_context *cso;
>      struct u_upload_mgr *uploader;
>
> +   struct hud_batch_query_context *batch_query;
>      struct list_head pane_list;
>
>      /* states */
> @@ -510,6 +511,8 @@ hud_draw(struct hud_context *hud, struct pipe_resource *tex)
>      hud_alloc_vertices(hud, &hud->text, 4 * 512, 4 * sizeof(float));
>
>      /* prepare all graphs */
> +   hud_batch_query_update(hud->batch_query);
> +
>      LIST_FOR_EACH_ENTRY(pane, &hud->pane_list, head) {
>         LIST_FOR_EACH_ENTRY(gr, &pane->graph_list, head) {
>            gr->query_new_value(gr);
> @@ -903,17 +906,21 @@ hud_parse_env_var(struct hud_context *hud, const char *env)
>         }
>         else if (strcmp(name, "samples-passed") == 0 &&
>                  has_occlusion_query(hud->pipe->screen)) {
> -         hud_pipe_query_install(pane, hud->pipe, "samples-passed",
> +         hud_pipe_query_install(&hud->batch_query, pane, hud->pipe,
> +                                "samples-passed",
>                                   PIPE_QUERY_OCCLUSION_COUNTER, 0, 0,
>                                   PIPE_DRIVER_QUERY_TYPE_UINT64,
> -                                PIPE_DRIVER_QUERY_RESULT_TYPE_AVERAGE);
> +                                PIPE_DRIVER_QUERY_RESULT_TYPE_AVERAGE,
> +                                0);
>         }
>         else if (strcmp(name, "primitives-generated") == 0 &&
>                  has_streamout(hud->pipe->screen)) {
> -         hud_pipe_query_install(pane, hud->pipe, "primitives-generated",
> +         hud_pipe_query_install(&hud->batch_query, pane, hud->pipe,
> +                                "primitives-generated",
>                                   PIPE_QUERY_PRIMITIVES_GENERATED, 0, 0,
>                                   PIPE_DRIVER_QUERY_TYPE_UINT64,
> -                                PIPE_DRIVER_QUERY_RESULT_TYPE_AVERAGE);
> +                                PIPE_DRIVER_QUERY_RESULT_TYPE_AVERAGE,
> +                                0);
>         }
>         else {
>            boolean processed = FALSE;
> @@ -938,17 +945,19 @@ hud_parse_env_var(struct hud_context *hud, const char *env)
>                  if (strcmp(name, pipeline_statistics_names[i]) == 0)
>                     break;
>               if (i < Elements(pipeline_statistics_names)) {
> -               hud_pipe_query_install(pane, hud->pipe, name,
> +               hud_pipe_query_install(&hud->batch_query, pane, hud->pipe, name,
>                                         PIPE_QUERY_PIPELINE_STATISTICS, i,
>                                         0, PIPE_DRIVER_QUERY_TYPE_UINT64,
> -                                      PIPE_DRIVER_QUERY_RESULT_TYPE_AVERAGE);
> +                                      PIPE_DRIVER_QUERY_RESULT_TYPE_AVERAGE,
> +                                      0);
>                  processed = TRUE;
>               }
>            }
>
>            /* driver queries */
>            if (!processed) {
> -            if (!hud_driver_query_install(pane, hud->pipe, name)){
> +            if (!hud_driver_query_install(&hud->batch_query, pane, hud->pipe,
> +                                          name)) {
>                  fprintf(stderr, "gallium_hud: unknown driver query '%s'\n", name);
>               }
>            }
> @@ -1287,6 +1296,7 @@ hud_destroy(struct hud_context *hud)
>         FREE(pane);
>      }
>
> +   hud_batch_query_cleanup(&hud->batch_query);
>      pipe->delete_fs_state(pipe, hud->fs_color);
>      pipe->delete_fs_state(pipe, hud->fs_text);
>      pipe->delete_vs_state(pipe, hud->vs);
> diff --git a/src/gallium/auxiliary/hud/hud_driver_query.c b/src/gallium/auxiliary/hud/hud_driver_query.c
> index 3198ab3..abc9f54 100644
> --- a/src/gallium/auxiliary/hud/hud_driver_query.c
> +++ b/src/gallium/auxiliary/hud/hud_driver_query.c
> @@ -34,13 +34,149 @@
>   #include "hud/hud_private.h"
>   #include "pipe/p_screen.h"
>   #include "os/os_time.h"
> +#include "util/u_math.h"
>   #include "util/u_memory.h"
>   #include <stdio.h>
>
> +// Must be a power of two
>   #define NUM_QUERIES 8
>
> +struct hud_batch_query_context {
> +   struct pipe_context *pipe;
> +   unsigned num_query_types;
> +   unsigned allocated_query_types;
> +   unsigned *query_types;
> +
> +   boolean failed;
> +   struct pipe_query *query[NUM_QUERIES];
> +   union pipe_query_result *result[NUM_QUERIES];
> +   unsigned head, pending, results;
> +};
> +
> +void
> +hud_batch_query_update(struct hud_batch_query_context *bq)
> +{
> +   struct pipe_context *pipe;
> +
> +   if (!bq || bq->failed)
> +      return;
> +
> +   pipe = bq->pipe;
> +
> +   if (bq->query[bq->head])
> +      pipe->end_query(pipe, bq->query[bq->head]);
> +
> +   bq->results = 0;
> +
> +   while (bq->pending) {
> +      unsigned idx = (bq->head - bq->pending + 1) % NUM_QUERIES;
> +      struct pipe_query *query = bq->query[idx];
> +
> +      if (!bq->result[idx])
> +         bq->result[idx] = MALLOC(sizeof(bq->result[idx]->batch[0]) *
> +                                  bq->num_query_types);
> +
> +      if (!pipe->get_query_result(pipe, query, FALSE, bq->result[idx]))
> +         break;
> +
> +      ++bq->results;
> +      --bq->pending;
> +   }
> +
> +   bq->head = (bq->head + 1) % NUM_QUERIES;
> +
> +   if (bq->pending == NUM_QUERIES) {
> +      fprintf(stderr,
> +              "gallium_hud: all queries busy after %i frames, dropping data.\n",
> +              NUM_QUERIES);
> +
> +      assert(bq->query[bq->head]);
> +
> +      pipe->destroy_query(bq->pipe, bq->query[bq->head]);
> +      bq->query[bq->head] = NULL;
> +   }
> +
> +   ++bq->pending;
> +
> +   if (!bq->query[bq->head]) {
> +      bq->query[bq->head] = pipe->create_batch_query(pipe,
> +                                                     bq->num_query_types,
> +                                                     bq->query_types);
> +
> +      if (!bq->query[bq->head]) {
> +         fprintf(stderr,
> +                 "gallium_hud: create_batch_query failed. You may have "
> +                 "selected too many or incompatible queries.\n");
> +         bq->failed = TRUE;
> +         return;
> +      }
> +   }
> +
> +   if (!pipe->begin_query(pipe, bq->query[bq->head])) {
> +      fprintf(stderr,
> +              "gallium_hud: could not begin batch query. You may have "
> +              "selected too many or incompatible queries.\n");
> +      bq->failed = TRUE;
> +   }
> +}
> +
> +static unsigned
> +batch_query_add(struct hud_batch_query_context **pbq,
> +                struct pipe_context *pipe, unsigned query_type)
> +{
> +   struct hud_batch_query_context *bq = *pbq;
> +   unsigned i;
> +
> +   if (!bq) {
> +      bq = CALLOC_STRUCT(hud_batch_query_context);

Please check if the allocation didn't fail.

> +      bq->pipe = pipe;
> +      *pbq = bq;
> +   }
> +
> +   for (i = 0; i < bq->num_query_types; ++i) {
> +      if (bq->query_types[i] == query_type)
> +         return i;
> +   }
> +
> +   if (bq->num_query_types == bq->allocated_query_types) {
> +      unsigned new_alloc = MAX2(16, bq->allocated_query_types * 2);
> +      bq->query_types = REALLOC(bq->query_types,
> +                                bq->allocated_query_types * sizeof(unsigned),
> +                                new_alloc * sizeof(unsigned));

Same here!

> +      bq->allocated_query_types = new_alloc;
> +   }
> +
> +   bq->query_types[bq->num_query_types] = query_type;
> +   return bq->num_query_types++;
> +}
> +
> +void
> +hud_batch_query_cleanup(struct hud_batch_query_context **pbq)
> +{
> +   struct hud_batch_query_context *bq = *pbq;
> +   unsigned idx;
> +
> +   if (!bq)
> +      return;
> +
> +   *pbq = NULL;
> +
> +   if (bq->query[bq->head] && !bq->failed)
> +      bq->pipe->end_query(bq->pipe, bq->query[bq->head]);
> +
> +   for (idx = 0; idx < NUM_QUERIES; ++idx) {
> +      if (bq->query[idx])
> +         bq->pipe->destroy_query(bq->pipe, bq->query[idx]);
> +      FREE(bq->result[idx]);
> +   }
> +
> +   FREE(bq->query_types);
> +   FREE(bq);
> +}
> +
>   struct query_info {
>      struct pipe_context *pipe;
> +   struct hud_batch_query_context *batch;
>      unsigned query_type;
>      unsigned result_index; /* unit depends on query_type */
>      enum pipe_driver_query_result_type result_type;
> @@ -55,11 +191,26 @@ struct query_info {
>   };
>
>   static void
> -query_new_value(struct hud_graph *gr)
> +query_new_value_batch(struct query_info *info)
> +{
> +   struct hud_batch_query_context *bq = info->batch;
> +   unsigned result_index = info->result_index;
> +   unsigned idx = (bq->head - bq->pending) % NUM_QUERIES;
> +   unsigned results = bq->results;
> +
> +   while (results) {
> +      info->results_cumulative += bq->result[idx]->batch[result_index].u64;
> +      ++info->num_results;
> +
> +      --results;
> +      idx = (idx - 1) % NUM_QUERIES;
> +   }
> +}
> +
> +static void
> +query_new_value_normal(struct query_info *info)
>   {
> -   struct query_info *info = gr->query_data;
>      struct pipe_context *pipe = info->pipe;
> -   uint64_t now = os_time_get();
>
>      if (info->last_time) {
>         if (info->query[info->head])
> @@ -106,30 +257,9 @@ query_new_value(struct hud_graph *gr)
>               break;
>            }
>         }
> -
> -      if (info->num_results && info->last_time + gr->pane->period <= now) {
> -         uint64_t value;
> -
> -         switch (info->result_type) {
> -         default:
> -         case PIPE_DRIVER_QUERY_RESULT_TYPE_AVERAGE:
> -            value = info->results_cumulative / info->num_results;
> -            break;
> -         case PIPE_DRIVER_QUERY_RESULT_TYPE_CUMULATIVE:
> -            value = info->results_cumulative;
> -            break;
> -         }
> -
> -         hud_graph_add_value(gr, value);
> -
> -         info->last_time = now;
> -         info->results_cumulative = 0;
> -         info->num_results = 0;
> -      }
>      }
>      else {
>         /* initialize */
> -      info->last_time = now;
>         info->query[info->head] = pipe->create_query(pipe, info->query_type, 0);
>      }
>
> @@ -138,11 +268,49 @@ query_new_value(struct hud_graph *gr)
>   }
>
>   static void
> +query_new_value(struct hud_graph *gr)
> +{
> +   struct query_info *info = gr->query_data;
> +   uint64_t now = os_time_get();
> +
> +   if (info->batch) {
> +      query_new_value_batch(info);
> +   } else {
> +      query_new_value_normal(info);
> +   }
> +
> +   if (!info->last_time) {
> +      info->last_time = now;
> +      return;
> +   }
> +
> +   if (info->num_results && info->last_time + gr->pane->period <= now) {
> +      uint64_t value;
> +
> +      switch (info->result_type) {
> +      default:
> +      case PIPE_DRIVER_QUERY_RESULT_TYPE_AVERAGE:
> +         value = info->results_cumulative / info->num_results;
> +         break;
> +      case PIPE_DRIVER_QUERY_RESULT_TYPE_CUMULATIVE:
> +         value = info->results_cumulative;
> +         break;
> +      }
> +
> +      hud_graph_add_value(gr, value);
> +
> +      info->last_time = now;
> +      info->results_cumulative = 0;
> +      info->num_results = 0;
> +   }
> +}
> +
> +static void
>   free_query_info(void *ptr)
>   {
>      struct query_info *info = ptr;
>
> -   if (info->last_time) {
> +   if (!info->batch && info->last_time) {
>         struct pipe_context *pipe = info->pipe;
>         int i;
>
> @@ -158,11 +326,13 @@ free_query_info(void *ptr)
>   }
>
>   void
> -hud_pipe_query_install(struct hud_pane *pane, struct pipe_context *pipe,
> +hud_pipe_query_install(struct hud_batch_query_context **pbq,
> +                       struct hud_pane *pane, struct pipe_context *pipe,
>                          const char *name, unsigned query_type,
>                          unsigned result_index,
>                          uint64_t max_value, enum pipe_driver_query_type type,
> -                       enum pipe_driver_query_result_type result_type)
> +                       enum pipe_driver_query_result_type result_type,
> +                       unsigned flags)
>   {
>      struct hud_graph *gr;
>      struct query_info *info;
> @@ -184,10 +354,16 @@ hud_pipe_query_install(struct hud_pane *pane, struct pipe_context *pipe,
>
>      info = gr->query_data;
>      info->pipe = pipe;
> -   info->query_type = query_type;
> -   info->result_index = result_index;
>      info->result_type = result_type;
>
> +   if (flags & PIPE_DRIVER_QUERY_FLAG_BATCH) {
> +      info->result_index = batch_query_add(pbq, pipe, query_type);
> +      info->batch = *pbq;
> +   } else {
> +      info->query_type = query_type;
> +      info->result_index = result_index;
> +   }
> +
>      hud_pane_add_graph(pane, gr);
>      if (pane->max_value < max_value)
>         hud_pane_set_max_value(pane, max_value);
> @@ -195,20 +371,21 @@ hud_pipe_query_install(struct hud_pane *pane, struct pipe_context *pipe,
>   }
>
>   boolean
> -hud_driver_query_install(struct hud_pane *pane, struct pipe_context *pipe,
> +hud_driver_query_install(struct hud_batch_query_context **pbq,
> +                         struct hud_pane *pane, struct pipe_context *pipe,
>                            const char *name)
>   {
>      struct pipe_screen *screen = pipe->screen;
>      struct pipe_driver_query_info query;
> -   unsigned num_queries, i;
> +   unsigned num_query_types, i;

This change (s/num_queries/num_query_types) is *really* not needed and 
it doesn't not improve readability of both the patch and the original 
code. Please drop.

>      boolean found = FALSE;
>
>      if (!screen->get_driver_query_info)
>         return FALSE;
>
> -   num_queries = screen->get_driver_query_info(screen, 0, NULL);
> +   num_query_types = screen->get_driver_query_info(screen, 0, NULL);
>
> -   for (i = 0; i < num_queries; i++) {
> +   for (i = 0; i < num_query_types; i++) {
>         if (screen->get_driver_query_info(screen, i, &query) &&
>             strcmp(query.name, name) == 0) {
>            found = TRUE;
> @@ -219,8 +396,9 @@ hud_driver_query_install(struct hud_pane *pane, struct pipe_context *pipe,
>      if (!found)
>         return FALSE;
>
> -   hud_pipe_query_install(pane, pipe, query.name, query.query_type, 0,
> -                          query.max_value.u64, query.type, query.result_type);
> +   hud_pipe_query_install(pbq, pane, pipe, query.name, query.query_type, 0,
> +                          query.max_value.u64, query.type, query.result_type,
> +                          query.flags);
>
>      return TRUE;
>   }
> diff --git a/src/gallium/auxiliary/hud/hud_private.h b/src/gallium/auxiliary/hud/hud_private.h
> index 01caf7b..4a788bb 100644
> --- a/src/gallium/auxiliary/hud/hud_private.h
> +++ b/src/gallium/auxiliary/hud/hud_private.h
> @@ -80,19 +80,26 @@ void hud_pane_set_max_value(struct hud_pane *pane, uint64_t value);
>   void hud_graph_add_value(struct hud_graph *gr, uint64_t value);
>
>   /* graphs/queries */
> +struct hud_batch_query_context;
> +
>   #define ALL_CPUS ~0 /* optionally set as cpu_index */
>
>   int hud_get_num_cpus(void);
>
>   void hud_fps_graph_install(struct hud_pane *pane);
>   void hud_cpu_graph_install(struct hud_pane *pane, unsigned cpu_index);
> -void hud_pipe_query_install(struct hud_pane *pane, struct pipe_context *pipe,
> +void hud_pipe_query_install(struct hud_batch_query_context **pbq,
> +                            struct hud_pane *pane, struct pipe_context *pipe,
>                               const char *name, unsigned query_type,
>                               unsigned result_index,
>                               uint64_t max_value,
>                               enum pipe_driver_query_type type,
> -                            enum pipe_driver_query_result_type result_type);
> -boolean hud_driver_query_install(struct hud_pane *pane,
> +                            enum pipe_driver_query_result_type result_type,
> +                            unsigned flags);
> +boolean hud_driver_query_install(struct hud_batch_query_context **pbq,
> +                                 struct hud_pane *pane,
>                                    struct pipe_context *pipe, const char *name);
> +void hud_batch_query_update(struct hud_batch_query_context *bq);
> +void hud_batch_query_cleanup(struct hud_batch_query_context **pbq);
>
>   #endif
>

-- 
-Samuel


More information about the mesa-dev mailing list