[Mesa-dev] [PATCH 11/22] i965: generalize brw_vs_pull_constants in preparation for GS.

[Kenneth Graunke]

On 08/26/2013 03:12 PM, Paul Berry wrote:
> ---
>   src/mesa/drivers/dri/i965/brw_state.h            |  8 +++
>   src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 66 +++++++++++++++---------
>   2 files changed, 50 insertions(+), 24 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
> index 4814639..e7a1b40 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -221,6 +221,14 @@ uint32_t
>   get_attr_override(const struct brw_vue_map *vue_map, int urb_entry_read_offset,
>                     int fs_attr, bool two_side_color, uint32_t *max_source_attr);
>
> +/* brw_vs_surface_state.c */
> +void
> +brw_upload_vec4_pull_constants(struct brw_context *brw,
> +                               GLbitfield64 brw_new_constbuf,

FWIW, brw->state.dirty.brw is only 32-bits currently.  That said, it's 
probably going to change in the not-too-distant future, so using 
GLbitfield64 preemptively isn't crazy.

> +                               const struct gl_program *prog,
> +                               struct brw_vec4_context_base *vec4_ctx,
> +                               const struct brw_vec4_prog_data *prog_data);
> +
>   #ifdef __cplusplus
>   }
>   #endif
> 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 629eb96..48124bf 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> @@ -35,56 +35,50 @@
>   #include "brw_context.h"
>   #include "brw_state.h"
>
> -/* Creates a new VS constant buffer reflecting the current VS program's
> - * constants, if needed by the VS program.
> - *
> - * Otherwise, constants go through the CURBEs using the brw_constant_buffer
> - * state atom.
> - */
> -static void
> -brw_upload_vs_pull_constants(struct brw_context *brw)
> -{
> -   struct brw_vec4_context_base *vec4_ctx = &brw->vs.base;
>
> -   /* BRW_NEW_VERTEX_PROGRAM */
> -   struct brw_vertex_program *vp =
> -      (struct brw_vertex_program *) brw->vertex_program;
> +void
> +brw_upload_vec4_pull_constants(struct brw_context *brw,
> +                               GLbitfield64 brw_new_constbuf,
> +                               const struct gl_program *prog,
> +                               struct brw_vec4_context_base *vec4_ctx,
> +                               const struct brw_vec4_prog_data *prog_data)
> +{
>      int i;
>
>      /* Updates the ParamaterValues[i] pointers for all parameters of the
>       * basic type of PROGRAM_STATE_VAR.
>       */
> -   _mesa_load_state_parameters(&brw->ctx, vp->program.Base.Parameters);
> +   _mesa_load_state_parameters(&brw->ctx, prog->Parameters);
>
> -   /* CACHE_NEW_VS_PROG */
> -   if (!brw->vs.prog_data->base.nr_pull_params) {
> +   if (!prog_data->nr_pull_params) {
>         if (vec4_ctx->const_bo) {
>   	 drm_intel_bo_unreference(vec4_ctx->const_bo);
>   	 vec4_ctx->const_bo = NULL;
>   	 vec4_ctx->surf_offset[SURF_INDEX_VEC4_CONST_BUFFER] = 0;
> -	 brw->state.dirty.brw |= BRW_NEW_VS_CONSTBUF;
> +	 brw->state.dirty.brw |= brw_new_constbuf;
>         }
>         return;
>      }
>
>      /* _NEW_PROGRAM_CONSTANTS */
>      drm_intel_bo_unreference(vec4_ctx->const_bo);
> -   uint32_t size = brw->vs.prog_data->base.nr_pull_params * 4;
> -   vec4_ctx->const_bo = drm_intel_bo_alloc(brw->bufmgr, "vp_const_buffer",
> +   uint32_t size = prog_data->nr_pull_params * 4;
> +   vec4_ctx->const_bo = drm_intel_bo_alloc(brw->bufmgr, "vec4_const_buffer",
>                                              size, 64);
>
>      drm_intel_gem_bo_map_gtt(vec4_ctx->const_bo);
> -   for (i = 0; i < brw->vs.prog_data->base.nr_pull_params; i++) {
> +
> +   for (i = 0; i < prog_data->nr_pull_params; i++) {
>         memcpy(vec4_ctx->const_bo->virtual + i * 4,
> -	     brw->vs.prog_data->base.pull_param[i],
> +	     prog_data->pull_param[i],
>   	     4);
>      }
>
>      if (0) {
> -      for (i = 0; i < ALIGN(brw->vs.prog_data->base.nr_pull_params, 4) / 4;
> +      for (i = 0; i < ALIGN(prog_data->nr_pull_params, 4) / 4;
>              i++) {

You could probably move the i++ up a line since it's shorter now.

This patch is great.

>   	 float *row = (float *)vec4_ctx->const_bo->virtual + i * 4;
> -	 printf("vs const surface %3d: %4.3f %4.3f %4.3f %4.3f\n",
> +	 printf("const surface %3d: %4.3f %4.3f %4.3f %4.3f\n",
>   		i, row[0], row[1], row[2], row[3]);
>         }
>      }
> @@ -95,7 +89,31 @@ brw_upload_vs_pull_constants(struct brw_context *brw)
>      brw->vtbl.create_constant_surface(brw, vec4_ctx->const_bo, 0, size,
>                                        &vec4_ctx->surf_offset[surf], false);
>
> -   brw->state.dirty.brw |= BRW_NEW_VS_CONSTBUF;
> +   brw->state.dirty.brw |= brw_new_constbuf;
> +}
> +
> +
> +/* Creates a new VS constant buffer reflecting the current VS program's
> + * constants, if needed by the VS program.
> + *
> + * Otherwise, constants go through the CURBEs using the brw_constant_buffer
> + * state atom.
> + */
> +static void
> +brw_upload_vs_pull_constants(struct brw_context *brw)
> +{
> +   struct brw_vec4_context_base *vec4_ctx = &brw->vs.base;
> +
> +   /* BRW_NEW_VERTEX_PROGRAM */
> +   struct brw_vertex_program *vp =
> +      (struct brw_vertex_program *) brw->vertex_program;
> +
> +   /* CACHE_NEW_VS_PROG */
> +   const struct brw_vec4_prog_data *prog_data = &brw->vs.prog_data->base;
> +
> +   /* _NEW_PROGRAM_CONSTANTS */
> +   brw_upload_vec4_pull_constants(brw, BRW_NEW_VS_CONSTBUF, &vp->program.Base,
> +                                  vec4_ctx, prog_data);
>   }
>
>   const struct brw_tracked_state brw_vs_pull_constants = {
>