[Mesa-dev] [PATCH 3/3] etnaviv: shader-db traces

[Lucas Stach]

Am Mittwoch, den 08.02.2017, 12:10 +0100 schrieb Christian Gmeiner:
> Signed-off-by: Christian Gmeiner <christian.gmeiner at gmail.com>
> ---
>  src/gallium/drivers/etnaviv/etnaviv_compiler.h |  2 ++
>  src/gallium/drivers/etnaviv/etnaviv_debug.h    |  1 +
>  src/gallium/drivers/etnaviv/etnaviv_screen.c   |  1 +
>  src/gallium/drivers/etnaviv/etnaviv_shader.c   | 44 +++++++++++++++++++++++++-
>  4 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler.h b/src/gallium/drivers/etnaviv/etnaviv_compiler.h
> index 211ae1a..de3e20d 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_compiler.h
> +++ b/src/gallium/drivers/etnaviv/etnaviv_compiler.h
> @@ -56,6 +56,8 @@ struct etna_shader_io_file {
>  
>  /* shader object, for linking */
>  struct etna_shader {
> +   uint32_t id; /* for debug */

Do you need this? It can certainly be removed from this patch, but I
don't know if you have other stuff building on top of this.

>     uint processor; /* TGSI_PROCESSOR_... */
>     uint32_t code_size; /* code size in uint32 words */
>     uint32_t *code;
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_debug.h b/src/gallium/drivers/etnaviv/etnaviv_debug.h
> index cfda52b..f47dffe 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_debug.h
> +++ b/src/gallium/drivers/etnaviv/etnaviv_debug.h
> @@ -51,6 +51,7 @@
>  #define ETNA_DBG_FLUSH_ALL       0x100000 /* Flush after every rendered primitive */
>  #define ETNA_DBG_ZERO            0x200000 /* Zero all resources after allocation */
>  #define ETNA_DBG_DRAW_STALL      0x400000 /* Stall FE/PE after every draw op */
> +#define ETNA_DBG_SHADERDB        0x800000 /* dump program compile information */
>  
>  extern int etna_mesa_debug; /* set in etna_screen.c from ETNA_DEBUG */
>  
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> index 8f2882f..6272f6f 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> @@ -62,6 +62,7 @@ static const struct debug_named_value debug_options[] = {
>     {"flush_all",      ETNA_DBG_FLUSH_ALL, "Flush after every rendered primitive"},
>     {"zero",           ETNA_DBG_ZERO, "Zero all resources after allocation"},
>     {"draw_stall",     ETNA_DBG_DRAW_STALL, "Stall FE/PE after each rendered primitive"},
> +   {"shaderdb",       ETNA_DBG_SHADERDB, "Enable shaderdb output"},
>     DEBUG_NAMED_VALUE_END
>  };
>  
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_shader.c b/src/gallium/drivers/etnaviv/etnaviv_shader.c
> index 8895311..87edf5b 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_shader.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_shader.c
> @@ -223,6 +223,42 @@ etna_shader_update_vs_inputs(struct etna_context *ctx,
>     return true;
>  }
>  
> +static inline const char *
> +etna_shader_stage(struct etna_shader *shader)
> +{
> +   switch (shader->processor) {
> +   case PIPE_SHADER_VERTEX:     return "VERT";
> +   case PIPE_SHADER_FRAGMENT:   return "FRAG";
> +   case PIPE_SHADER_COMPUTE:    return "CL";
> +   default:
> +      unreachable("invalid type");
> +      return NULL;
> +   }
> +}
> +
> +static void
> +dump_shader_info(struct etna_shader *shader, struct pipe_debug_callback *debug)
> +{
> +   if (!unlikely(etna_mesa_debug & ETNA_DBG_SHADERDB))
> +      return;
> +
> +   pipe_debug_message(debug, SHADER_INFO, "\n"
> +         "SHADER-DB: %s prog %d: %u instructions %u temps\n"
> +         "SHADER-DB: %s prog %d: %u immediates %u consts\n"
> +         "SHADER-DB: %s prog %d: %u loops\n",
> +         etna_shader_stage(shader),
> +         shader->id,
> +         shader->code_size,
> +         shader->num_temps,
> +         etna_shader_stage(shader),
> +         shader->id,
> +         shader->uniforms.imm_count,
> +         shader->uniforms.const_count,
> +         etna_shader_stage(shader),
> +         shader->id,
> +         shader->num_loops);
> +}
> +
>  bool
>  etna_shader_update_vertex(struct etna_context *ctx)
>  {
> @@ -235,8 +271,14 @@ etna_create_shader_state(struct pipe_context *pctx,
>                           const struct pipe_shader_state *pss)
>  {
>     struct etna_context *ctx = etna_context(pctx);
> +   struct etna_shader *shader = etna_compile_shader(&ctx->specs, pss->tokens);
> +
> +   static uint32_t id;

A matter of personal taste, but I would prefer this static variable to
be outside of the function. Makes it easier to grok that it is external
state and stable across function calls.

> +   shader->id = id++;
> +
> +   dump_shader_info(shader, &ctx->debug);
>  
> -   return etna_compile_shader(&ctx->specs, pss->tokens);
> +   return shader;
>  }
>  
>  static void