[Mesa-dev] [PATCH 1/3] st/mesa: Make an enum for pipeline statistics query result indices.

[Ilia Mirkin]

On Sat, Dec 15, 2018 at 4:45 AM Kenneth Graunke <kenneth at whitecape.org> wrote:
> Gallium handles pipeline statistics queries as a single query
> (PIPE_QUERY_PIPELINE_STATISTICS) which returns a struct with 11 values.
> Sometimes it's useful to refer to each of those values individually,
> rather than as a group.  To avoid hardcoding numbers, we define a new
> enum for each value.  Here, the name and enum value correspond to the
> index in the struct pipe_query_data_pipeline_statistics result.

This later-in-the-series desire to be able to get just one value back
from Gallium is an API change which would break any existing d3d1x
state trackers. I realize you're not changing any drivers, but in my
mind, it's preferable not to have ambiguous APIs where some drivers do
one thing, others do another. For NVIDIA, we have to fetch the
counters individually too, but we just do them all. It's ~free to do
so, and we only do one set of synchronization for all of them.

Anyways, I'd rather not have this ambiguous thing of "you could return
some or all" situation. We should be more definitive. I'd recommend
adding a PIPE_QUERY_PIPELINE_STATISTICS_ONE to differentiate [or
PIPE_QUERY_PIPELINE_STATISTIC if you want to be clever and cause lots
of typos], along with a CAP for support.

>
> Cc: Roland Scheidegger <sroland at vmware.com>
> ---
>  src/gallium/include/pipe/p_defines.h    | 17 +++++++++++++++++
>  src/mesa/state_tracker/st_cb_queryobj.c | 22 +++++++++++-----------
>  2 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
> index 6d96f1ccb5b..21005955a36 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -568,6 +568,23 @@ enum pipe_query_type {
>     PIPE_QUERY_DRIVER_SPECIFIC = 256,
>  };
>
> +/**
> + * Index for PIPE_QUERY_PIPELINE_STATISTICS subqueries.
> + */
> +enum pipe_statistics_query_index {
> +   PIPE_STAT_QUERY_IA_VERTICES,
> +   PIPE_STAT_QUERY_IA_PRIMITIVES,
> +   PIPE_STAT_QUERY_VS_INVOCATIONS,
> +   PIPE_STAT_QUERY_GS_INVOCATIONS,
> +   PIPE_STAT_QUERY_GS_PRIMITIVES,
> +   PIPE_STAT_QUERY_C_INVOCATIONS,
> +   PIPE_STAT_QUERY_C_PRIMITIVES,
> +   PIPE_STAT_QUERY_PS_INVOCATIONS,
> +   PIPE_STAT_QUERY_HS_INVOCATIONS,
> +   PIPE_STAT_QUERY_DS_INVOCATIONS,
> +   PIPE_STAT_QUERY_CS_INVOCATIONS,
> +};
> +
>  /**
>   * Conditional rendering modes
>   */
> diff --git a/src/mesa/state_tracker/st_cb_queryobj.c b/src/mesa/state_tracker/st_cb_queryobj.c
> index 69e6004c3f1..82f53243336 100644
> --- a/src/mesa/state_tracker/st_cb_queryobj.c
> +++ b/src/mesa/state_tracker/st_cb_queryobj.c
> @@ -386,37 +386,37 @@ st_StoreQueryResult(struct gl_context *ctx, struct gl_query_object *q,
>     } else if (stq->type == PIPE_QUERY_PIPELINE_STATISTICS) {
>        switch (q->Target) {
>        case GL_VERTICES_SUBMITTED_ARB:
> -         index = 0;
> +         index = PIPE_STAT_QUERY_IA_VERTICES;
>           break;
>        case GL_PRIMITIVES_SUBMITTED_ARB:
> -         index = 1;
> +         index = PIPE_STAT_QUERY_IA_PRIMITIVES;
>           break;
>        case GL_VERTEX_SHADER_INVOCATIONS_ARB:
> -         index = 2;
> +         index = PIPE_STAT_QUERY_VS_INVOCATIONS;
>           break;
>        case GL_GEOMETRY_SHADER_INVOCATIONS:
> -         index = 3;
> +         index = PIPE_STAT_QUERY_GS_INVOCATIONS;
>           break;
>        case GL_GEOMETRY_SHADER_PRIMITIVES_EMITTED_ARB:
> -         index = 4;
> +         index = PIPE_STAT_QUERY_GS_PRIMITIVES;
>           break;
>        case GL_CLIPPING_INPUT_PRIMITIVES_ARB:
> -         index = 5;
> +         index = PIPE_STAT_QUERY_C_INVOCATIONS;
>           break;
>        case GL_CLIPPING_OUTPUT_PRIMITIVES_ARB:
> -         index = 6;
> +         index = PIPE_STAT_QUERY_C_PRIMITIVES;
>           break;
>        case GL_FRAGMENT_SHADER_INVOCATIONS_ARB:
> -         index = 7;
> +         index = PIPE_STAT_QUERY_PS_INVOCATIONS;
>           break;
>        case GL_TESS_CONTROL_SHADER_PATCHES_ARB:
> -         index = 8;
> +         index = PIPE_STAT_QUERY_HS_INVOCATIONS;
>           break;
>        case GL_TESS_EVALUATION_SHADER_INVOCATIONS_ARB:
> -         index = 9;
> +         index = PIPE_STAT_QUERY_DS_INVOCATIONS;
>           break;
>        case GL_COMPUTE_SHADER_INVOCATIONS_ARB:
> -         index = 10;
> +         index = PIPE_STAT_QUERY_CS_INVOCATIONS;
>           break;
>        default:
>           unreachable("Unexpected target");
> --
> 2.19.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev