[Mesa-dev] [PATCH 2/3] st/mesa: delay initialization of performance counters

[Nicolai Hähnle]

On 25.11.2015 14:11, Samuel Pitoiset wrote:
> One comment below.
>
> Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>
> On 11/25/2015 01:00 PM, Nicolai Hähnle wrote:
>> ---
>>   src/mesa/state_tracker/st_cb_perfmon.c | 28
>> +++++++++++++++++-----------
>>   src/mesa/state_tracker/st_cb_perfmon.h |  2 +-
>>   src/mesa/state_tracker/st_context.c    |  2 +-
>>   3 files changed, 19 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_cb_perfmon.c
>> b/src/mesa/state_tracker/st_cb_perfmon.c
>> index 8fdf0e8..cd7fdc0 100644
>> --- a/src/mesa/state_tracker/st_cb_perfmon.c
>> +++ b/src/mesa/state_tracker/st_cb_perfmon.c
>> @@ -329,8 +329,20 @@ st_GetPerfMonitorResult(struct gl_context *ctx,
>>
>>
>>   bool
>> -st_init_perfmon(struct st_context *st)
>> +st_have_perfmon(struct st_context *st)
>>   {
>> +   struct pipe_screen *screen = st->pipe->screen;
>> +
>> +   if (!screen->get_driver_query_info ||
>> !screen->get_driver_query_group_info)
>> +      return false;
>> +
>> +   return screen->get_driver_query_group_info(screen, 0, NULL) != 0;
>> +}
>> +
>> +static void
>> +st_InitPerfMonitorGroups(struct gl_context *ctx)
>> +{
>> +   struct st_context *st = st_context(ctx);
>>      struct gl_perf_monitor_state *perfmon = &st->ctx->PerfMonitor;
>>      struct pipe_screen *screen = st->pipe->screen;
>>      struct gl_perf_monitor_group *groups = NULL;
>> @@ -338,20 +350,14 @@ st_init_perfmon(struct st_context *st)
>>      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));
>> +   groups = CALLOC(num_groups, sizeof(*groups));
>>      if (!groups)
>> -      return false;
>> +      return;
>>
>>      stgroups = CALLOC(num_groups, sizeof(*stgroups));
>>      if (!stgroups)
>> @@ -432,7 +438,7 @@ st_init_perfmon(struct st_context *st)
>>      perfmon->Groups = groups;
>>      st->perfmon = stgroups;
>>
>> -   return true;
>> +   return;
>>
>>   fail:
>>      for (gid = 0; gid < num_groups; gid++) {
>> @@ -442,7 +448,6 @@ fail:
>>      FREE(stgroups);
>>   fail_only_groups:
>>      FREE(groups);
>> -   return false;
>>   }
>>
>>   void
>> @@ -461,6 +466,7 @@ st_destroy_perfmon(struct st_context *st)
>>
>>   void st_init_perfmon_functions(struct dd_function_table *functions)
>>   {
>> +   functions->InitPerfMonitorGroups = st_InitPerfMonitorGroups;
>>      functions->NewPerfMonitor = st_NewPerfMonitor;
>>      functions->DeletePerfMonitor = st_DeletePerfMonitor;
>>      functions->BeginPerfMonitor = st_BeginPerfMonitor;
>> diff --git a/src/mesa/state_tracker/st_cb_perfmon.h
>> b/src/mesa/state_tracker/st_cb_perfmon.h
>> index 2973286..98202f2 100644
>> --- a/src/mesa/state_tracker/st_cb_perfmon.h
>> +++ b/src/mesa/state_tracker/st_cb_perfmon.h
>> @@ -73,7 +73,7 @@ st_perf_monitor_object(struct gl_perf_monitor_object
>> *q)
>>   }
>>
>>   bool
>> -st_init_perfmon(struct st_context *st);
>> +st_have_perfmon(struct st_context *st);
>>
>>   void
>>   st_destroy_perfmon(struct st_context *st);
>> diff --git a/src/mesa/state_tracker/st_context.c
>> b/src/mesa/state_tracker/st_context.c
>> index 6e20fd1..eea3de0 100644
>> --- a/src/mesa/state_tracker/st_context.c
>> +++ b/src/mesa/state_tracker/st_context.c
>> @@ -255,7 +255,7 @@ st_create_context_priv( struct gl_context *ctx,
>> struct pipe_context *pipe,
>>      st_init_extensions(st->pipe->screen, &ctx->Const,
>>                         &ctx->Extensions, &st->options, ctx->Mesa_DXTn);
>>
>> -   if (st_init_perfmon(st)) {
>> +   if (st_have_perfmon(st)) {
>>         /* GL_AMD_performance_monitor is only enabled when the underlying
>>          * driver expose GPU hardware performance counters. */
>
> Mmmh, we should drop that comment since you removed that notion of
> GPU/CPU counters.

Good point. I'll put this in a separate patch, since it's somewhat 
unrelated. Thanks for the review!

Cheers,
Nicolai

>>         ctx->Extensions.AMD_performance_monitor = GL_TRUE;
>>
>