[Mesa-dev] [PATCH v2 06/15] st/mesa: implement GL_AMD_performance_monitor

Martin Peres martin.peres at free.fr
Sun Mar 29 07:59:18 PDT 2015


On 29/03/2015 17:57, Samuel Pitoiset wrote:
>
> On 03/29/2015 11:13 AM, Martin Peres wrote:
>> On 29/03/2015 04:02, Marek Olšák wrote:
>>> On Sat, Mar 28, 2015 at 9:43 PM, Martin Peres <martin.peres at free.fr>
>>> wrote:
>>>> On 22/03/2015 17:35, Samuel Pitoiset wrote:
>>>>> From: Christoph Bumiller <e0425955 at student.tuwien.ac.at>
>>>>>
>>>>> This is based on the original patch of Christoph Bumiller.
>>>>> (source: http://people.freedesktop.org/~chrisbmr/perfmon.diff)
>>>> It would be nice if you could add "v2: Samuel Pitoiset" and tell
>>>> what you
>>>> changed. Christoph may delete his perfmon.diff and no-one will be
>>>> able to
>>>> diff the diffs :)
>>>>
>>>>> As for the Gallium HUD, we keep a list of busy queries in a ring
>>>>> buffer in order to prevent stalls when reading queries.
>>>>>
>>>>> Drivers must implement get_driver_query_group_info and
>>>>> get_driver_query_info in order to enable this extension.
>>>>>
>>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>>> ---
>>>>>     src/mesa/Makefile.sources              |   2 +
>>>>>     src/mesa/state_tracker/st_cb_perfmon.c | 455
>>>>> +++++++++++++++++++++++++++++++++
>>>>>     src/mesa/state_tracker/st_cb_perfmon.h |  70 +++++
>>>>>     src/mesa/state_tracker/st_context.c    |   4 +
>>>>>     src/mesa/state_tracker/st_extensions.c |   3 +
>>>>>     5 files changed, 534 insertions(+)
>>>>>     create mode 100644 src/mesa/state_tracker/st_cb_perfmon.c
>>>>>     create mode 100644 src/mesa/state_tracker/st_cb_perfmon.h
>>>>>
>>>>> diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
>>>>> index 217be9a..e54e618 100644
>>>>> --- a/src/mesa/Makefile.sources
>>>>> +++ b/src/mesa/Makefile.sources
>>>>> @@ -432,6 +432,8 @@ STATETRACKER_FILES = \
>>>>>           state_tracker/st_cb_flush.h \
>>>>>           state_tracker/st_cb_msaa.c \
>>>>>           state_tracker/st_cb_msaa.h \
>>>>> +       state_tracker/st_cb_perfmon.c \
>>>>> +       state_tracker/st_cb_perfmon.h \
>>>>>           state_tracker/st_cb_program.c \
>>>>>           state_tracker/st_cb_program.h \
>>>>>           state_tracker/st_cb_queryobj.c \
>>>>> diff --git a/src/mesa/state_tracker/st_cb_perfmon.c
>>>>> b/src/mesa/state_tracker/st_cb_perfmon.c
>>>>> new file mode 100644
>>>>> index 0000000..fb6774b
>>>>> --- /dev/null
>>>>> +++ b/src/mesa/state_tracker/st_cb_perfmon.c
>>>>> @@ -0,0 +1,455 @@
>>>>> +/*
>>>>> + * Copyright (C) 2013 Christoph Bumiller
>>>>> + * Copyright (C) 2015 Samuel Pitoiset
>>>>> + *
>>>>> + * Permission is hereby granted, free of charge, to any person
>>>>> obtaining
>>>>> a
>>>>> + * copy of this software and associated documentation files (the
>>>>> "Software"),
>>>>> + * to deal in the Software without restriction, including without
>>>>> limitation
>>>>> + * the rights to use, copy, modify, merge, publish, distribute,
>>>>> sublicense,
>>>>> + * and/or sell copies of the Software, and to permit persons to
>>>>> whom the
>>>>> + * Software is furnished to do so, subject to the following
>>>>> conditions:
>>>>> + *
>>>>> + * The above copyright notice and this permission notice shall be
>>>>> included in
>>>>> + * all copies or substantial portions of the Software.
>>>>> + *
>>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>>>> EXPRESS OR
>>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>>>> MERCHANTABILITY,
>>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>>>>> SHALL
>>>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
>>>>> DAMAGES OR
>>>>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>>>>> OTHERWISE,
>>>>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
>>>>> USE OR
>>>>> + * OTHER DEALINGS IN THE SOFTWARE.
>>>>> + */
>>>>> +
>>>>> +/**
>>>>> + * Performance monitoring counters interface to gallium.
>>>>> + */
>>>>> +
>>>>> +#include "st_context.h"
>>>>> +#include "st_cb_bitmap.h"
>>>>> +#include "st_cb_perfmon.h"
>>>>> +
>>>>> +#include "util/bitset.h"
>>>>> +
>>>>> +#include "pipe/p_context.h"
>>>>> +#include "pipe/p_screen.h"
>>>>> +#include "util/u_memory.h"
>>>>> +
>>>>> +/**
>>>>> + * Return a PIPE_QUERY_x type >= PIPE_QUERY_DRIVER_SPECIFIC, or -1 if
>>>>> + * the driver-specific query doesn't exist.
>>>>> + */
>>>>> +static int
>>>>> +find_query_type(struct pipe_screen *screen, const char *name)
>>>>> +{
>>>>> +   int num_queries;
>>>>> +   int type = -1;
>>>>> +   int i;
>>>>> +
>>>>> +   num_queries = screen->get_driver_query_info(screen, 0, NULL);
>>>>> +   if (!num_queries)
>>>>> +      return type;
>>>>> +
>>>>> +   for (i = 0; i < num_queries; i++) {
>>>>> +      struct pipe_driver_query_info info;
>>>>> +
>>>>> +      if (!screen->get_driver_query_info(screen, i, &info))
>>>>> +         continue;
>>>>> +
>>>>> +      if (!strncmp(info.name, name, strlen(name))) {
>>>>> +         type = info.query_type;
>>>>> +         break;
>>>>> +      }
>>>>> +   }
>>>>> +   return type;
>>>>> +}
>>>>> +
>>>>> +static bool
>>>>> +init_perf_monitor(struct gl_context *ctx, struct
>>>>> gl_perf_monitor_object
>>>>> *m)
>>>>> +{
>>>>> +   struct st_perf_monitor_object *stm = st_perf_monitor_object(m);
>>>>> +   struct pipe_screen *screen = st_context(ctx)->pipe->screen;
>>>>> +   struct pipe_context *pipe = st_context(ctx)->pipe;
>>>>> +   int gid, cid;
>>>>> +
>>>>> +   st_flush_bitmap_cache(st_context(ctx));
>>>>> +
>>>>> +   /* Create a query for each active counter. */
>>>>> +   for (gid = 0; gid < ctx->PerfMonitor.NumGroups; gid++) {
>>>>> +      const struct gl_perf_monitor_group *g =
>>>>> &ctx->PerfMonitor.Groups[gid];
>>>>> +      for (cid = 0; cid < g->NumCounters; cid++) {
>>>>> +         const struct gl_perf_monitor_counter *c = &g->Counters[cid];
>>>>> +         struct st_perf_counter_object *cntr;
>>>>> +         int query_type;
>>>>> +
>>>>> +         if (!BITSET_TEST(m->ActiveCounters[gid], cid))
>>>>> +            continue;
>>>> It would seem like the extension would not work with more than 32
>>>> counters
>>>> per group.
>>>>
>>>> This certainly is not a problem on the NVIDIA side but it may become a
>>>> problem for another
>>>> GPU manufacturer. It may warrant a note disclosing this limitation.
>>>> What do
>>>> you think?
>>>>
>>>>> +
>>>>> +         query_type = find_query_type(screen, c->Name);
>>>>> +         assert(query_type != -1);
>>>>> +
>>>>> +         cntr = CALLOC_STRUCT(st_perf_counter_object);
>>>>> +         if (!cntr)
>>>>> +            return false;
>>>>> +
>>>>> +         cntr->queries[cntr->head] = pipe->create_query(pipe,
>>>>> query_type,
>>>>> 0);
>>>>> +         cntr->query_type = query_type;
>>>>> +         cntr->id = cid;
>>>>> +         cntr->group_id = gid;
>>>>> +
>>>>> +         list_addtail(&cntr->list, &stm->active_counters);
>>>>> +      }
>>>>> +   }
>>>>> +   return true;
>>>>> +}
>>>>> +
>>>>> +static void
>>>>> +reset_perf_monitor(struct st_perf_monitor_object *stm,
>>>>> +                   struct pipe_context *pipe)
>>>>> +{
>>>>> +   struct st_perf_counter_object *cntr, *tmp;
>>>>> +   int i;
>>>>> +
>>>>> +   LIST_FOR_EACH_ENTRY_SAFE(cntr, tmp, &stm->active_counters, list) {
>>>>> +      for (i = 0; i < ARRAY_SIZE(cntr->queries); i++) {
>>>>> +         if (cntr->queries[i])
>>>>> +            pipe->destroy_query(pipe, cntr->queries[i]);
>>>>> +      }
>>>>> +      list_del(&cntr->list);
>>>>> +      free(cntr);
>>>>> +   }
>>>>> +}
>>>>> +
>>>>> +static unsigned
>>>>> +read_query_results(struct pipe_context *pipe, GLenum type,
>>>>> +                   struct st_perf_counter_object *cntr,
>>>>> +                   union pipe_query_result *results)
>>>>> +{
>>>>> +   unsigned num_results = 0;
>>>>> +
>>>>> +   while (1) {
>>>>> +      union pipe_query_result result;
>>>>> +
>>>>> +      if (!pipe->get_query_result(pipe, cntr->queries[cntr->tail],
>>>>> +                                  FALSE, &result))
>>>>> +         break;
>>>>> +
>>>>> +      switch (type) {
>>>>> +         case GL_UNSIGNED_INT64_AMD:
>>>>> +            (*results).u64 += result.u64;
>>>>> +            break;
>>>>> +         case GL_UNSIGNED_INT:
>>>>> +            (*results).u32 += result.u32;
>>>>> +            break;
>>>>> +         case GL_FLOAT:
>>>>> +         case GL_PERCENTAGE_AMD:
>>>>> +            (*results).f += result.f;
>>>>> +            break;
>>>>> +      }
>>>>> +      num_results++;
>>>>> +
>>>>> +      if (cntr->tail == cntr->head)
>>>>> +         break;
>>>>> +
>>>>> +      cntr->tail = (cntr->tail + 1) % ST_PERFMON_NUM_QUERIES;
>>>>> +   }
>>>>> +   return num_results;
>>>>> +}
>>>>> +
>>>>> +static struct gl_perf_monitor_object *
>>>>> +st_NewPerfMonitor()
>>>>> +{
>>>>> +   struct st_perf_monitor_object *stq =
>>>>> ST_CALLOC_STRUCT(st_perf_monitor_object);
>>>>> +   if (stq) {
>>>>> +      list_inithead(&stq->active_counters);
>>>>> +      return &stq->base;
>>>>> +   }
>>>>> +   return NULL;
>>>>> +}
>>>>> +
>>>>> +static void
>>>>> +st_DeletePerfMonitor(struct gl_context *ctx, struct
>>>>> gl_perf_monitor_object *m)
>>>>> +{
>>>>> +   struct st_perf_monitor_object *stm = st_perf_monitor_object(m);
>>>>> +   struct pipe_context *pipe = st_context(ctx)->pipe;
>>>>> +
>>>>> +   reset_perf_monitor(stm, pipe);
>>>>> +   FREE(stm);
>>>>> +}
>>>>> +
>>>>> +static GLboolean
>>>>> +st_BeginPerfMonitor(struct gl_context *ctx, struct
>>>>> gl_perf_monitor_object
>>>>> *m)
>>>>> +{
>>>>> +   struct st_perf_monitor_object *stm = st_perf_monitor_object(m);
>>>>> +   struct pipe_context *pipe = st_context(ctx)->pipe;
>>>>> +   struct st_perf_counter_object *cntr;
>>>>> +   int gid;
>>>>> +
>>>>> +   /* Check the number of active counters for each group. */
>>>>> +   for (gid = 0; gid < ctx->PerfMonitor.NumGroups; gid++) {
>>>>> +      const struct gl_perf_monitor_group *g =
>>>>> &ctx->PerfMonitor.Groups[gid];
>>>>> +      if (m->ActiveGroups[gid] > g->MaxActiveCounters) {
>>>>> +         /* Maximum number of counters reached. Cannot start the
>>>>> session.
>>>>> */
>>>>> +         return false;
>>>>> +      }
>>>>> +   }
>>>> I was about to complain that you did not check for errors returned by
>>>> pipe_query, but I guess that with the above test, only the lying
>>>> drivers
>>>> would get impacted. It is going to be hard for Nouveau to allow more
>>>> than 1
>>>> counter at a time because of the B6 ones...
>>>>
>>>>> +
>>>>> +   if (LIST_IS_EMPTY(&stm->active_counters)) {
>>>>> +      /* Create queries for each active counter before starting
>>>>> +       * a new monitoring session. */
>>>>> +      if (!init_perf_monitor(ctx, m))
>>>>> +         goto fail;
>>>>> +   } else {
>>>>> +      /* As for the Gallium HUD, we keep a list of busy queries in
>>>>> a ring
>>>>> +       * buffer in order to prevent stalls when reading queries. */
>>>>> +      LIST_FOR_EACH_ENTRY(cntr, &stm->active_counters, list) {
>>>>> +         if ((cntr->head + 1) % ST_PERFMON_NUM_QUERIES ==
>>>>> cntr->tail) {
>>>>> +            /* All queries are busy, throw away the last query and
>>>>> create
>>>>> +             * a new one. */
>>>>> +            pipe->destroy_query(pipe, cntr->queries[cntr->head]);
>>>>> +            cntr->queries[cntr->head] =
>>>>> +               pipe->create_query(pipe, cntr->query_type, 0);
>>>> Shouldn't you block here, waiting for the oldest query to complete? By
>>>> dropping some queries, you may create a deadlock if the application
>>>> submits
>>>> a ton of queries and then waits for them sequentially.
>>>>
>>>> An easy fix is to make sure that ST_PERFMON_NUM_QUERIES > 2 *
>>>> g->MaxActiveCounters, then, you can simply drop the oldest request.
>>>> Here is
>>>> why:
>>>>
>>>> - The spec forbids nested BeginPerfMonitor calls.
>>>> - EndPerfMonitor make previous counter values impossible to reach.
>>>>
>>>> So, the worst case scenario is the following:
>>>> BeginPerfMonitor(mon=1, ...)
>>>> EndPerfMonitor(mon=1, ...)
>>>> BeginPerfMonitor(mon=1, ...)
>>>> GetPerfMonitorCounterData(mon=1, ...)
>>>>
>>>> Anything that happened before the first BeginPerf here is irrelevant
>>>> and can
>>>> be safely optimised away.
>>>>
>>>> I would thus suggest dynamically allocating this ring buffer and
>>>> dropping
>>>> the oldest requests. To make the code simpler, you could simply always
>>>> select the oldest.
>>>>
>>>> Any thoughts on this?
>>>>
>>>>
>>>> +         } else {
>>>> +            /* Add a new query for this frame. */
>>>> +            cntr->head = (cntr->head + 1) % ST_PERFMON_NUM_QUERIES;
>>>> +            if (!cntr->queries[cntr->head]) {
>>>> +               cntr->queries[cntr->head] =
>>>> +                  pipe->create_query(pipe, cntr->query_type, 0);
>>>> +            }
>>>> +         }
>>>> +      }
>>>> +   }
>>>> +
>>>> +   /* Start the newest query for each active counter. */
>>>> +   LIST_FOR_EACH_ENTRY(cntr, &stm->active_counters, list) {
>>>> +      if (!pipe->begin_query(pipe, cntr->queries[cntr->head]))
>>>> +          goto fail;
>>>> +   }
>>>> +   return true;
>>>> +
>>>> +fail:
>>>> +   /* Failed to start the monitoring session. */
>>>> +   reset_perf_monitor(stm, pipe);
>>>> +   return false;
>>>> +}
>>>> +
>>>> +static void
>>>> +st_EndPerfMonitor(struct gl_context *ctx, struct
>>>> gl_perf_monitor_object *m)
>>>> +{
>>>> +   struct st_perf_monitor_object *stm = st_perf_monitor_object(m);
>>>> +   struct pipe_context *pipe = st_context(ctx)->pipe;
>>>> +   struct st_perf_counter_object *cntr;
>>>> +
>>>> +   /* Stop the newest query for each active counter. */
>>>> +   LIST_FOR_EACH_ENTRY(cntr, &stm->active_counters, list)
>>>> +      pipe->end_query(pipe, cntr->queries[cntr->head]);
>>>> +}
>>>> +
>>>> +static void
>>>> +st_ResetPerfMonitor(struct gl_context *ctx, struct
>>>> gl_perf_monitor_object
>>>> *m)
>>>> +{
>>>> +   struct st_perf_monitor_object *stm = st_perf_monitor_object(m);
>>>> +   struct pipe_context *pipe = st_context(ctx)->pipe;
>>>> +
>>>> +   if (!m->Ended)
>>>> +      st_EndPerfMonitor(ctx, m);
>>>> +
>>>> +   reset_perf_monitor(stm, pipe);
>>>> +
>>>> +   if (m->Active)
>>>> +      st_BeginPerfMonitor(ctx, m);
>>>> +}
>>>> +
>>>> +static GLboolean
>>>> +st_IsPerfMonitorResultAvailable(struct gl_context *ctx,
>>>> +                                struct gl_perf_monitor_object *m)
>>>> +{
>>>> +   struct st_perf_monitor_object *stm = st_perf_monitor_object(m);
>>>> +   struct pipe_context *pipe = st_context(ctx)->pipe;
>>>> +   struct st_perf_counter_object *cntr;
>>>> +
>>>> +   if (LIST_IS_EMPTY(&stm->active_counters))
>>>> +      return false;
>>>> +
>>>> +   /* The result of a monitoring session is only available if the
>>>> oldest
>>>> +    * query of each active counter is idle. */
>>>> +   LIST_FOR_EACH_ENTRY(cntr, &stm->active_counters, list) {
>>>> +      union pipe_query_result result;
>>>> +      if (!pipe->get_query_result(pipe, cntr->queries[cntr->tail],
>>>> +                                  FALSE, &result)) {
>>>> +         /* The oldest query is busy. */
>>>> +         return false;
>>>> +      }
>>>> +   }
>>>> +   return true;
>>>> +}
>>>> +
>>>> +static void
>>>> +st_GetPerfMonitorResult(struct gl_context *ctx,
>>>> +                        struct gl_perf_monitor_object *m,
>>>> +                        GLsizei dataSize,
>>>> +                        GLuint *data,
>>>> +                        GLint *bytesWritten)
>>>> +{
>>>> +   struct st_perf_monitor_object *stm = st_perf_monitor_object(m);
>>>> +   struct pipe_context *pipe = st_context(ctx)->pipe;
>>>> +   struct st_perf_counter_object *cntr;
>>>> +
>>>> +   /* Copy data to the supplied array (data).
>>>> +    *
>>>> +    * The output data format is: <group ID, counter ID, value> for
>>>> each
>>>> +    * active counter. The API allows counters to appear in any order.
>>>> +    */
>>>> +   GLsizei offset = 0;
>>>> +
>>>> +   /* Read query results for each active counter. */
>>>> +   LIST_FOR_EACH_ENTRY(cntr, &stm->active_counters, list) {
>>>> +      union pipe_query_result results = {};
>>>> +      unsigned num_results = 0;
>>>> +      int gid, cid;
>>>> +      GLenum type;
>>>> +
>>>> +      cid  = cntr->id;
>>>> +      gid  = cntr->group_id;
>>>> +      type = ctx->PerfMonitor.Groups[gid].Counters[cid].Type;
>>>> +
>>>> +      num_results = read_query_results(pipe, type, cntr, &results);
>>>> +      if (!num_results)
>>>> +         continue;
>>>>
>>>> It seems like you do not allow the case I presented earlier
>>>> (GetPerfMonitorCounterData after BeginPerfMonitor) unless you rely
>>>> on the
>>>> drivers to do the right thing?
>>>>
>>>>
>>>> +
>>>> +      data[offset++] = gid;
>>>> +      data[offset++] = cid;
>>>> +      switch (type) {
>>>> +      case GL_UNSIGNED_INT64_AMD:
>>>> +         *(uint64_t *)&data[offset] = results.u64 / num_results;
>>>> +         offset += sizeof(uint64_t) / sizeof(GLuint);
>>>> +         break;
>>>> +      case GL_UNSIGNED_INT:
>>>> +         *(uint32_t *)&data[offset] = results.u32 / num_results;
>>>> +         offset += sizeof(uint32_t) / sizeof(GLuint);
>>>> +         break;
>>>> +      case GL_FLOAT:
>>>> +      case GL_PERCENTAGE_AMD:
>>>> +         *(GLfloat *)&data[offset] = results.f / (float)num_results;
>>>> +         offset += sizeof(GLfloat) / sizeof(GLuint);
>>>> +         break;
>>>> +      }
>>>> +   }
>>>> +
>>>> +   if (bytesWritten)
>>>> +      *bytesWritten = offset * sizeof(GLuint);
>>>> +}
>>>> +
>>>> +bool
>>>> +st_init_perfmon(struct st_context *st)
>>>> +{
>>>> +   struct gl_perf_monitor_state *perfmon = &st->ctx->PerfMonitor;
>>>> +   struct pipe_screen *screen = st->pipe->screen;
>>>> +   struct gl_perf_monitor_group *groups = NULL;
>>>> +   int num_counters, num_groups;
>>>> +   int gid, cid;
>>>> +
>>>> +   if (!screen->get_driver_query_info ||
>>>> !screen->get_driver_query_group_info)
>>>> +      return false;
>>>> +
>>>> +   /* Get the number of available queries. */
>>>> +   num_counters = screen->get_driver_query_info(screen, 0, NULL);
>>>> +   if (!num_counters)
>>>> +      return false;
>>>> +
>>>> +   /* Get the number of available groups. */
>>>> +   num_groups = screen->get_driver_query_group_info(screen, 0, NULL);
>>>> +   if (num_groups)
>>>> +      groups = CALLOC(num_groups, sizeof(*groups));
>>>> +   if (!groups)
>>>> +      return false;
>>>> +
>>>> +   for (gid = 0; gid < num_groups; gid++) {
>>>> +      struct gl_perf_monitor_group *g = &groups[gid];
>>>> +      struct pipe_driver_query_group_info group_info;
>>>> +      struct gl_perf_monitor_counter *counters = NULL;
>>>> +
>>>> +      if (!screen->get_driver_query_group_info(screen, gid,
>>>> &group_info))
>>>> +         continue;
>>>> +
>>>> +      g->Name = group_info.name;
>>>> +      g->MaxActiveCounters = group_info.max_active_queries;
>>>> +      g->NumCounters = 0;
>>>> +      g->Counters = NULL;
>>>> +
>>>> +      if (group_info.num_queries)
>>>> +         counters = CALLOC(group_info.num_queries, sizeof(*counters));
>>>> +      if (!counters)
>>>> +         goto fail;
>>>> +
>>>> +      for (cid = 0; cid < num_counters; cid++) {
>>>> +         struct gl_perf_monitor_counter *c =
>>>> &counters[g->NumCounters];
>>>> +         struct pipe_driver_query_info info;
>>>> +
>>>> +         if (!screen->get_driver_query_info(screen, cid, &info))
>>>> +            continue;
>>>> +         if (info.group_id != gid)
>>>> +            continue;
>>>> +
>>>> +         c->Name = info.name;
>>>> +         switch (info.type) {
>>>> +            case PIPE_DRIVER_QUERY_TYPE_UINT64:
>>>> +               c->Minimum.u64 = 0;
>>>> +               c->Maximum.u64 = info.max_value.u64;
>>>> +               c->Type = GL_UNSIGNED_INT64_AMD;
>>>> +               break;
>>>> +            case PIPE_DRIVER_QUERY_TYPE_UINT:
>>>> +               c->Minimum.u32 = 0;
>>>> +               c->Maximum.u32 = info.max_value.u32;
>>>> +               c->Type = GL_UNSIGNED_INT;
>>>> +               break;
>>>> +            case PIPE_DRIVER_QUERY_TYPE_FLOAT:
>>>> +               c->Minimum.f = 0.0;
>>>> +               c->Maximum.f = info.max_value.f;
>>>> +               c->Type = GL_FLOAT;
>>>> +               break;
>>>> +            case PIPE_DRIVER_QUERY_TYPE_PERCENTAGE:
>>>> +               c->Minimum.f = 0.0;
>>>> +               c->Maximum.f = 100.0;
>>>> +               c->Type = GL_PERCENTAGE_AMD;
>>>> +               break;
>>>> +            default:
>>>> +               unreachable("Should never happen: invalid driver query
>>>> type");
>>>>
>>>>
>>>> The definition of "unreachable" should be enough, no need to add
>>>> "Should
>>>> never happen:" in my opinion.
>>>>
>>>>
>>>>> +         }
>>>>> +         g->NumCounters++;
>>>>> +      }
>>>>> +      g->Counters = counters;
>>>>> +   }
>>>>> +
>>>>> +   perfmon->NumGroups = num_groups;
>>>>> +   perfmon->Groups = groups;
>>>>> +   return true;
>>>>> +
>>>>> +fail:
>>>>> +   for (gid = 0; gid < num_groups; gid++)
>>>>> +      FREE((void *)groups[gid].Counters);
>>>>> +   FREE(groups);
>>>>> +   return false;
>>>>> +}
>>>>> +
>>>>> +void
>>>>> +st_destroy_perfmon(struct st_context *st)
>>>>> +{
>>>>> +   struct gl_perf_monitor_state *perfmon = &st->ctx->PerfMonitor;
>>>>> +   int gid;
>>>>> +
>>>>> +   for (gid = 0; gid < perfmon->NumGroups; gid++)
>>>>> +      FREE((void *)perfmon->Groups[gid].Counters);
>>>>> +   FREE((void *)perfmon->Groups);
>>>>> +}
>>>>> +
>>>>> +void st_init_perfmon_functions(struct dd_function_table *functions)
>>>>> +{
>>>>> +   functions->NewPerfMonitor = st_NewPerfMonitor;
>>>>> +   functions->DeletePerfMonitor = st_DeletePerfMonitor;
>>>>> +   functions->BeginPerfMonitor = st_BeginPerfMonitor;
>>>>> +   functions->EndPerfMonitor = st_EndPerfMonitor;
>>>>> +   functions->ResetPerfMonitor = st_ResetPerfMonitor;
>>>>> +   functions->IsPerfMonitorResultAvailable =
>>>>> st_IsPerfMonitorResultAvailable;
>>>>> +   functions->GetPerfMonitorResult = st_GetPerfMonitorResult;
>>>> I don't see SelectPerfMonitorCountersAMD, who defined the entry points?
>>>> Seems like I am missing something here. Are you piggy backing on the
>>>> intel
>>>> implementation?
>>>>
>>>>> +}
>>>>> diff --git a/src/mesa/state_tracker/st_cb_perfmon.h
>>>>> b/src/mesa/state_tracker/st_cb_perfmon.h
>>>>> new file mode 100644
>>>>> index 0000000..ba5bf04
>>>>> --- /dev/null
>>>>> +++ b/src/mesa/state_tracker/st_cb_perfmon.h
>>>>> @@ -0,0 +1,70 @@
>>>>> +/*
>>>>> + * Copyright (C) 2013 Christoph Bumiller
>>>>> + * Copyright (C) 2015 Samuel Pitoiset
>>>>> + *
>>>>> + * Permission is hereby granted, free of charge, to any person
>>>>> obtaining
>>>>> a
>>>>> + * copy of this software and associated documentation files (the
>>>>> "Software"),
>>>>> + * to deal in the Software without restriction, including without
>>>>> limitation
>>>>> + * the rights to use, copy, modify, merge, publish, distribute,
>>>>> sublicense,
>>>>> + * and/or sell copies of the Software, and to permit persons to
>>>>> whom the
>>>>> + * Software is furnished to do so, subject to the following
>>>>> conditions:
>>>>> + *
>>>>> + * The above copyright notice and this permission notice shall be
>>>>> included in
>>>>> + * all copies or substantial portions of the Software.
>>>>> + *
>>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>>>> EXPRESS OR
>>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>>>> MERCHANTABILITY,
>>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>>>>> SHALL
>>>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
>>>>> DAMAGES OR
>>>>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>>>>> OTHERWISE,
>>>>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
>>>>> USE OR
>>>>> + * OTHER DEALINGS IN THE SOFTWARE.
>>>>> + */
>>>>> +
>>>>> +#ifndef ST_CB_PERFMON_H
>>>>> +#define ST_CB_PERFMON_H
>>>>> +
>>>>> +#include "util/u_double_list.h"
>>>>> +
>>>>> +#define ST_PERFMON_NUM_QUERIES 8
>>>>> +
>>>>> +/**
>>>>> + * Subclass of gl_perf_monitor_object
>>>>> + */
>>>>> +struct st_perf_monitor_object
>>>>> +{
>>>>> +   struct gl_perf_monitor_object base;
>>>>> +   struct list_head active_counters;
>>>>> +};
>>>>> +
>>>>> +struct st_perf_counter_object
>>>>> +{
>>>>> +   struct list_head list;
>>>>> +   int query_type;
>>>>> +   int id;
>>>>> +   int group_id;
>>>>> +
>>>>> +   /* Ring of queries. If a query is busy, we use another slot. */
>>>>> +   struct pipe_query *queries[ST_PERFMON_NUM_QUERIES];
>>>>> +   unsigned head, tail;
>>>>> +};
>>>>> +
>>>>> +/**
>>>>> + * Cast wrapper
>>>>> + */
>>>>> +static INLINE struct st_perf_monitor_object *
>>>>> +st_perf_monitor_object(struct gl_perf_monitor_object *q)
>>>>> +{
>>>>> +   return (struct st_perf_monitor_object *)q;
>>>>> +}
>>>>> +
>>>>> +bool
>>>>> +st_init_perfmon(struct st_context *st);
>>>>> +
>>>>> +void
>>>>> +st_destroy_perfmon(struct st_context *st);
>>>>> +
>>>>> +extern void
>>>>> +st_init_perfmon_functions(struct dd_function_table *functions);
>>>>> +
>>>>> +#endif
>>>>> diff --git a/src/mesa/state_tracker/st_context.c
>>>>> b/src/mesa/state_tracker/st_context.c
>>>>> index 5fe132a..2146424 100644
>>>>> --- a/src/mesa/state_tracker/st_context.c
>>>>> +++ b/src/mesa/state_tracker/st_context.c
>>>>> @@ -51,6 +51,7 @@
>>>>>     #include "st_cb_fbo.h"
>>>>>     #include "st_cb_feedback.h"
>>>>>     #include "st_cb_msaa.h"
>>>>> +#include "st_cb_perfmon.h"
>>>>>     #include "st_cb_program.h"
>>>>>     #include "st_cb_queryobj.h"
>>>>>     #include "st_cb_readpixels.h"
>>>>> @@ -116,6 +117,7 @@ st_destroy_context_priv(struct st_context *st)
>>>>>        st_destroy_bitmap(st);
>>>>>        st_destroy_drawpix(st);
>>>>>        st_destroy_drawtex(st);
>>>>> +   st_destroy_perfmon(st);
>>>>>          for (shader = 0; shader < ARRAY_SIZE(st->state.sampler_views);
>>>>> shader++) {
>>>>>           for (i = 0; i < ARRAY_SIZE(st->state.sampler_views[0]);
>>>>> i++) {
>>>>> @@ -190,6 +192,7 @@ st_create_context_priv( struct gl_context *ctx,
>>>>> struct
>>>>> pipe_context *pipe,
>>>>>        st_init_bitmap(st);
>>>>>        st_init_clear(st);
>>>>>        st_init_draw( st );
>>>>> +   st_init_perfmon(st);
>>>> Why is there no checks here on the return value?
>>>>>          /* Choose texture target for glDrawPixels, glBitmap,
>>>>> renderbuffers
>>>>> */
>>>>>        if (pipe->screen->get_param(pipe->screen,
>>>>> PIPE_CAP_NPOT_TEXTURES))
>>>>> @@ -414,6 +417,7 @@ void st_init_driver_functions(struct
>>>>> dd_function_table
>>>>> *functions)
>>>>>        st_init_fbo_functions(functions);
>>>>>        st_init_feedback_functions(functions);
>>>>>        st_init_msaa_functions(functions);
>>>>> +   st_init_perfmon_functions(functions);
>>>>>        st_init_program_functions(functions);
>>>>>        st_init_query_functions(functions);
>>>>>        st_init_cond_render_functions(functions);
>>>>> diff --git a/src/mesa/state_tracker/st_extensions.c
>>>>> b/src/mesa/state_tracker/st_extensions.c
>>>>> index bc20f73..5b6d6c6 100644
>>>>> --- a/src/mesa/state_tracker/st_extensions.c
>>>>> +++ b/src/mesa/state_tracker/st_extensions.c
>>>>> @@ -629,6 +629,9 @@ void st_init_extensions(struct pipe_screen
>>>>> *screen,
>>>>>        extensions->OES_EGL_image_external = GL_TRUE;
>>>>>        extensions->OES_draw_texture = GL_TRUE;
>>>>>     +   if (screen->get_driver_query_info &&
>>>>> screen->get_driver_query_group_info)
>>>>> +      extensions->AMD_performance_monitor = GL_TRUE;
>>>> Isn't this a bit silly to enable this extension if a driver does not
>>>> export
>>>> any group or counter?
>>>> The return value of st_init_perfmon() would be well suited for
>>>> enabling or
>>>> not the extension.
>>>>
>>>> I am OK with the current solution though as driver devs should not
>>>> export
>>>> those functions
>>>> unless they do have counters to expose.
>>> Originally, the functions were used to expose driver statistics,
>>> memory usage, and other variables for the Gallium HUD, so most drivers
>>> implement them, but don't expose any hardware performance counters.
>> Ok, then I have a problem with this because the spec says that the GPU
>> should sample those counters which means that the commands should be
>> emitted in the pushbuffer.
> Mmmh. In that case, we should only expose *hardware* performance
> counters through GL_AMD_performance_monitor, but this requires to add
> flag for GPU/CPU counters.
> What do you think of this?

Yes, like we discussed on IRC, having CPU/GPU flags on the groups would 
be sufficient. AMD perf mon can only expose the GPU counters.


More information about the mesa-dev mailing list