[Mesa-dev] [PATCH v3 1/6] gallium: expose a debug message callback settable by context owner
Brian Paul
brianp at vmware.com
Tue Nov 3 08:58:28 PST 2015
On 10/31/2015 11:45 AM, Ilia Mirkin wrote:
> 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?
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
More information about the mesa-dev
mailing list