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 <[email protected]>
---
  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


_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to