On Tue, Nov 3, 2015 at 11:58 AM, Brian Paul <bri...@vmware.com> wrote: > 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
That all sounds good to me, thanks for taking the time to work these out. Unless I hear any objections, will resend tonight with the updated mechanism. > >> >> 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. Dave, complain now or forever hold your peace :) -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev