[Mesa-dev] [PATCH v3 1/6] gallium: expose a debug message callback settable by context owner

Brian Paul brianp at vmware.com
Sat Oct 31 07:23:22 PDT 2015


On 10/30/2015 11:15 PM, Ilia Mirkin wrote:
> This will allow gallium drivers to send messages to KHR_debug endpoints
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>   src/gallium/auxiliary/util/u_debug.c | 16 ++++++++++++++++
>   src/gallium/auxiliary/util/u_debug.h | 24 ++++++++++++++++++++++++
>   src/gallium/docs/source/context.rst  |  3 +++
>   src/gallium/include/pipe/p_context.h |  4 ++++
>   src/gallium/include/pipe/p_defines.h | 35 +++++++++++++++++++++++++++++++++++
>   src/gallium/include/pipe/p_state.h   | 29 +++++++++++++++++++++++++++++
>   6 files changed, 111 insertions(+)
>
> diff --git a/src/gallium/auxiliary/util/u_debug.c b/src/gallium/auxiliary/util/u_debug.c
> index 7388a49..81280ea 100644
> --- a/src/gallium/auxiliary/util/u_debug.c
> +++ b/src/gallium/auxiliary/util/u_debug.c
> @@ -70,6 +70,22 @@ void _debug_vprintf(const char *format, va_list ap)
>   #endif
>   }
>
> +void
> +_pipe_debug_message(
> +   struct pipe_debug_info *info,
> +   unsigned *id,
> +   enum pipe_debug_source source,
> +   enum pipe_debug_type type,
> +   enum pipe_debug_severity severity,
> +   const char *fmt, ...)
> +{
> +   va_list args;
> +   va_start(args, fmt);
> +   if (info && info->debug_message)
> +      info->debug_message(info->data, id, source, type, severity, fmt, args);
> +   va_end(args);
> +}
> +
>
>   void
>   debug_disable_error_message_boxes(void)
> diff --git a/src/gallium/auxiliary/util/u_debug.h b/src/gallium/auxiliary/util/u_debug.h
> index 926063a..a4ce88b 100644
> --- a/src/gallium/auxiliary/util/u_debug.h
> +++ b/src/gallium/auxiliary/util/u_debug.h
> @@ -42,6 +42,7 @@
>   #include "os/os_misc.h"
>
>   #include "pipe/p_format.h"
> +#include "pipe/p_defines.h"
>
>
>   #ifdef	__cplusplus
> @@ -262,6 +263,29 @@ void _debug_assert_fail(const char *expr,
>      _debug_printf("error: %s\n", __msg)
>   #endif
>
> +/**
> + * Output a debug log message to the debug info callback.
> + */
> +#define pipe_debug_message(info, source, type, severity, fmt, ...) do { \
> +   static unsigned id = 0; \
> +   _pipe_debug_message(info, &id, \
> +                       PIPE_DEBUG_SOURCE_ ## source,\
> +                       PIPE_DEBUG_TYPE_ ## type, \
> +                       PIPE_DEBUG_SEVERITY_ ## severity, \
> +                       fmt, __VA_ARGS__); \
> +} while (0)
> +
> +struct pipe_debug_info;
> +
> +void
> +_pipe_debug_message(
> +   struct pipe_debug_info *info,
> +   unsigned *id,
> +   enum pipe_debug_source source,
> +   enum pipe_debug_type type,
> +   enum pipe_debug_severity severity,
> +   const char *fmt, ...) _util_printf_format(6, 7);
> +
>
>   /**
>    * Used by debug_dump_enum and debug_dump_flags to describe symbols.
> diff --git a/src/gallium/docs/source/context.rst b/src/gallium/docs/source/context.rst
> index a7d08d2..5cae4d6 100644
> --- a/src/gallium/docs/source/context.rst
> +++ b/src/gallium/docs/source/context.rst
> @@ -84,6 +84,9 @@ objects. They all follow simple, one-method binding calls, e.g.
>       levels. This corresponds to GL's ``PATCH_DEFAULT_OUTER_LEVEL``.
>     * ``default_inner_level`` is the default value for the inner tessellation
>       levels. This corresponds to GL's ``PATCH_DEFAULT_INNER_LEVEL``.
> +* ``set_debug_info`` sets the callback to be used for reporting
> +  various debug messages, eventually reported via KHR_debug and
> +  similar mechanisms.
>
>
>   Sampler Views
> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
> index 6f9fe76..0d5eeab 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -45,6 +45,7 @@ struct pipe_blit_info;
>   struct pipe_box;
>   struct pipe_clip_state;
>   struct pipe_constant_buffer;
> +struct pipe_debug_info;
>   struct pipe_depth_stencil_alpha_state;
>   struct pipe_draw_info;
>   struct pipe_fence_handle;
> @@ -238,6 +239,9 @@ struct pipe_context {
>                             const float default_outer_level[4],
>                             const float default_inner_level[2]);
>
> +   void (*set_debug_info)(struct pipe_context *,
> +                          struct pipe_debug_info *);

Evidently, the implementation of this function must make a copy of the 
pipe_debug_info and can't just save the pointer.  Could you add a 
comment about that?  'info' could be const-qualified too.


> +
>      /**
>       * Bind an array of shader buffers that will be used by a shader.
>       * Any buffers that were previously bound to the specified range
> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
> index b15c880..860ebc6 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -868,6 +868,41 @@ struct pipe_driver_query_group_info
>      unsigned num_queries;
>   };
>
> +enum pipe_debug_source
> +{
> +   PIPE_DEBUG_SOURCE_API,
> +   PIPE_DEBUG_SOURCE_WINDOW_SYSTEM,
> +   PIPE_DEBUG_SOURCE_SHADER_COMPILER,
> +   PIPE_DEBUG_SOURCE_THIRD_PARTY,
> +   PIPE_DEBUG_SOURCE_APPLICATION,
> +   PIPE_DEBUG_SOURCE_OTHER,
> +   PIPE_DEBUG_SOURCE_COUNT
> +};
> +
> +enum pipe_debug_type
> +{
> +   PIPE_DEBUG_TYPE_ERROR,
> +   PIPE_DEBUG_TYPE_DEPRECATED,
> +   PIPE_DEBUG_TYPE_UNDEFINED,
> +   PIPE_DEBUG_TYPE_PORTABILITY,
> +   PIPE_DEBUG_TYPE_PERFORMANCE,
> +   PIPE_DEBUG_TYPE_OTHER,
> +   PIPE_DEBUG_TYPE_MARKER,
> +   PIPE_DEBUG_TYPE_PUSH_GROUP,
> +   PIPE_DEBUG_TYPE_POP_GROUP,
> +   PIPE_DEBUG_TYPE_COUNT
> +};
> +
> +enum pipe_debug_severity
> +{
> +   PIPE_DEBUG_SEVERITY_LOW,
> +   PIPE_DEBUG_SEVERITY_MEDIUM,
> +   PIPE_DEBUG_SEVERITY_HIGH,
> +   PIPE_DEBUG_SEVERITY_NOTIFICATION,
> +   PIPE_DEBUG_SEVERITY_COUNT
> +};

I think these enums are overkill and not really a good match for what we 
want to communicate.

In nouveau you report fence stalls as API, OTHER, NOTIFICATION.  At 
least, OTHER could be replaced with PERFORMANCE, I think.  In nv50/nvc0, 
shader info is reported as SHADER_COMPILER, OTHER, NOTIFICATION.  I can 
imagine a lot of future messages being reported as OTHER/NOTIFICATION, 
which doesn't add much value.


Maybe we could boil things down to a single enum.  How about:

enum pipe_debug_type {
    PIPE_DEBUG_TYPE_OUT_OF_MEMORY,
    PIPE_DEBUG_TYPE_ERROR,        // generic errors
    PIPE_DEBUG_TYPE_SHADER_INFO,
    PIPE_DEBUG_TYPE_PERF_INFO,
    PIPE_DEBUG_TYPE_INFO,         // generic info of interest
    PIPE_DEBUG_TYPE_FALLBACK,     // some fallback was hit
    PIPE_DEBUG_TYPE_CONFORMANCE   // non-conformance issue.
};

Others could be added as needed.  I'd rather start small than begin with 
a large set of enums where many are useless.  Afterall, the trend in 
gallium is to remove unused stuff whenever possible.

The state tracker could easily convert these types into 
GL_ARB_debug_output tokens/messages.

-Brian


> +
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
> index 4bf8d46..2843bb6 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -684,6 +684,35 @@ struct pipe_compute_state
>      unsigned req_input_mem; /**< Required size of the INPUT resource. */
>   };
>
> +/**
> + * Structure that contains a callback for debug messages from the driver back
> + * to the state tracker.
> + */
> +struct pipe_debug_info
> +{
> +   /**
> +    * Callback for the driver to report debug/performance/etc information back
> +    * to the state tracker.
> +    *
> +    * \param data       user-supplied data pointer
> +    * \param id         message type identifier, if pointed value is 0, then a
> +    *                   new id is assigned
> +    * \param source     PIPE_DEBUG_SOURCE_*
> +    * \param type       PIPE_DEBUG_TYPE_*
> +    * \param severity   PIPE_DEBUG_SEVERITY_*
> +    * \param format     printf-style format string
> +    * \param args       args for format string
> +    */
> +   void (*debug_message)(void *data,
> +                         unsigned *id,
> +                         enum pipe_debug_source source,
> +                         enum pipe_debug_type type,
> +                         enum pipe_debug_severity severity,
> +                         const char *fmt,
> +                         va_list args);
> +   void *data;
> +};
> +
>   #ifdef __cplusplus
>   }
>   #endif
>



More information about the mesa-dev mailing list