Re: [Mesa-dev] [PATCH 2/3] Model INTEL perf query backend after query object BE
On Wednesday, February 22, 2017 10:35:24 AM PST Robert Bragg wrote:
> On Wed, Feb 22, 2017 at 1:24 AM, Kenneth Graunke
> wrote:
> > On Wednesday, February 15, 2017 1:37:36 PM PST Robert Bragg wrote:
[snip]
> >> 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.
>
> Ah right I was generally aware of that but doing a skimming everything
> with this in mind I found a few other little bits to clean up though I
> ended having some second thoughts about these particular members:
>
> This Id is a record of the GLuint ID given to the application, just
> used for debugging currently. The value is returned by
> _mesa_HashFindFreeKeyBlock() which is currently implemented in terms
> of the GLuint type. One other place where we access the same ID for
> debugging is via _mesa_HashWalk() which takes a callback expecting a
> GLuint argument. I can still change, but when I thought about this it
> felt like it was indeed a directly api facing value.
>
> For the bitfields I started over thinking what it means to have a bool
> bitfield since I doubted whether it could be assumed to be unsigned
> and then wondered about the potential for a bug with some code trying
> to compare a bitfield value == 1, or indexing an array. Does Mesa
> require a c99 compiler, otherwise I don't think it's unheard of for
> bool to end up as a signed int typedef. Anyway, besides the
> overly-pedantic thought, I guessed you wouldn't really mind me using
> "unsigned Used:1;" for the sake of avoiding GLuint. I don't think it
> would make a practical difference since the struct will be naturally
> padded to 8 bytes in all likelyhood either way. I'll prod you on IRC
> to check if you really have a strong opinion here before I push.
We assume bool types become 0 or 1 when cast to integers, as guaranteed
by C99. There are existing examples of bool:1 in NIR, i965, and vc4.
I think it should be OK in Mesa core.
But, feel free to use unsigned. Or GLuint when you have a rationale
for doing so, as above. A lot of people just use GL types everywhere
for no particular reason, but it makes some sense here.
--Ken
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] Model INTEL perf query backend after query object BE
On Wed, Feb 22, 2017 at 1:24 AM, Kenneth Graunke wrote:
> 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: ".
Ah, yup okey.
>
>> 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.
Ok, makes sense now. I've updated the comment to note there's no
equivalent to EverBound needed here.
>
>> 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
>> ---
>> 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
Re: [Mesa-dev] [PATCH 2/3] Model INTEL perf query backend after query object BE
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
> ---
> 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/m
[Mesa-dev] [PATCH 2/3] Model INTEL perf query backend after query object BE
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.
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.
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
---
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
