[Mesa-dev] [PATCH v3 1/6] gallium: expose a debug message callback settable by context owner
Ilia Mirkin
imirkin at alum.mit.edu
Sat Oct 31 10:45:26 PDT 2015
On Sat, Oct 31, 2015 at 10:23 AM, Brian Paul <brianp at 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 <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.
>
Will do. I believe that's how all the set_foo's work though, no?
>
>
>> +
>> /**
>> * 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.
Note that this will prevent virgl from passing the host's message
types through, but I guess that's not such a huge deal.
-ilia
More information about the mesa-dev
mailing list