[Mesa-dev] [PATCH 24/30] i965/gs: Support UBOs by generating surface state and binding tables.

Kenneth Graunke kenneth at whitecape.org
Thu Aug 22 13:58:53 PDT 2013


On 08/20/2013 11:30 AM, Paul Berry wrote:
> From: Eric Anholt <eric at anholt.net>
>
> All but two of the piglit GLSL 1.50/uniform_buffer tests work, and
> maxuniformblocksize and referenced-by-shader work.
>
> v2 (Paul Berry <stereotype441 at gmail.com>): Account for Ken's recent
> binding table re-work.  Use brw->vec4_gs.bind_bo_offset instead of
> brw->gs.bind_bo_offset.
> ---
>   src/mesa/drivers/dri/i965/Makefile.sources       |   1 +
>   src/mesa/drivers/dri/i965/brw_context.h          |  17 ++-
>   src/mesa/drivers/dri/i965/brw_gs_emit.c          |   2 +-
>   src/mesa/drivers/dri/i965/brw_gs_surface_state.c | 182 +++++++++++++++++++++++
>   src/mesa/drivers/dri/i965/brw_state.h            |   3 +
>   src/mesa/drivers/dri/i965/brw_state_upload.c     |   3 +
>   src/mesa/drivers/dri/i965/gen6_sol.c             |   6 +-
>   7 files changed, 208 insertions(+), 6 deletions(-)
>   create mode 100644 src/mesa/drivers/dri/i965/brw_gs_surface_state.c
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources b/src/mesa/drivers/dri/i965/Makefile.sources
> index 290cd93..81a16ff 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -63,6 +63,7 @@ i965_FILES = \
>   	brw_gs.c \
>   	brw_gs_emit.c \
>   	brw_gs_state.c \
> +	brw_gs_surface_state.c \
>   	brw_interpolation_map.c \
>   	brw_lower_texture_gradients.cpp \
>   	brw_misc_state.c \
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index 4f6c767..a6d0786 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -148,6 +148,7 @@ enum brw_state_id {
>      BRW_STATE_BATCH,
>      BRW_STATE_INDEX_BUFFER,
>      BRW_STATE_VS_CONSTBUF,
> +   BRW_STATE_GS_CONSTBUF,
>      BRW_STATE_PROGRAM_CACHE,
>      BRW_STATE_STATE_BASE_ADDRESS,
>      BRW_STATE_VUE_MAP_VS,
> @@ -184,6 +185,7 @@ enum brw_state_id {
>   /** \see brw.state.depth_region */
>   #define BRW_NEW_INDEX_BUFFER           (1 << BRW_STATE_INDEX_BUFFER)
>   #define BRW_NEW_VS_CONSTBUF            (1 << BRW_STATE_VS_CONSTBUF)
> +#define BRW_NEW_GS_CONSTBUF            (1 << BRW_STATE_GS_CONSTBUF)
>   #define BRW_NEW_PROGRAM_CACHE		(1 << BRW_STATE_PROGRAM_CACHE)
>   #define BRW_NEW_STATE_BASE_ADDRESS	(1 << BRW_STATE_STATE_BASE_ADDRESS)
>   #define BRW_NEW_VUE_MAP_VS		(1 << BRW_STATE_VUE_MAP_VS)
> @@ -654,8 +656,19 @@ struct brw_vec4_gs_prog_data
>   #define SURF_INDEX_VS_SHADER_TIME    (SURF_INDEX_VS_UBO(12))
>   #define BRW_MAX_VS_SURFACES          (SURF_INDEX_VS_SHADER_TIME + 1)
>
> -#define SURF_INDEX_SOL_BINDING(t)    ((t))
> -#define BRW_MAX_GS_SURFACES          SURF_INDEX_SOL_BINDING(BRW_MAX_SOL_BINDINGS)
> +#define SURF_INDEX_GEN6_SOL_BINDING(t) (t)
> +#define BRW_MAX_GEN6_GS_SURFACES       SURF_INDEX_GEN6_SOL_BINDING(BRW_MAX_SOL_BINDINGS)

It's not a big deal, but it would be nice to do the 
SURF_INDEX_SOL_BINDING renames in a separate patch.

> +#define SURF_INDEX_GS_GEN6_SOL_BUFFER 0
> +#define SURF_INDEX_GS_CONST_BUFFER   (SURF_INDEX_GS_GEN6_SOL_BUFFER)
> +
> +#define SURF_INDEX_GS_TEXTURE(t)     (SURF_INDEX_GS_CONST_BUFFER + 1 + (t))
> +#define SURF_INDEX_GS_UBO(u)         (SURF_INDEX_GS_TEXTURE(BRW_MAX_TEX_UNIT) + u)
> +#define SURF_INDEX_GS_SHADER_TIME    (SURF_INDEX_GS_UBO(12))
> +#define BRW_MAX_GEN7_GS_SURFACES          (SURF_INDEX_GS_SHADER_TIME + 1)
> +
> +#define BRW_MAX_GS_SURFACES MAX2(BRW_MAX_GEN6_GS_SURFACES, \
> +                                 BRW_MAX_GEN7_GS_SURFACES)
>
>   /**
>    * Stride in bytes between shader_time entries.
> diff --git a/src/mesa/drivers/dri/i965/brw_gs_emit.c b/src/mesa/drivers/dri/i965/brw_gs_emit.c
> index fff3585..f576a81 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs_emit.c
> @@ -432,7 +432,7 @@ gen6_sol_program(struct brw_gs_compile *c, struct brw_gs_prog_key *key,
>                             final_write ? c->reg.temp : brw_null_reg(), /* dest */
>                             1, /* msg_reg_nr */
>                             c->reg.header, /* src0 */
> -                          SURF_INDEX_SOL_BINDING(binding), /* binding_table_index */
> +                          SURF_INDEX_GEN6_SOL_BINDING(binding), /* binding_table_index */
>                             final_write); /* send_commit_msg */
>            }
>         }
> diff --git a/src/mesa/drivers/dri/i965/brw_gs_surface_state.c b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c
> new file mode 100644
> index 0000000..ed99d65
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c
> @@ -0,0 +1,182 @@
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "main/mtypes.h"
> +#include "program/prog_parameter.h"
> +
> +#include "brw_context.h"
> +#include "brw_state.h"
> +
> +/* Creates a new GS constant buffer reflecting the current GS program's
> + * constants, if needed by the GS program.
> + *
> + * Otherwise, constants go through the CURBEs using the brw_constant_buffer
> + * state atom.
> + */

This is a lot of code duplication, and I'd really like to avoid that. 
(Avoiding it should help even more when we add tesselation shaders.)

Paul and I had talked briefly about creating a new structure type to 
hold some of the fields that are common between vs, gs, and wm:

- scratch_bo
- const_bo
- prog_offset
- state_offset
- push_const_offset
- push_const_size
- bind_bo_offset
- surf_offset
   (the size varies slightly, but we can probably take the largest)
- sampler_count
- sampler_offset
- sdc_offset

Then vs, gs, and wm could use instances of that structure, rather than 
containing those fields directly.  This would allow us to pass around 
that "shader stage data" (pipeline stage data?) struct, rather than 
inspecting vs or gs directly.

I think we could create a function for at least VS/GS binding tables 
that takes that struct, struct gl_program, and brw_vec4_prog_data, which 
would work for both.

> +static void
> +brw_upload_gs_pull_constants(struct brw_context *brw)
> +{
> +   /* BRW_NEW_GEOMETRY_PROGRAM */
> +   struct brw_geometry_program *gp =
> +      (struct brw_geometry_program *) brw->geometry_program;
> +   int i;
> +
> +   if (!gp)
> +      return;
> +
> +   /* Updates the ParamaterValues[i] pointers for all parameters of the
> +    * basic type of PROGRAM_STATE_VAR.
> +    */
> +
> +   _mesa_load_state_parameters(&brw->ctx, gp->program.Base.Parameters);
> +
> +   /* CACHE_NEW_GS_PROG */
> +   if (!brw->vec4_gs.prog_data->base.nr_pull_params) {
> +      if (brw->gs.const_bo) {
> +	 drm_intel_bo_unreference(brw->gs.const_bo);
> +	 brw->gs.const_bo = NULL;
> +	 brw->gs.surf_offset[SURF_INDEX_GS_CONST_BUFFER] = 0;
> +	 brw->state.dirty.brw |= BRW_NEW_GS_CONSTBUF;
> +      }
> +      return;
> +   }
> +
> +   /* _NEW_PROGRAM_CONSTANTS */
> +   drm_intel_bo_unreference(brw->gs.const_bo);
> +   uint32_t size = brw->vec4_gs.prog_data->base.nr_pull_params * 4;
> +   brw->gs.const_bo = drm_intel_bo_alloc(brw->bufmgr, "gp_const_buffer",
> +					 size, 64);
> +
> +   drm_intel_gem_bo_map_gtt(brw->gs.const_bo);
> +   for (i = 0; i < brw->vec4_gs.prog_data->base.nr_pull_params; i++) {
> +      memcpy(brw->gs.const_bo->virtual + i * 4,
> +	     brw->vec4_gs.prog_data->base.pull_param[i],
> +	     4);
> +   }
> +
> +   if (0) {
> +      for (i = 0; i < ALIGN(brw->vec4_gs.prog_data->base.nr_pull_params, 4) / 4;
> +           i++) {
> +	 float *row = (float *)brw->gs.const_bo->virtual + i * 4;
> +	 printf("gs const surface %3d: %4.3f %4.3f %4.3f %4.3f\n",
> +		i, row[0], row[1], row[2], row[3]);
> +      }
> +   }
> +
> +   drm_intel_gem_bo_unmap_gtt(brw->gs.const_bo);
> +
> +   const int surf = SURF_INDEX_GS_CONST_BUFFER;
> +   brw->vtbl.create_constant_surface(brw, brw->gs.const_bo, 0, size,
> +                                     &brw->gs.surf_offset[surf], false);
> +
> +   brw->state.dirty.brw |= BRW_NEW_GS_CONSTBUF;
> +}
> +
> +const struct brw_tracked_state brw_gs_pull_constants = {
> +   .dirty = {
> +      .mesa = (_NEW_PROGRAM_CONSTANTS),
> +      .brw = (BRW_NEW_BATCH | BRW_NEW_GEOMETRY_PROGRAM),
> +      .cache = CACHE_NEW_GS_PROG,
> +   },
> +   .emit = brw_upload_gs_pull_constants,
> +};
> +
> +static void
> +brw_upload_gs_ubo_surfaces(struct brw_context *brw)
> +{
> +   struct gl_context *ctx = &brw->ctx;
> +   /* _NEW_PROGRAM */
> +   struct gl_shader_program *prog = ctx->Shader.CurrentGeometryProgram;
> +
> +   if (!prog)
> +      return;
> +
> +   brw_upload_ubo_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_GEOMETRY],
> +			   &brw->gs.surf_offset[SURF_INDEX_GS_UBO(0)]);
> +}
> +
> +const struct brw_tracked_state brw_gs_ubo_surfaces = {
> +   .dirty = {
> +      .mesa = (_NEW_PROGRAM |
> +	       _NEW_BUFFER_OBJECT),
> +      .brw = BRW_NEW_BATCH,
> +      .cache = 0,
> +   },
> +   .emit = brw_upload_gs_ubo_surfaces,
> +};
> +
> +/**
> + * Constructs the binding table for the WM surface state, which maps unit
> + * numbers to surface state objects.
> + */
> +static void
> +brw_gs_upload_binding_table(struct brw_context *brw)
> +{
> +   uint32_t *bind;
> +   int i;
> +
> +   /* If there's no GS, skip changing anything. */
> +   if (!brw->vec4_gs.prog_data)
> +      return;
> +
> +   if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
> +      gen7_create_shader_time_surface(brw, &brw->gs.surf_offset[SURF_INDEX_GS_SHADER_TIME]);
> +   }
> +
> +   /* CACHE_NEW_GS_PROG: Skip making a binding table if we don't use textures or
> +    * pull constants.
> +    */
> +   const unsigned entries = brw->vec4_gs.prog_data->base.binding_table_size;
> +   if (entries == 0) {
> +      if (brw->vec4_gs.bind_bo_offset != 0) {
> +	 brw->state.dirty.brw |= BRW_NEW_GS_BINDING_TABLE;
> +	 brw->vec4_gs.bind_bo_offset = 0;
> +      }
> +      return;
> +   }
> +
> +   /* Might want to calculate nr_surfaces first, to avoid taking up so much
> +    * space for the binding table.
> +    */
> +   bind = brw_state_batch(brw, AUB_TRACE_BINDING_TABLE,
> +			  sizeof(uint32_t) * entries,
> +			  32, &brw->vec4_gs.bind_bo_offset);
> +
> +   /* BRW_NEW_SURFACES and BRW_NEW_GS_CONSTBUF */
> +   for (i = 0; i < entries; i++) {
> +      bind[i] = brw->gs.surf_offset[i];
> +   }
> +
> +   brw->state.dirty.brw |= BRW_NEW_GS_BINDING_TABLE;
> +}
> +
> +const struct brw_tracked_state brw_gs_binding_table = {
> +   .dirty = {
> +      .mesa = 0,
> +      .brw = (BRW_NEW_BATCH |
> +	      BRW_NEW_GS_CONSTBUF |
> +	      BRW_NEW_SURFACES),
> +      .cache = CACHE_NEW_GS_PROG
> +   },
> +   .emit = brw_gs_upload_binding_table,
> +};
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
> index 0127e10..8597687 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -47,6 +47,7 @@ extern const struct brw_tracked_state brw_cc_unit;
>   extern const struct brw_tracked_state brw_clip_prog;
>   extern const struct brw_tracked_state brw_clip_unit;
>   extern const struct brw_tracked_state brw_vs_pull_constants;
> +extern const struct brw_tracked_state brw_gs_pull_constants;
>   extern const struct brw_tracked_state brw_wm_pull_constants;
>   extern const struct brw_tracked_state brw_constant_buffer;
>   extern const struct brw_tracked_state brw_curbe_offsets;
> @@ -69,11 +70,13 @@ extern const struct brw_tracked_state brw_urb_fence;
>   extern const struct brw_tracked_state brw_vs_prog;
>   extern const struct brw_tracked_state brw_vs_samplers;
>   extern const struct brw_tracked_state brw_vs_ubo_surfaces;
> +extern const struct brw_tracked_state brw_gs_ubo_surfaces;
>   extern const struct brw_tracked_state brw_vs_unit;
>   extern const struct brw_tracked_state brw_wm_prog;
>   extern const struct brw_tracked_state brw_renderbuffer_surfaces;
>   extern const struct brw_tracked_state brw_texture_surfaces;
>   extern const struct brw_tracked_state brw_wm_binding_table;
> +extern const struct brw_tracked_state brw_gs_binding_table;
>   extern const struct brw_tracked_state brw_vs_binding_table;
>   extern const struct brw_tracked_state brw_wm_ubo_surfaces;
>   extern const struct brw_tracked_state brw_wm_unit;
> diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
> index 64e0bcf..41c11cc 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> @@ -201,11 +201,14 @@ static const struct brw_tracked_state *gen7_atoms[] =
>       */
>      &brw_vs_pull_constants,
>      &brw_vs_ubo_surfaces,
> +   &brw_gs_pull_constants,
> +   &brw_gs_ubo_surfaces,
>      &brw_wm_pull_constants,
>      &brw_wm_ubo_surfaces,
>      &gen6_renderbuffer_surfaces,
>      &brw_texture_surfaces,
>      &brw_vs_binding_table,
> +   &brw_gs_binding_table,
>      &brw_wm_binding_table,
>
>      &brw_fs_samplers,
> diff --git a/src/mesa/drivers/dri/i965/gen6_sol.c b/src/mesa/drivers/dri/i965/gen6_sol.c
> index 5c294b1..5dac257 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sol.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sol.c
> @@ -48,7 +48,7 @@ gen6_update_sol_surfaces(struct brw_context *brw)
>      int i;
>
>      for (i = 0; i < BRW_MAX_SOL_BINDINGS; ++i) {
> -      const int surf_index = SURF_INDEX_SOL_BINDING(i);
> +      const int surf_index = SURF_INDEX_GEN6_SOL_BINDING(i);
>         if (_mesa_is_xfb_active_and_unpaused(ctx) &&
>             i < linked_xfb_info->NumOutputs) {
>            unsigned buffer = linked_xfb_info->Outputs[i].OutputBuffer;
> @@ -112,11 +112,11 @@ brw_gs_upload_binding_table(struct brw_context *brw)
>       * space for the binding table.
>       */
>      bind = brw_state_batch(brw, AUB_TRACE_BINDING_TABLE,
> -			  sizeof(uint32_t) * BRW_MAX_GS_SURFACES,
> +			  sizeof(uint32_t) * BRW_MAX_GEN6_GS_SURFACES,
>   			  32, &brw->gs.bind_bo_offset);
>
>      /* BRW_NEW_SURFACES */
> -   memcpy(bind, brw->gs.surf_offset, BRW_MAX_GS_SURFACES * sizeof(uint32_t));
> +   memcpy(bind, brw->gs.surf_offset, BRW_MAX_GEN6_GS_SURFACES * sizeof(uint32_t));
>
>      brw->state.dirty.brw |= BRW_NEW_GS_BINDING_TABLE;
>   }
>



More information about the mesa-dev mailing list