[Mesa-dev] [PATCH 7/7] i965: Reduce vertex state reemission

[Ian Romanick]

On 12/15/2015 12:28 AM, Kristian Høgsberg Kristensen wrote:
> We can inspect VS prog_data for iterations i > 0, and only flag
> BRW_NEW_VERTICES when one of our system values change.
> 
> This change also flags BRW_NEW_VERTICES in one case we were missing
> before: if we're doing an indirect draw, prims[i].basevertex is always 0
> and the real base vertex value is in the indirect parameter
> buffer. Thus, if a program uses base vertex or base instance, and the
> draw call is indirect, flag BRW_NEW_VERTICES.  A new piglit test,
> spec/ARB_shader_draw_parameters/drawid-indirect-vertexid tests this.
> ---
>  src/mesa/drivers/dri/i965/brw_draw.c | 44 ++++++++++++++++++++++++++++++++----
>  1 file changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
> index b0710c67..9e400ca 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -491,9 +491,44 @@ brw_try_draw_prims(struct gl_context *ctx,
>           }
>        }
>  
> -      brw->draw.params.gl_basevertex =
> +      /* Determine if we need to flag BRW_NEW_VERTICES for updating the
> +       * gl_BaseVertexARB, gl_BaseInstanceARB or gl_DrawIDARB values. As
> +       * above, we don't need to check first iteration, since the flag is set
> +       * before the loop. We also can't rely on vs prog_data in the first
> +       * iteration, but after drawing once, we've uploaded the programs and
> +       * can look at prog_data.
> +       *
> +       * Despite the prims[] name, eache iteration correspond to a draw call
                                      each            corresponds

> +       * from a glMulti* style draw call. We need to re-upload vertex state if
> +       *
> +       *  1) the program uses gl_DrawIDARB (changes every iteration),
> +       *
> +       *  2) the program uses gl_BaseVertexARB or gl_BaseInstanceARB and the
> +       *     draw call is indirect (meaning we can't check if the value change
> +       *     or not), or
> +       *
> +       *  3) the program uses gl_BaseVertexARB or gl_BaseInstanceARB and the
> +       *  value changed
> +       */
> +      const int new_basevertex =
>           prims[i].indexed ? prims[i].basevertex : prims[i].start;
> -      brw->draw.params.gl_baseinstance = prims[i].base_instance;
> +      const int new_baseinstance = prims[i].base_instance;
> +      if (i > 0) {
> +         const bool uses_draw_parameters =
> +            brw->vs.prog_data->uses_basevertex ||
> +            brw->vs.prog_data->uses_baseinstance;
> +
> +         if (brw->vs.prog_data->uses_drawid ||
> +             (uses_draw_parameters && prims[i].is_indirect) ||
> +             (brw->vs.prog_data->uses_basevertex &&
> +              brw->draw.params.gl_basevertex != new_basevertex) ||
> +             (brw->vs.prog_data->uses_baseinstance &&
> +              brw->draw.params.gl_baseinstance != new_baseinstance))
> +            brw->ctx.NewDriverState |= BRW_NEW_VERTICES;
> +      }
> +
> +      brw->draw.params.gl_basevertex = new_basevertex;
> +      brw->draw.params.gl_baseinstance = new_baseinstance;
>        drm_intel_bo_unreference(brw->draw.draw_params_bo);
>  
>        if (prims[i].is_indirect) {
> @@ -512,10 +547,11 @@ brw_try_draw_prims(struct gl_context *ctx,
>        }
>  
>        /* gl_DrawID always needs its own vertex buffer since it's not part of
> -       * the indirect parameter buffer. */
> +       * the indirect parameter buffer.
> +       */

Lol

>        brw->draw.gl_drawid = prims[i].draw_id;
>        drm_intel_bo_unreference(brw->draw.draw_id_bo);
> -      brw->ctx.NewDriverState |= BRW_NEW_VERTICES;
> +      brw->draw.draw_id_bo = NULL;

It seems odd that this change is in this patch.  Should it have always
been after the drm_intel_bo_unreference call?

>  
>        if (brw->gen < 6)
>  	 brw_set_prim(brw, &prims[i]);
>