[Mesa-dev] [PATCH 17/33] i965: Fold prepare() and emit() of VS surface state setup together.

[Paul Berry]

On 24 October 2011 14:17, Eric Anholt <eric at anholt.net> wrote:

> This rearranges the code a bit, and makes the upload of the binding
> table take only as many surfaces as there are in use.
> ---
>  src/mesa/drivers/dri/i965/brw_vs_surface_state.c |   62
> +++++++++-------------
>  1 files changed, 25 insertions(+), 37 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> index bfc4f5d..0237b58 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> @@ -138,22 +138,6 @@ brw_update_vs_constant_surface( struct gl_context
> *ctx,
>    }
>  }
>
> -
> -static void
> -prepare_vs_surfaces(struct brw_context *brw)
> -{
> -   int nr_surfaces = 0;
> -
> -   if (brw->vs.const_bo) {
> -      nr_surfaces = 1;
> -   }
> -
> -   if (brw->vs.nr_surfaces != nr_surfaces) {
> -      brw->state.dirty.brw |= BRW_NEW_NR_VS_SURFACES;
> -      brw->vs.nr_surfaces = nr_surfaces;
> -   }
> -}
> -
>  /**
>  * Vertex shader surfaces (constant buffer).
>  *
> @@ -161,36 +145,41 @@ prepare_vs_surfaces(struct brw_context *brw)
>  * to be updated, and produces BRW_NEW_NR_VS_SURFACES for the VS unit and
>  * CACHE_NEW_SURF_BIND for the binding table upload.
>  */
> -static void upload_vs_surfaces(struct brw_context *brw)
> +static void
> +brw_upload_vs_surfaces(struct brw_context *brw)
>  {
>    struct gl_context *ctx = &brw->intel.ctx;
>    uint32_t *bind;
>    int i;
> +   int nr_surfaces = 0;
> +
> +   /* BRW_NEW_VS_CONSTBUF */
> +   if (brw->vs.const_bo) {
> +      nr_surfaces = 1;
> +      brw_update_vs_constant_surface(ctx, SURF_INDEX_VERT_CONST_BUFFER);
> +   }
> +
> +   if (nr_surfaces != 0) {
>

This reads a little strange to me.  The only way nr_surfaces can be nonzero
is if the previous if test succeeded, so why not just merge the two if
statements?


> +      bind = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
> +                            sizeof(uint32_t) * nr_surfaces,
> +                            32, &brw->vs.bind_bo_offset);
>
> -   /* BRW_NEW_NR_VS_SURFACES */
> -   if (brw->vs.nr_surfaces == 0) {
> +      for (i = 0; i < nr_surfaces; i++) {
> +        /* BRW_NEW_VS_CONSTBUF */
> +        bind[i] = brw->vs.surf_offset[i];
> +      }
>

This for loop also seems weird, since at this point in the code nr_surfaces
is known to be equal to 1.  Why not just do "bind[0] =
brw->vs.surf_offset[0];"?


> +      brw->state.dirty.brw |= BRW_NEW_VS_BINDING_TABLE;
> +   } else {
>       if (brw->vs.bind_bo_offset) {
>         brw->state.dirty.brw |= BRW_NEW_VS_BINDING_TABLE;
> +        brw->vs.bind_bo_offset = 0;
>       }
> -      brw->vs.bind_bo_offset = 0;
> -      return;
>    }
>
> -   brw_update_vs_constant_surface(ctx, SURF_INDEX_VERT_CONST_BUFFER);
> -
> -   /* Might want to calculate nr_surfaces first, to avoid taking up so
> much
> -    * space for the binding table. (once we have vs samplers)
> -    */
> -   bind = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
> -                         sizeof(uint32_t) * BRW_VS_MAX_SURF,
> -                         32, &brw->vs.bind_bo_offset);
> -
> -   for (i = 0; i < BRW_VS_MAX_SURF; i++) {
> -      /* BRW_NEW_VS_CONSTBUF */
> -      bind[i] = brw->vs.surf_offset[i];
> +   if (brw->vs.nr_surfaces != nr_surfaces) {
> +      brw->state.dirty.brw |= BRW_NEW_NR_VS_SURFACES;
> +      brw->vs.nr_surfaces = nr_surfaces;
>    }
> -
> -   brw->state.dirty.brw |= BRW_NEW_VS_BINDING_TABLE;
>  }
>
>  const struct brw_tracked_state brw_vs_surfaces = {
> @@ -201,6 +190,5 @@ const struct brw_tracked_state brw_vs_surfaces = {
>              BRW_NEW_BATCH),
>       .cache = 0
>    },
> -   .prepare = prepare_vs_surfaces,
> -   .emit = upload_vs_surfaces,
> +   .emit = brw_upload_vs_surfaces,
>  };
> --
> 1.7.7
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111027/5686a260/attachment-0001.html>