On 10/31/2015 11:45 AM, Ilia Mirkin wrote:
On Sat, Oct 31, 2015 at 10:23 AM, Brian Paul <bri...@vmware.com> wrote: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 <imir...@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.Will do. I believe that's how all the set_foo's work though, no?
I think so but would have to check to be sure.
+ /** * 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.SHADER_COMPILER/OTHER/NOTIFICATION is what the i965 compiler uses to report shader-db info, and what the shader-db runner is rigged for.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. };Sure! Can you provide the mapping that each one should have onto the KHR_debug/ARB_debug_output types? Please make sure that one of them is SHADER_COMPILER/OTHER/NOTIFICATION, as I need that for shader-db. The reason that I wanted to just reuse the GL types is that I have no idea what any of these mean or how they're meant to be distinguished from one another. My hope was that this would cause less bike-shedding.
How about this: PIPE_DEBUG_TYPE_OUT_OF_MEMORY: API/ERROR/MEDIUM PIPE_DEBUG_TYPE_ERROR: API/ERROR/MEDIUM PIPE_DEBUG_TYPE_SHADER_INFO: SHADER_COMPILER/OTHER/NOTIFICATION PIPE_DEBUG_TYPE_PERF_INFO: API/PERFORMANCE/NOTIFICATION PIPE_DEBUG_TYPE_INFO: API/OTHER/NOTIFICATION PIPE_DEBUG_TYPE_FALLBACK: API/PERFORMANCE/NOTIFICATION PIPE_DEBUG_TYPE_CONFORMANCE: API/OTHER/NOTIFICATION
Note that this will prevent virgl from passing the host's message types through, but I guess that's not such a huge deal.
Perhaps Dave can chime in if this is a concern for him now. -Brian _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
