On 03/30/2015 11:17 PM, Marek Olšák wrote:
You can add a flag to each driver query identifying what kind of query
it is (a hw perf counter or a CPU-only query). Then you can enumerate
all queries and see if there's at least one perf counter and if so,
advertise the extension.

Or add a CAP and let drivers decide if they want the extension or not.
You still need the per-query flag to skip the non-hw queries though.

I would prefer to add this GPU/CPU flags on groups instead of per-query because it seems like much simpler to define groups of both GPU and CPU counters. Currently, I think only nvc0 driver exposes GPU hardware performance counters So, I removed the patches for svga, freedreno and radeon drivers in my series.

In my opinion, other drivers which want to enable GL_AMD_perfmon extension should *explicitly* define groups of GPU counters.
Currently, that flag is called PIPE_DRIVER_QUERY_GROUP_TYPE_{CPU,GPU}.

If you want to have a quick look, the code is located under the gl_amd_perfmon_v3 branch.
http://cgit.freedesktop.org/~hakzsam/mesa/log/?h=gl_amd_perfmon_v3

Let me know if you have strong objections against it.


Marek

On Mon, Mar 30, 2015 at 3:06 PM, Samuel Pitoiset
<samuel.pitoi...@gmail.com> wrote:

On 03/29/2015 06:11 PM, Martin Peres wrote:
On 29/03/2015 17:56, Samuel Pitoiset wrote:

On 03/28/2015 09:43 PM, Martin Peres wrote:
On 22/03/2015 17:35, Samuel Pitoiset wrote:
From: Christoph Bumiller <e0425...@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 :)
Good idea!

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.pitoi...@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?
Ok, I'll add a note for that.

+
+         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...
This extension has not been designed for NVIDIA hardware. :-)
But yes, it will not really possible to monitor more than 1 counter per
group for "special" counting modes like B4/B6.

As discussed on IRC, it will still be possible, but it will be more
annoying for applications. We can discuss about this later when the core
gallium code is in place :)

+
+   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:
Yeah, this was a bit insane, I'll make the changes and try to avoid that
possible deadlock.

Wonderful!

- 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, ...)
No, this scenario is not possible.
This is actually not clearly specified by the spec but the "core
support" in mesa doesn't allow to read counters data while a monitoring
sessions is active, and this makes sense.

In this case, you only need to have ST_PERFMON_NUM_QUERIES ==
g->MaxActiveCounters

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?
See above.

+
+      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.
Sure.

+         }
+         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?
SelectPerfMonitorCountersAMD is defined in
src/mesa/main/performance_monitor.c

Ok, I will have a look at it later. Thanks.

+}
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?
Good catch!

         /* 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.
I don't think this is silly because there is no point to define those
functions if the driver doesn't expose any counters.

You mean it would be silly for the driver dev to export those functions if
they had not counters? Yes, I guess you are right.

Well, actually it's a bad solution to expose this extension like that
because amd perfmon must only expose hw performance counters.
The good one is to enable it *only* if the driver expose GPU counters.



+
       /* Expose the extensions which directly correspond to gallium
caps. */
       for (i = 0; i < ARRAY_SIZE(cap_mapping); i++) {
          if (screen->get_param(screen, cap_mapping[i].cap)) {
Great work Samuel! 1-5 are R-b: Martin Peres <martin.pe...@free.fr>

As for this patch, I will hold my R-b until we clear this query
management issues.
Thanks a lot for the review.

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to