On Fri, Dec 4, 2015 at 6:32 PM, Jose Fonseca <[email protected]> wrote: > On 04/12/15 23:27, Ilia Mirkin wrote: >> >> On Fri, Dec 4, 2015 at 6:25 PM, Jose Fonseca <[email protected]> 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.
Sure, I'd be perfectly fine with moving the if from _pipe_debug_message into pipe_debug_message. I probably had a reason why I did it this way, but can't think of anything even halfway logical now. -ilia _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
