[Mesa-dev] [PATCH 4/6] mesa: Implement INTEL_performance_query.

Kenneth Graunke kenneth at whitecape.org
Fri May 2 11:18:46 PDT 2014


On 04/23/2014 01:08 AM, Petri Latvala wrote:
> Using the existing driver hooks made for AMD_performance_monitor, implement
> INTEL_performance_query functions.
> 
> v2: Whitespace changes.
> v3: Whitespace changes, add a _mesa_warning()
> 
> Signed-off-by: Petri Latvala <petri.latvala at intel.com>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/mesa/main/performance_monitor.c | 487 ++++++++++++++++++++++++++++++++----
>  1 file changed, 441 insertions(+), 46 deletions(-)
> 
> diff --git a/src/mesa/main/performance_monitor.c b/src/mesa/main/performance_monitor.c
> index 597f633..21b9423 100644
> --- a/src/mesa/main/performance_monitor.c
> +++ b/src/mesa/main/performance_monitor.c
> @@ -137,6 +137,46 @@ get_counter(const struct gl_perf_monitor_group *group_obj, GLuint id)
>     return &group_obj->Counters[id];
>  }
>  
> +/* For INTEL_performance_query, query id 0 is reserved to be invalid. We use
> + * index to Groups array + 1 as the query id. Same applies to counter id.
> + */
> +static inline GLuint
> +queryid_to_index(GLuint queryid)
> +{
> +   return queryid - 1;
> +}
> +
> +static inline GLuint
> +index_to_queryid(GLuint index)
> +{
> +   return index + 1;
> +}
> +
> +static inline bool
> +queryid_valid(const struct gl_context *ctx, GLuint queryid)
> +{
> +   return get_group(ctx, queryid_to_index(queryid)) != NULL;
> +}
> +
> +static inline GLuint
> +counterid_to_index(GLuint counterid)
> +{
> +   return counterid - 1;
> +}
> +
> +static inline GLuint
> +index_to_counterid(GLuint index)
> +{
> +   return index + 1;
> +}
> +
> +static inline bool
> +counterid_valid(const struct gl_perf_monitor_group *group_obj,
> +                GLuint counterid)
> +{
> +   return get_counter(group_obj, counterid_to_index(counterid)) != NULL;
> +}
> +
>  /*****************************************************************************/
>  
>  void GLAPIENTRY
> @@ -644,6 +684,7 @@ extern void GLAPIENTRY
>  _mesa_GetFirstPerfQueryIdINTEL(GLuint *queryId)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> +   unsigned numGroups;
>  
>     /* The GL_INTEL_performance_query spec says:
>      *
> @@ -655,16 +696,22 @@ _mesa_GetFirstPerfQueryIdINTEL(GLuint *queryId)
>        return;
>     }
>  
> +   numGroups = ctx->PerfMonitor.NumGroups;
> +
>     /* The GL_INTEL_performance_query spec says:
>      *
>      *    "If the given hardware platform doesn't support any performance
>      *    queries, then the value of 0 is returned and INVALID_OPERATION error
>      *    is raised."
>      */
> +   if (numGroups == 0) {
> +      *queryId = 0;
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "glGetFirstPerfQueryIdINTEL(no queries supported)");
> +      return;
> +   }
>  
> -   *queryId = 0;
> -   _mesa_error(ctx, GL_INVALID_OPERATION,
> -               "glGetFirstPerfQueryIdINTEL(no queries supported)");
> +   *queryId = index_to_queryid(0);
>  }
>  
>  extern void GLAPIENTRY
> @@ -674,40 +721,66 @@ _mesa_GetNextPerfQueryIdINTEL(GLuint queryId, GLuint *nextQueryId)
>  
>     /* The GL_INTEL_performance_query spec says:
>      *
> -    *    "If nextQueryId pointer is equal to 0, an INVALID_VALUE error is
> -    *    generated."
> +    *    "The result is passed in location pointed by nextQueryId. If query
> +    *    identified by queryId is the last query available the value of 0 is
> +    *    returned. If the specified performance query identifier is invalid
> +    *    then INVALID_VALUE error is generated. If nextQueryId pointer is
> +    *    equal to 0, an INVALID_VALUE error is generated.  Whenever error is
> +    *    generated, the value of 0 is returned."
>      */
> +
>     if (!nextQueryId) {
>        _mesa_error(ctx, GL_INVALID_VALUE,
>                    "glGetNextPerfQueryIdINTEL(nextQueryId == NULL)");
>        return;
>     }
>  
> -   /* The GL_INTEL_performance_query spec says:
> -    *
> -    *    "If the specified performance query identifier is invalid then
> -    *    INVALID_VALUE error is generated. Whenever error is generated, the
> -    *    value of 0 is returned."
> -    *
> -    * No queries are supported, so all queries are invalid.
> -    */
> -   *nextQueryId = 0;
> -   _mesa_error(ctx, GL_INVALID_VALUE,
> -               "glGetNextPerfQueryIdINTEL(invalid query)");
> +   if (!queryid_valid(ctx, queryId)) {
> +      *nextQueryId = 0;
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glGetNextPerfQueryIdINTEL(invalid query)");
> +      return;
> +   }
> +
> +   ++queryId;
> +
> +   if (!queryid_valid(ctx, queryId)) {
> +      *nextQueryId = 0;
> +   } else {
> +      *nextQueryId = queryId;
> +   }
>  }
>  
>  extern void GLAPIENTRY
>  _mesa_GetPerfQueryIdByNameINTEL(char *queryName, GLuint *queryId)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> +   unsigned i;
>  
>     /* The GL_INTEL_performance_query spec says:
>      *
>      *    "If queryName does not reference a valid query name, an INVALID_VALUE
>      *    error is generated."
> -    *
> -    * No queries are supported, so all query names are invalid.
>      */
> +   if (!queryName) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glGetPerfQueryIdByNameINTEL(queryName == NULL)");
> +      return;
> +   }
> +
> +   /* The specification does not state that this produces an error. */
> +   if (!queryId) {
> +      _mesa_warning(ctx, "glGetPerfQueryIdByNameINTEL(queryId == NULL)");
> +      return;

I think raising INVALID_VALUE would be reasonable, though.  It does so
in basically every other case, so I'm guessing this is more of an
oversight when writing the spec than an intentional decision.

> +   }
> +
> +   for (i = 0; i < ctx->PerfMonitor.NumGroups; ++i) {
> +      const struct gl_perf_monitor_group *group_obj = get_group(ctx, i);
> +      if (strcmp(group_obj->Name, queryName) == 0) {
> +         *queryId = index_to_queryid(i);
> +         return;
> +      }
> +   }
>  
>     _mesa_error(ctx, GL_INVALID_VALUE,
>                 "glGetPerfQueryIdByNameINTEL(invalid query name)");
> @@ -721,17 +794,69 @@ _mesa_GetPerfQueryInfoINTEL(GLuint queryId,
>                              GLuint *capsMask)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> +   unsigned i;
> +
> +   const struct gl_perf_monitor_group *group_obj =
> +      get_group(ctx, queryid_to_index(queryId));
> +
> +   if (group_obj == NULL) {
> +      /* The GL_INTEL_performance_query spec says:
> +       *
> +       *    "If queryId does not reference a valid query type, an
> +       *    INVALID_VALUE error is generated."
> +       */
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glGetPerfQueryInfoINTEL(invalid query)");
> +      return;
> +   }
> +
> +   if (queryName) {
> +      strncpy(queryName, group_obj->Name, queryNameLength);
> +
> +      /* No specification given about whether the string needs to be
> +       * zero-terminated. Zero-terminate the string always as we don't
> +       * otherwise communicate the length of the returned string.
> +       */
> +      if (queryNameLength > 0) {
> +         queryName[queryNameLength - 1] = '\0';
> +      }
> +   }
> +
> +   if (dataSize) {
> +      unsigned size = 0;
> +      for (i = 0; i < group_obj->NumCounters; ++i) {
> +         /* What we get from the driver is group id (uint32_t) + counter id
> +          * (uint32_t) + value.
> +          */
> +         size += 2 * sizeof(uint32_t) + _mesa_perf_monitor_counter_size(&group_obj->Counters[i]);
> +      }
> +      *dataSize = size;
> +   }
> +
> +   if (noCounters) {
> +      *noCounters = group_obj->NumCounters;
> +   }
>  
>     /* The GL_INTEL_performance_query spec says:
>      *
> -    *    "If queryId does not reference a valid query type, an INVALID_VALUE
> -    *    error is generated."
> +    *    "-- the actual number of already created query instances in
> +    *    maxInstances location"
>      *
> -    * No queries are supported, so all queries are invalid.
> +    * 1) Typo in the specification, should be noActiveInstances.
> +    * 2) Another typo in the specification, maxInstances parameter is not listed
> +    *    in the declaration of this function in the list of new functions.
>      */

Have you followed up to see if maxInstances is meant to exist or not?
It would be pretty awful to ship this and then go "wait, the function
signature was wrong..."

If it is meant to exist, we should probably just set it to UINT_MAX.

> +   if (noActiveInstances) {
> +      *noActiveInstances = _mesa_HashNumEntries(ctx->PerfMonitor.Monitors);
> +   }

I'm unclear whether this is the intent.  Do they mean the number of all
queries active in the system, or the number of things actively querying
this queryId?  If it's the latter, you have more work to do here.

>  
> -   _mesa_error(ctx, GL_INVALID_VALUE,
> -               "glGetPerfQueryInfoINTEL(invalid query)");
> +   if (capsMask) {
> +      /* TODO: This information not yet available in the monitor structs. For
> +       * now, we hardcode SINGLE_CONTEXT, since that's what the implementation
> +       * currently tries very hard to do.
> +       */
> +      *capsMask = GL_PERFQUERY_SINGLE_CONTEXT_INTEL;
> +   }
>  }
>  
>  extern void GLAPIENTRY
> @@ -743,73 +868,281 @@ _mesa_GetPerfCounterInfoINTEL(GLuint queryId, GLuint counterId,
>  {
>     GET_CURRENT_CONTEXT(ctx);
>  
> +   const struct gl_perf_monitor_group *group_obj;
> +   const struct gl_perf_monitor_counter *counter_obj;
> +   unsigned counterIndex;
> +   unsigned i;
> +
> +   group_obj = get_group(ctx, queryid_to_index(queryId));
> +
>     /* The GL_INTEL_performance_query spec says:
>      *
>      *    "If the pair of queryId and counterId does not reference a valid
>      *    counter, an INVALID_VALUE error is generated."
> -    *
> -    * No queries are supported, so all queries are invalid.
>      */
> +   if (group_obj == NULL) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glGetPerfCounterInfoINTEL(invalid queryId)");
> +      return;
> +   }
>  
> -   _mesa_error(ctx, GL_INVALID_VALUE,
> -               "glGetPerfCounterInfoINTEL(invalid counterId)");
> +   counterIndex = counterid_to_index(counterId);
> +   counter_obj = get_counter(group_obj, counterIndex);
> +
> +   if (counter_obj == NULL) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glGetPerfCounterInfoINTEL(invalid counterId)");
> +      return;
> +   }
> +
> +   if (counterName) {
> +      strncpy(counterName, counter_obj->Name, counterNameLength);
> +
> +      /* No specification given about whether the string needs to be
> +       * zero-terminated. Zero-terminate the string always as we don't
> +       * otherwise communicate the length of the returned string.
> +       */
> +      if (counterNameLength > 0) {
> +         counterName[counterNameLength - 1] = '\0';
> +      }
> +   }
> +
> +   if (counterDesc) {
> +      /* TODO: No separate description text at the moment. We pass the name
> +       * again for the moment.
> +       */
> +      strncpy(counterDesc, counter_obj->Name, counterDescLength);

Seems like a good plan for now.  It'd be great to add real descriptions
at some point---I wanted to extend the AMD interface to have those too.
 There are descriptions for most counters in
intel-gpu-tools/tools/intel_perf_counters.c.

> +
> +      /* No specification given about whether the string needs to be
> +       * zero-terminated. Zero-terminate the string always as we don't
> +       * otherwise communicate the length of the returned string.
> +       */
> +      if (counterDescLength > 0) {
> +         counterDesc[counterDescLength - 1] = '\0';
> +      }
> +   }
> +
> +   if (counterOffset) {
> +      unsigned offset = 0;
> +      for (i = 0; i < counterIndex; ++i) {
> +         /* What we get from the driver is group id (uint32_t) + counter id
> +          * (uint32_t) + value.
> +          */
> +         offset += 2 * sizeof(uint32_t) + _mesa_perf_monitor_counter_size(&group_obj->Counters[i]);
> +      }
> +      *counterOffset = 2 * sizeof(uint32_t) + offset;
> +   }
> +
> +   if (counterDataSize) {
> +      *counterDataSize = _mesa_perf_monitor_counter_size(counter_obj);
> +   }
> +
> +   if (counterTypeEnum) {
> +      /* TODO: Different counter types (semantic type, not data type) not
> +       * supported as of yet.
> +       */
> +      *counterTypeEnum = GL_PERFQUERY_COUNTER_RAW_INTEL;
> +   }
> +
> +   if (counterDataTypeEnum) {
> +      switch (counter_obj->Type) {
> +      case GL_FLOAT:
> +      case GL_PERCENTAGE_AMD:
> +         *counterDataTypeEnum = GL_PERFQUERY_COUNTER_DATA_FLOAT_INTEL;
> +         break;
> +      case GL_UNSIGNED_INT:
> +         *counterDataTypeEnum = GL_PERFQUERY_COUNTER_DATA_UINT32_INTEL;
> +         break;
> +      case GL_UNSIGNED_INT64_AMD:
> +         *counterDataTypeEnum = GL_PERFQUERY_COUNTER_DATA_UINT64_INTEL;
> +         break;
> +      default:
> +         assert(!"Should not get here: invalid counter type");
> +         return;
> +      }
> +   }
> +
> +   if (rawCounterMaxValue) {
> +      /* This value is (implicitly) specified to be used only with
> +       * GL_PERFQUERY_COUNTER_RAW_INTEL counters. When semantic types for
> +       * counters are added, that needs to be checked.
> +       */
> +
> +      /* The GL_INTEL_performance_query spec says:
> +       *
> +       *    "for some raw counters for which the maximal value is
> +       *    deterministic, the maximal value of the counter in 1 second is
> +       *    returned in the location pointed by rawCounterMaxValue, otherwise,
> +       *    the location is written with the value of 0."
> +       *
> +       * The maximum value reported by the driver at the moment is not with
> +       * these semantics, so write 0 always to be safe.
> +       */
> +      *rawCounterMaxValue = 0;
> +   }
>  }
>  
>  extern void GLAPIENTRY
>  _mesa_CreatePerfQueryINTEL(GLuint queryId, GLuint *queryHandle)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> +   GLuint first;
> +   GLuint group;
> +   const struct gl_perf_monitor_group *group_obj;
> +   struct gl_perf_monitor_object *m;
> +   unsigned i;
> +
> +   /* This is not specified in the extension, but is the only sane thing to
> +    * do.
> +    */
> +   if (queryHandle == NULL) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glCreatePerfQueryINTEL(queryHandle == NULL)");
> +      return;
> +   }
> +
> +   group = queryid_to_index(queryId);
> +   group_obj = get_group(ctx, group);
>  
>     /* The GL_INTEL_performance_query spec says:
>      *
>      *    "If queryId does not reference a valid query type, an INVALID_VALUE
>      *    error is generated."
> -    *
> -    * No queries are supported, so all queries are invalid.
>      */
> +   if (group_obj == NULL) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glCreatePerfQueryINTEL(invalid queryId)");
> +      return;
> +   }
>  
> -   _mesa_error(ctx, GL_INVALID_VALUE,
> -               "glCreatePerfQueryINTEL(invalid queryId)");
> +   /* The query object created here is the counterpart of a `monitor' in
> +    * AMD_performance_monitor. This call is equivalent to calling
> +    * GenPerfMonitorsAMD and SelectPerfMonitorCountersAMD with a list of all
> +    * counters in a group.
> +    */
> +
> +   /* We keep the monitor ids contiguous */

You find a contiguous set of...1...thing? :)

> +   first = _mesa_HashFindFreeKeyBlock(ctx->PerfMonitor.Monitors, 1);
> +   if (!first) {
> +      /* The GL_INTEL_performance_query spec says:
> +       *
> +       *    "If the query instance cannot be created due to exceeding the
> +       *    number of allowed instances or driver fails query creation due to
> +       *    an insufficient memory reason, an OUT_OF_MEMORY error is
> +       *    generated, and the location pointed by queryHandle returns NULL."
> +      */
> +      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glCreatePerfQueryINTEL");
> +      return;
> +   }
> +
> +   m = new_performance_monitor(ctx, first);

You need to check for GL_OUT_OF_MEMORY here, but...

> +   _mesa_HashInsert(ctx->PerfMonitor.Monitors, first, m);

I don't see much point in copy and pasting all of this code.  You could
replace this whole block with:

   _mesa_GenPerfMonitorsAMD(1, &m);

which is much easier :)  Would you mind changing that in a follow-up patch?

> +   *queryHandle = first;
> +
> +   ctx->Driver.ResetPerfMonitor(ctx, m);
> +
> +   for (i = 0; i < group_obj->NumCounters; ++i) {
> +      ++m->ActiveGroups[group];
> +      /* Counters are a continuous range of integers, 0 to NumCounters (excl),
> +       * so i is the counter value to use here.
> +       */
> +      BITSET_SET(m->ActiveCounters[group], i);
> +   }

However, I do approve of you duplicating this part of the code - it's
much simpler than trying to use _mesa_SelectPerfMonitorCountersAMD.

>  }
>  
>  extern void GLAPIENTRY
>  _mesa_DeletePerfQueryINTEL(GLuint queryHandle)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> +   struct gl_perf_monitor_object *m;
> +
> +   /* The queryHandle is the counterpart to AMD_performance_monitor's monitor
> +    * id.
> +    */
> +   m = lookup_monitor(ctx, queryHandle);
>  
>     /* The GL_INTEL_performance_query spec says:
>      *
>      *    "If a query handle doesn't reference a previously created performance
>      *    query instance, an INVALID_VALUE error is generated."
> -    *
> -    * No queries are supported, so all queries are invalid.
>      */
> +   if (m == NULL) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glDeletePerfQueryINTEL(invalid queryHandle)");
> +      return;
> +   }
>  
> -   _mesa_error(ctx, GL_INVALID_VALUE,
> -               "glDeletePerfQueryINTEL(invalid queryHandle)");
> +   /* Let the driver stop the monitor if it's active. */
> +   if (m->Active) {
> +      ctx->Driver.ResetPerfMonitor(ctx, m);
> +      m->Ended = false;
> +   }
> +
> +   _mesa_HashRemove(ctx->PerfMonitor.Monitors, queryHandle);
> +   ralloc_free(m->ActiveGroups);
> +   ralloc_free(m->ActiveCounters);
> +   ctx->Driver.DeletePerfMonitor(ctx, m);
>  }
>  
>  extern void GLAPIENTRY
>  _mesa_BeginPerfQueryINTEL(GLuint queryHandle)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> +   struct gl_perf_monitor_object *m;
> +
> +   /* The queryHandle is the counterpart to AMD_performance_monitor's monitor
> +    * id.
> +    */
> +
> +   m = lookup_monitor(ctx, queryHandle);
>  
>     /* The GL_INTEL_performance_query spec says:
>      *
>      *    "If a query handle doesn't reference a previously created performance
>      *    query instance, an INVALID_VALUE error is generated."
> +    */
> +   if (m == NULL) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glBeginPerfQueryINTEL(invalid queryHandle)");
> +      return;
> +   }
> +
> +   /* The GL_INTEL_performance_query spec says:
>      *
> -    * No queries are supported, so all queries are invalid.
> +    *    "Note that some query types, they cannot be collected in the same
> +    *    time. Therefore calls of BeginPerfQueryINTEL() cannot be nested if
> +    *    they refer to queries of such different types. In such case
> +    *    INVALID_OPERATION error is generated."
> +    *
> +    * We also generate an INVALID_OPERATION error if the driver can't begin
> +    * monitoring for its own reasons, and for nesting the same query.
>      */
> +   if (m->Active) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "glBeginPerfQueryINTEL(already active)");
> +      return;
> +   }
>  
> -   _mesa_error(ctx, GL_INVALID_VALUE,
> -               "glBeginPerfQueryINTEL(invalid queryHandle)");
> +   if (ctx->Driver.BeginPerfMonitor(ctx, m)) {
> +      m->Active = true;
> +      m->Ended = false;
> +   } else {
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "glBeginPerfQueryINTEL(driver unable to begin monitoring)");
> +   }
>  }
>  
>  extern void GLAPIENTRY
>  _mesa_EndPerfQueryINTEL(GLuint queryHandle)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> +   struct gl_perf_monitor_object *m;
> +
> +   /* The queryHandle is the counterpart to AMD_performance_monitor's monitor
> +    * id.
> +    */
> +
> +   m = lookup_monitor(ctx, queryHandle);
>  
>     /* The GL_INTEL_performance_query spec says:
>      *
> @@ -818,13 +1151,24 @@ _mesa_EndPerfQueryINTEL(GLuint queryHandle)
>      *
>      * The specification doesn't state that an invalid handle would be an
>      * INVALID_VALUE error. Regardless, query for such a handle will not be
> -    * started, so we generate an INVALID_OPERATION in that case.
> -    *
> -    * No queries are supported, so all handles are invalid.
> +    * started, so we generate an INVALID_OPERATION in that case too.
>      */
> +   if (m == NULL) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "glEndPerfQueryINTEL(invalid queryHandle)");
> +      return;
> +   }
>  
> -   _mesa_error(ctx, GL_INVALID_OPERATION,
> -               "glEndPerfQueryINTEL(query not started)");
> +   if (!m->Active) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "glEndPerfQueryINTEL(not active)");
> +      return;
> +   }
> +
> +   ctx->Driver.EndPerfMonitor(ctx, m);
> +
> +   m->Active = false;
> +   m->Ended = true;
>  }
>  
>  extern void GLAPIENTRY
> @@ -832,6 +1176,8 @@ _mesa_GetPerfQueryDataINTEL(GLuint queryHandle, GLuint flags,
>                              GLsizei dataSize, void *data, GLuint *bytesWritten)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> +   struct gl_perf_monitor_object *m;
> +   bool result_available;
>  
>     /* The GL_INTEL_performance_query spec says:
>      *
> @@ -844,6 +1190,12 @@ _mesa_GetPerfQueryDataINTEL(GLuint queryHandle, GLuint flags,
>        return;
>     }
>  
> +   /* The queryHandle is the counterpart to AMD_performance_monitor's monitor
> +    * id.
> +    */
> +
> +   m = lookup_monitor(ctx, queryHandle);
> +
>     /* The specification doesn't state that an invalid handle generates an
>      * error. We could interpret that to mean the case should be handled as
>      * "measurement not ready for this query", but what should be done if
> @@ -851,10 +1203,53 @@ _mesa_GetPerfQueryDataINTEL(GLuint queryHandle, GLuint flags,
>      *
>      * To resolve this, we just generate an INVALID_VALUE from an invalid query
>      * handle.
> +    */
> +   if (m == NULL) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glGetPerfQueryDataINTEL(invalid queryHandle)");
> +      return;
> +   }
> +
> +   /* We need at least enough room for a single value. */
> +   if (dataSize < sizeof(GLuint)) {
> +      *bytesWritten = 0;
> +      return;
> +   }
> +
> +   /* The GL_INTEL_performance_query spec says:
>      *
> -    * No queries are supported, so all handles are invalid.
> +    *    "The call may end without returning any data if they are not ready
> +    *    for reading as the measurement session is still pending (the
> +    *    EndPerfQueryINTEL() command processing is not finished by
> +    *    hardware). In this case location pointed by the bytesWritten
> +    *    parameter will be set to 0."
> +    *
> +    * If EndPerfQueryINTEL() is not called at all, we follow this.
>      */
> +   if (!m->Ended) {
> +      *bytesWritten = 0;
> +      return;
> +   }
>  
> -   _mesa_error(ctx, GL_INVALID_VALUE,
> -               "glGetPerfQueryDataINTEL(invalid queryHandle)");
> +   result_available = ctx->Driver.IsPerfMonitorResultAvailable(ctx, m);
> +
> +   if (!result_available) {
> +      if (flags == GL_PERFQUERY_FLUSH_INTEL) {
> +         ctx->Driver.Flush(ctx);
> +      } else if (flags == GL_PERFQUERY_WAIT_INTEL) {
> +         /* Assume Finish() is both enough and not too much to wait for
> +          * results. If results are still not available after Finish(), the
> +          * later code automatically bails out with 0 for bytesWritten.
> +          */
> +         ctx->Driver.Finish(ctx);
> +         result_available =
> +            ctx->Driver.IsPerfMonitorResultAvailable(ctx, m);

I was going to suggest creating a new WaitForPerfMonitorResult driver
hook, but all that would do is flush the batch and wait until
oa_bo/pipeline_stats_bo weren't busy...which seems equivalent to just
wait_rendering on the batch...which is what Finish does.

So, I think your plan is fine.

> +      }
> +   }
> +
> +   if (result_available) {
> +      ctx->Driver.GetPerfMonitorResult(ctx, m, dataSize, data, (GLint*)bytesWritten);
> +   } else {
> +      *bytesWritten = 0;
> +   }
>  }
> 

I see Ian pushed these about an hour ago, but they would have earned a:

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Nice work on this series.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140502/3091f089/attachment-0001.sig>


More information about the mesa-dev mailing list