[Mesa-dev] [PATCH] svga: use the debug callback to report issues to the state tracker

Ilia Mirkin imirkin at alum.mit.edu
Fri Dec 4 15:27:05 PST 2015


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.

  -ilia


More information about the mesa-dev mailing list