[Mesa-dev] [PATCH] svga: use the debug callback to report issues to the state tracker
Jose Fonseca
jfonseca at vmware.com
Fri Dec 4 15:32:44 PST 2015
On 04/12/15 23:27, Ilia Mirkin wrote:
> On Fri, Dec 4, 2015 at 6:25 PM, Jose Fonseca <jfonseca at vmware.com> wrote:
>> On 04/12/15 19:42, Brian Paul wrote:
>>>
>>> Use the new debug callback hook to report conformance, performance
>>> and fallbacks to the state tracker. The state tracker, in turn can
>>> report this issues to the user via the GL_ARB_debug_output extension.
>>>
>>> More issues can be reported in the future; this is just a start.
>>> ---
>>> src/gallium/drivers/svga/svga_context.h | 3 +++
>>> src/gallium/drivers/svga/svga_draw_arrays.c | 7 +++++++
>>> src/gallium/drivers/svga/svga_pipe_blend.c | 11 +++++++++++
>>> src/gallium/drivers/svga/svga_pipe_misc.c | 17 +++++++++++++++++
>>> src/gallium/drivers/svga/svga_pipe_rasterizer.c | 6 ++++++
>>> src/gallium/drivers/svga/svga_state_need_swtnl.c | 23
>>> +++++++++++++++++++++++
>>> 6 files changed, 67 insertions(+)
>>>
>>> diff --git a/src/gallium/drivers/svga/svga_context.h
>>> b/src/gallium/drivers/svga/svga_context.h
>>> index 6a4f9d8..c4284cc 100644
>>> --- a/src/gallium/drivers/svga/svga_context.h
>>> +++ b/src/gallium/drivers/svga/svga_context.h
>>> @@ -392,6 +392,9 @@ struct svga_context
>>>
>>> boolean no_line_width;
>>> boolean force_hw_line_stipple;
>>> +
>>> + /** To report perf/conformance/etc issues to the state tracker */
>>> + struct pipe_debug_callback callback;
>>> } debug;
>>>
>>> struct {
>>> diff --git a/src/gallium/drivers/svga/svga_draw_arrays.c
>>> b/src/gallium/drivers/svga/svga_draw_arrays.c
>>> index acb2e95..f6956b5 100644
>>> --- a/src/gallium/drivers/svga/svga_draw_arrays.c
>>> +++ b/src/gallium/drivers/svga/svga_draw_arrays.c
>>> @@ -26,6 +26,7 @@
>>> #include "svga_cmd.h"
>>>
>>> #include "util/u_inlines.h"
>>> +#include "util/u_prim.h"
>>> #include "indices/u_indices.h"
>>>
>>> #include "svga_hw_reg.h"
>>> @@ -277,6 +278,12 @@ svga_hwtnl_draw_arrays(struct svga_hwtnl *hwtnl,
>>> if (ret != PIPE_OK)
>>> goto done;
>>>
>>> + if (svga->debug.callback.debug_message) {
>>> + pipe_debug_message(&svga->debug.callback, PERF_INFO,
>>> + "generating temporary index buffer for
>>> drawing %s",
>>> + u_prim_name(prim));
>>> + }
>>> +
>>> ret = svga_hwtnl_simple_draw_range_elements(hwtnl,
>>> gen_buf,
>>> gen_size,
>>> diff --git a/src/gallium/drivers/svga/svga_pipe_blend.c
>>> b/src/gallium/drivers/svga/svga_pipe_blend.c
>>> index 0c9d612..5538193 100644
>>> --- a/src/gallium/drivers/svga/svga_pipe_blend.c
>>> +++ b/src/gallium/drivers/svga/svga_pipe_blend.c
>>> @@ -243,6 +243,17 @@ svga_create_blend_state(struct pipe_context *pipe,
>>> blend->rt[i].srcblend_alpha = blend->rt[i].srcblend;
>>> blend->rt[i].dstblend_alpha = blend->rt[i].dstblend;
>>> blend->rt[i].blendeq_alpha = blend->rt[i].blendeq;
>>> +
>>> + if (svga->debug.callback.debug_message) {
>>> + if (templ->logicop_func == PIPE_LOGICOP_XOR) {
>>> + pipe_debug_message(&svga->debug.callback, CONFORMANCE,
>>> + "XOR logicop mode has limited
>>> support%s", "");
>>> + }
>>> + else if (templ->logicop_func != PIPE_LOGICOP_COPY) {
>>> + pipe_debug_message(&svga->debug.callback, CONFORMANCE,
>>> + "general logicops are not supported%s",
>>> "");
>>> + }
>>> + }
>>> }
>>> else {
>>> /* Note: the vgpu10 device does not yet support independent
>>> diff --git a/src/gallium/drivers/svga/svga_pipe_misc.c
>>> b/src/gallium/drivers/svga/svga_pipe_misc.c
>>> index c8020da..af9356d 100644
>>> --- a/src/gallium/drivers/svga/svga_pipe_misc.c
>>> +++ b/src/gallium/drivers/svga/svga_pipe_misc.c
>>> @@ -244,6 +244,22 @@ static void svga_set_viewport_states( struct
>>> pipe_context *pipe,
>>> }
>>>
>>>
>>> +/**
>>> + * Called by state tracker to specify a callback function the driver
>>> + * can use to report info back to the state tracker.
>>> + */
>>> +static void
>>> +svga_set_debug_callback(struct pipe_context *pipe,
>>> + const struct pipe_debug_callback *cb)
>>> +{
>>> + struct svga_context *svga = svga_context(pipe);
>>> +
>>> + if (cb)
>>> + svga->debug.callback = *cb;
>>> + else
>>> + memset(&svga->debug.callback, 0, sizeof(svga->debug.callback));
>>> +}
>>> +
>>>
>>> void svga_init_misc_functions( struct svga_context *svga )
>>> {
>>> @@ -252,6 +268,7 @@ void svga_init_misc_functions( struct svga_context
>>> *svga )
>>> svga->pipe.set_framebuffer_state = svga_set_framebuffer_state;
>>> svga->pipe.set_clip_state = svga_set_clip_state;
>>> svga->pipe.set_viewport_states = svga_set_viewport_states;
>>> + svga->pipe.set_debug_callback = svga_set_debug_callback;
>>> }
>>>
>>>
>>> diff --git a/src/gallium/drivers/svga/svga_pipe_rasterizer.c
>>> b/src/gallium/drivers/svga/svga_pipe_rasterizer.c
>>> index 6310b7a..c6215e6 100644
>>> --- a/src/gallium/drivers/svga/svga_pipe_rasterizer.c
>>> +++ b/src/gallium/drivers/svga/svga_pipe_rasterizer.c
>>> @@ -352,6 +352,12 @@ svga_create_rasterizer_state(struct pipe_context
>>> *pipe,
>>> define_rasterizer_object(svga, rast);
>>> }
>>>
>>> + if (templ->poly_smooth && svga->debug.callback.debug_message) {
>>> + /* note: we always need a % something in the message string */
>>> + pipe_debug_message(&svga->debug.callback, CONFORMANCE,
>>> + "GL_POLYGON_SMOOTH not supported%s", "");
>>> + }
>>> +
>>> svga->hud.num_state_objects++;
>>>
>>> return rast;
>>> diff --git a/src/gallium/drivers/svga/svga_state_need_swtnl.c
>>> b/src/gallium/drivers/svga/svga_state_need_swtnl.c
>>> index 429241e..f85904c 100644
>>> --- a/src/gallium/drivers/svga/svga_state_need_swtnl.c
>>> +++ b/src/gallium/drivers/svga/svga_state_need_swtnl.c
>>> @@ -62,6 +62,7 @@ update_need_pipeline(struct svga_context *svga, unsigned
>>> dirty)
>>> {
>>> boolean need_pipeline = FALSE;
>>> struct svga_vertex_shader *vs = svga->curr.vs;
>>> + const char *reason = "";
>>>
>>> /* SVGA_NEW_RAST, SVGA_NEW_REDUCED_PRIMITIVE
>>> */
>>> @@ -76,6 +77,20 @@ update_need_pipeline(struct svga_context *svga,
>>> unsigned dirty)
>>> svga->curr.rast->need_pipeline_lines_str,
>>> svga->curr.rast->need_pipeline_points_str);
>>> need_pipeline = TRUE;
>>> +
>>> + switch (svga->curr.reduced_prim) {
>>> + case PIPE_PRIM_POINTS:
>>> + reason = svga->curr.rast->need_pipeline_points_str;
>>> + break;
>>> + case PIPE_PRIM_LINES:
>>> + reason = svga->curr.rast->need_pipeline_lines_str;
>>> + break;
>>> + case PIPE_PRIM_TRIANGLES:
>>> + reason = svga->curr.rast->need_pipeline_tris_str;
>>> + break;
>>> + default:
>>> + assert(!"Unexpected reduced prim type");
>>> + }
>>> }
>>>
>>> /* EDGEFLAGS
>>> @@ -83,6 +98,7 @@ update_need_pipeline(struct svga_context *svga, unsigned
>>> dirty)
>>> if (vs && vs->base.info.writes_edgeflag) {
>>> SVGA_DBG(DEBUG_SWTNL, "%s: edgeflags\n", __FUNCTION__);
>>> need_pipeline = TRUE;
>>> + reason = "edge flags";
>>> }
>>>
>>> /* SVGA_NEW_FS, SVGA_NEW_RAST, SVGA_NEW_REDUCED_PRIMITIVE
>>> @@ -104,6 +120,7 @@ update_need_pipeline(struct svga_context *svga,
>>> unsigned dirty)
>>> * point stage.
>>> */
>>> need_pipeline = TRUE;
>>> + reason = "point sprite coordinate generation";
>>> }
>>> }
>>>
>>> @@ -116,6 +133,12 @@ update_need_pipeline(struct svga_context *svga,
>>> unsigned dirty)
>>> if (0 && svga->state.sw.need_pipeline)
>>> debug_printf("sw.need_pipeline = %d\n",
>>> svga->state.sw.need_pipeline);
>>>
>>> + if (svga->state.sw.need_pipeline &&
>>> svga->debug.callback.debug_message) {
>>> + assert(reason);
>>> + pipe_debug_message(&svga->debug.callback, FALLBACK,
>>> + "Using semi-fallback for %s", reason);
>>> + }
>>> +
>>> return PIPE_OK;
>>> }
>>>
>>>
>>
>> It's a great idea.
>>
>> I think you'll probably want to have the
>>
>> if (svga->debug.callback.debug_message) {
>> pipe_debug_message(&svga->debug.callback, ...);
>> }
>>
>> in a macro.
>>
>> Or maybe should move this logic into pipe_debug_message.
>
> Actually the logic is there, in _pipe_debug_message. But of course if
> you want to avoid computing the relevant values, then you have to do
> it by hand. Note that the format string isn't evaluated until it hits
> mesa-land, so as long as you're not actively computing stuff for this
> message that you wouldn't be otherwise, it's safe to just use
> pipe_debug_message(...) directly.
pipe_debug_message() is a macro already, but the if() is inside the
hidden _pipe_debug_message function.
If the if() statement was moved to the pipe_debug_message() we could
have both things with less typing.
One way or the other, Brian, this is
Reviewed-by: Jose Fonseca <jfonseca at vmware.com>
Jose
More information about the mesa-dev
mailing list