[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