[Mesa-dev] [PATCH 2/3] Model INTEL perf query backend after query object BE

[Kenneth Graunke]

On Wednesday, February 15, 2017 1:37:36 PM PST Robert Bragg wrote:
> Instead of using the same backend interface as AMD_performance_monitor
> this defines a dedicated INTEL_performance_query interface that is
> modelled more on the ARB_query_buffer_object interface (considering the
> similarity of the extensions) with the addition of vfuncs for
> initializing and enumerating query and counter info.

Patches 1 and 2's commit titles should start with "mesa: ".

> Compared to the previous backend, some notable differences are:
> 
> - The backend is free to represent counters using whatever data
>   structures are optimal/convenient since queries and counters are
>   enumerated via an iterator api instead of declaring them using
>   structures directly shared with the frontend.
> 
>   This is also done to help us support the full range of data and
>   semantic types available with INTEL_performance_query which is awkward
>   while using a structure shared with the AMD_performance_monitor
>   backend since neither extension's types are a subset of the other.
> 
> - The backend must support waiting for a query instead of the frontend
>   simply using glFinish().
> 
> - Objects go through 'Active' and 'Ready' states consistent with the
>   query object backend (hopefully making them more familiar). There is
>   no 'Ended' state (which used to show that a query has ended at least
>   once for a given object). There is a new 'Used' state similar to the
>   'EverBound' state of query objects, set when a query is first begun
>   which implies that we are expecting to get results back for the object
>   at some point.

That's a little different from EverBound, which is used to answer stupid
glIsFoo() queries - where glGenFoo() doesn't actually "create" a Foo,
but glBindFoo() does.  An awkward concept.

> The INTEL_performance_query and AMD_performance_monitor extensions are
> now completely orthogonal within Mesa main (though a driver could
> optionally choose to implement both extensions within a unified backend
> if that were convenient for the sake of sharing state/code).
> 
> v2: (Samuel Pitoiset)
> - init PerfQuery.NumQueries in frontend
> - s/return_string/output_clipped_string/
> - s/backed/backend/ typo
> - remove redundant *bytesWritten = 0
> v3:
> - Add InitPerfQueryInfo for lazy probing of available queries
> 
> Signed-off-by: Robert Bragg <robert at sixbynine.org>
> ---
>  src/mesa/main/dd.h                |  41 +++
>  src/mesa/main/mtypes.h            |  24 +-
>  src/mesa/main/performance_query.c | 625 ++++++++++++++------------------------
>  src/mesa/main/performance_query.h |   6 +-
>  4 files changed, 295 insertions(+), 401 deletions(-)
> 
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index 7ebd084ca3..e77df31cf2 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -780,6 +780,47 @@ struct dd_function_table {
>     /*@}*/
>  
>     /**
> +    * \name Performance Query objects
> +    */
> +   /*@{*/
> +   GLuint (*InitPerfQueryInfo)(struct gl_context *ctx);
> +   void (*GetPerfQueryInfo)(struct gl_context *ctx,
> +                            int queryIndex,
> +                            const char **name,
> +                            GLuint *dataSize,
> +                            GLuint *numCounters,
> +                            GLuint *numActive);
> +   void (*GetPerfCounterInfo)(struct gl_context *ctx,
> +                              int queryIndex,
> +                              int counterIndex,
> +                              const char **name,
> +                              const char **desc,
> +                              GLuint *offset,
> +                              GLuint *data_size,
> +                              GLuint *type_enum,
> +                              GLuint *data_type_enum,
> +                              GLuint64 *raw_max);
> +   struct gl_perf_query_object * (*NewPerfQueryObject)(struct gl_context *ctx,
> +                                                       int queryIndex);
> +   void (*DeletePerfQuery)(struct gl_context *ctx,
> +                           struct gl_perf_query_object *obj);
> +   GLboolean (*BeginPerfQuery)(struct gl_context *ctx,
> +                               struct gl_perf_query_object *obj);
> +   void (*EndPerfQuery)(struct gl_context *ctx,
> +                        struct gl_perf_query_object *obj);
> +   void (*WaitPerfQuery)(struct gl_context *ctx,
> +                         struct gl_perf_query_object *obj);
> +   GLboolean (*IsPerfQueryReady)(struct gl_context *ctx,
> +                                 struct gl_perf_query_object *obj);
> +   void (*GetPerfQueryData)(struct gl_context *ctx,
> +                            struct gl_perf_query_object *obj,
> +                            GLsizei dataSize,
> +                            GLuint *data,
> +                            GLuint *bytesWritten);
> +   /*@}*/
> +
> +
> +   /**
>      * \name GREMEDY debug/marker functions
>      */
>     /*@{*/
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index f3a24df589..e6cf1f4af6 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1860,6 +1860,23 @@ struct gl_perf_monitor_group
>  
>  
>  /**
> + * A query object instance as described in INTEL_performance_query.
> + *
> + * NB: We want to keep this and the corresponding backend structure
> + * relatively lean considering that applications may expect to
> + * allocate enough objects to be able to query around all draw calls
> + * in a frame.
> + */
> +struct gl_perf_query_object
> +{
> +   GLuint Id;        /**< hash table ID/name */
> +   GLuint Used:1;    /**< has been used for 1 or more queries */
> +   GLuint Active:1;  /**< inside Begin/EndPerfQuery */
> +   GLuint Ready:1;   /**< result is ready? */

Please use "unsigned Id" and "bool Used:1" - we're trying to get away
from GL type aliases when not directly API-facing.

[snip]

> -   /* The specification does not state that this produces an error. */
> +   /* The specification does not state that this produces an error but
> +    * to be consistent with glGetFirstPerfQueryIdINTEL we generate an
> +    * INVALID_VALUE error */

   */ goes on its own line.

With the "mesa: ..." prefix added, patches 1-2 are (weakly):
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170221/ef7f82f8/attachment.sig>