[Mesa-dev] [PATCH v2 3/5] i965: Use brw.vue_map_geom_out instead of VS output VUE map where appropriate.
Kenneth Graunke
kenneth at whitecape.org
Sat Mar 23 10:59:45 PDT 2013
On 03/21/2013 05:40 PM, Paul Berry wrote:
> This patch modifies post-GS pipeline stages (transform feedback, clip,
> sf, fs) to refer to the VUE map through brw->vue_map_geom_out rather
> than brw->vs.prog_data->vue_map. This ensures that when geometry
> shader support is added, these pipeline stages will consult the
> geometry shader output VUE map when appropriate, rather than the
> vertex shader output VUE map.
> ---
> src/mesa/drivers/dri/i965/brw_clip.c | 7 +++----
> src/mesa/drivers/dri/i965/brw_sf.c | 7 +++----
> src/mesa/drivers/dri/i965/brw_state.h | 2 +-
> src/mesa/drivers/dri/i965/brw_wm.c | 6 +++---
> src/mesa/drivers/dri/i965/gen6_sf_state.c | 10 +++++-----
> src/mesa/drivers/dri/i965/gen7_sf_state.c | 8 ++++----
> src/mesa/drivers/dri/i965/gen7_sol_state.c | 14 +++++++-------
> 7 files changed, 26 insertions(+), 28 deletions(-)
I heartily approve of this patch :) It really untangles the mess of VS
dependencies.
Some comments below...
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.c b/src/mesa/drivers/dri/i965/brw_clip.c
> index e20f7c2..fa7e85d 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip.c
> @@ -69,7 +69,7 @@ static void compile_clip_prog( struct brw_context *brw,
> c.func.single_program_flow = 1;
>
> c.key = *key;
> - c.vue_map = brw->vs.prog_data->vue_map;
> + c.vue_map = brw->vue_map_geom_out;
>
> /* nr_regs is the number of registers filled by reading data from the VUE.
> * This program accesses the entire VUE, so nr_regs needs to be the size of
> @@ -146,7 +146,7 @@ brw_upload_clip_prog(struct brw_context *brw)
> /* BRW_NEW_REDUCED_PRIMITIVE */
> key.primitive = brw->intel.reduced_primitive;
> /* CACHE_NEW_VS_PROG (also part of VUE map) */
> - key.attrs = brw->vs.prog_data->vue_map.slots_valid;
> + key.attrs = brw->vue_map_geom_out.slots_valid;
This comment is now wrong. It should be
/* BRW_NEW_VUE_MAP_GEOM_OUT */ not /* CACHE_NEW_VS_PROG */.
> /* _NEW_LIGHT */
> key.do_flat_shading = (ctx->Light.ShadeModel == GL_FLAT);
> key.pv_first = (ctx->Light.ProvokingVertex == GL_FIRST_VERTEX_CONVENTION);
> @@ -258,8 +258,7 @@ const struct brw_tracked_state brw_clip_prog = {
> _NEW_TRANSFORM |
> _NEW_POLYGON |
> _NEW_BUFFERS),
> - .brw = (BRW_NEW_REDUCED_PRIMITIVE),
> - .cache = CACHE_NEW_VS_PROG
...and I think this was actually the last use of CACHE_NEW_VS_PROG, so
you can probably eliminate that.
> + .brw = (BRW_NEW_REDUCED_PRIMITIVE | BRW_NEW_VUE_MAP_GEOM_OUT)
> },
> .emit = brw_upload_clip_prog
> };
> diff --git a/src/mesa/drivers/dri/i965/brw_sf.c b/src/mesa/drivers/dri/i965/brw_sf.c
> index c8b7033..fc36961 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf.c
> +++ b/src/mesa/drivers/dri/i965/brw_sf.c
> @@ -65,7 +65,7 @@ static void compile_sf_prog( struct brw_context *brw,
> brw_init_compile(brw, &c.func, mem_ctx);
>
> c.key = *key;
> - c.vue_map = brw->vs.prog_data->vue_map;
> + c.vue_map = brw->vue_map_geom_out;
> if (c.key.do_point_coord) {
> /*
> * gl_PointCoord is a FS instead of VS builtin variable, thus it's
> @@ -145,7 +145,7 @@ brw_upload_sf_prog(struct brw_context *brw)
> /* Populate the key, noting state dependencies:
> */
> /* CACHE_NEW_VS_PROG */
> - key.attrs = brw->vs.prog_data->vue_map.slots_valid;
Ditto (wrong comment)
> + key.attrs = brw->vue_map_geom_out.slots_valid;
>
> /* BRW_NEW_REDUCED_PRIMITIVE */
> switch (brw->intel.reduced_primitive) {
> @@ -216,8 +216,7 @@ const struct brw_tracked_state brw_sf_prog = {
> .dirty = {
> .mesa = (_NEW_HINT | _NEW_LIGHT | _NEW_POLYGON | _NEW_POINT |
> _NEW_TRANSFORM | _NEW_BUFFERS | _NEW_PROGRAM),
> - .brw = (BRW_NEW_REDUCED_PRIMITIVE),
> - .cache = CACHE_NEW_VS_PROG
Ditto (CACHE_NEW_VS_PROG doesn't appear necessary)
> + .brw = (BRW_NEW_REDUCED_PRIMITIVE | BRW_NEW_VUE_MAP_GEOM_OUT)
> },
> .emit = brw_upload_sf_prog
> };
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
> index 02ce57b..1f5e18a 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -227,7 +227,7 @@ void upload_default_color(struct brw_context *brw,
>
> /* gen6_sf_state.c */
> uint32_t
> -get_attr_override(struct brw_vue_map *vue_map, int urb_entry_read_offset,
> +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);
>
> #ifdef __cplusplus
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
> index e7e9ddc..6053f94 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -481,7 +481,7 @@ static void brw_wm_populate_key( struct brw_context *brw,
>
> /* CACHE_NEW_VS_PROG */
> if (intel->gen < 6)
> - key->vp_outputs_written = brw->vs.prog_data->vue_map.slots_valid;
> + key->vp_outputs_written = brw->vue_map_geom_out.slots_valid;
Ditto (wrong comment)
>
> /* The unique fragment program ID */
> key->program_string_id = fp->id;
> @@ -524,8 +524,8 @@ const struct brw_tracked_state brw_wm_prog = {
> _NEW_MULTISAMPLE),
> .brw = (BRW_NEW_FRAGMENT_PROGRAM |
> BRW_NEW_WM_INPUT_DIMENSIONS |
> - BRW_NEW_REDUCED_PRIMITIVE),
> - .cache = CACHE_NEW_VS_PROG,
> + BRW_NEW_REDUCED_PRIMITIVE |
> + BRW_NEW_VUE_MAP_GEOM_OUT)
Hey, you caught it in the rest of the cases :)
I would like to see this series run through Piglit on pre-Gen6 just to
be sure we haven't broken anything. I can do that, as I'm setting up my
Ironlake for other reasons anyway...
> },
> .emit = brw_upload_wm_prog
> };
> diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> index 7fe1dca..4ee7f56 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> @@ -53,7 +53,7 @@
> * 256-bit increments.
> */
> uint32_t
> -get_attr_override(struct brw_vue_map *vue_map, int urb_entry_read_offset,
> +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)
> {
> if (fs_attr == VARYING_SLOT_POS) {
> @@ -312,9 +312,9 @@ upload_sf_state(struct brw_context *brw)
> */
> assert(input_index < 16 || attr == input_index);
>
> - /* CACHE_NEW_VS_PROG | _NEW_LIGHT | _NEW_PROGRAM */
> + /* BRW_NEW_VUE_MAP_GEOM_OUT | _NEW_LIGHT | _NEW_PROGRAM */
> attr_overrides[input_index++] =
> - get_attr_override(&brw->vs.prog_data->vue_map,
> + get_attr_override(&brw->vue_map_geom_out,
> urb_entry_read_offset, attr,
> ctx->VertexProgram._TwoSideEnabled,
> &max_source_attr);
> @@ -370,8 +370,8 @@ const struct brw_tracked_state gen6_sf_state = {
> _NEW_POINT |
> _NEW_MULTISAMPLE),
> .brw = (BRW_NEW_CONTEXT |
> - BRW_NEW_FRAGMENT_PROGRAM),
> - .cache = CACHE_NEW_VS_PROG
> + BRW_NEW_FRAGMENT_PROGRAM |
> + BRW_NEW_VUE_MAP_GEOM_OUT)
> },
> .emit = upload_sf_state,
> };
> diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> index 86809a1..5ebe6f2 100644
> --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> @@ -100,9 +100,9 @@ upload_sbe_state(struct brw_context *brw)
> */
> assert(input_index < 16 || attr == input_index);
>
> - /* CACHE_NEW_VS_PROG | _NEW_LIGHT | _NEW_PROGRAM */
> + /* BRW_NEW_VUE_MAP_GEOM_OUT | _NEW_LIGHT | _NEW_PROGRAM */
> attr_overrides[input_index++] =
> - get_attr_override(&brw->vs.prog_data->vue_map,
> + get_attr_override(&brw->vue_map_geom_out,
> urb_entry_read_offset, attr,
> ctx->VertexProgram._TwoSideEnabled,
> &max_source_attr);
> @@ -149,8 +149,8 @@ const struct brw_tracked_state gen7_sbe_state = {
> _NEW_POINT |
> _NEW_PROGRAM),
> .brw = (BRW_NEW_CONTEXT |
> - BRW_NEW_FRAGMENT_PROGRAM),
> - .cache = CACHE_NEW_VS_PROG
> + BRW_NEW_FRAGMENT_PROGRAM |
> + BRW_NEW_VUE_MAP_GEOM_OUT)
> },
> .emit = upload_sbe_state,
> };
> diff --git a/src/mesa/drivers/dri/i965/gen7_sol_state.c b/src/mesa/drivers/dri/i965/gen7_sol_state.c
> index b55fccc..1c71d0f 100644
> --- a/src/mesa/drivers/dri/i965/gen7_sol_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_sol_state.c
> @@ -107,7 +107,7 @@ upload_3dstate_so_buffers(struct brw_context *brw)
> */
> static void
> upload_3dstate_so_decl_list(struct brw_context *brw,
> - struct brw_vue_map *vue_map)
> + const struct brw_vue_map *vue_map)
Good call on the const.
> {
> struct intel_context *intel = &brw->intel;
> struct gl_context *ctx = &intel->ctx;
> @@ -183,7 +183,7 @@ upload_3dstate_so_decl_list(struct brw_context *brw,
>
> static void
> upload_3dstate_streamout(struct brw_context *brw, bool active,
> - struct brw_vue_map *vue_map)
> + const struct brw_vue_map *vue_map)
> {
> struct intel_context *intel = &brw->intel;
> struct gl_context *ctx = &intel->ctx;
> @@ -241,8 +241,8 @@ upload_sol_state(struct brw_context *brw)
>
> if (active) {
> upload_3dstate_so_buffers(brw);
> - /* CACHE_NEW_VS_PROG */
> - upload_3dstate_so_decl_list(brw, &brw->vs.prog_data->vue_map);
> + /* BRW_NEW_VUE_MAP_GEOM_OUT */
> + upload_3dstate_so_decl_list(brw, &brw->vue_map_geom_out);
>
> intel->batch.needs_sol_reset = true;
> }
> @@ -252,7 +252,7 @@ upload_sol_state(struct brw_context *brw)
> * MMIO register updates (current performed by the kernel at each batch
> * emit).
> */
> - upload_3dstate_streamout(brw, active, &brw->vs.prog_data->vue_map);
> + upload_3dstate_streamout(brw, active, &brw->vue_map_geom_out);
> }
>
> const struct brw_tracked_state gen7_sol_state = {
> @@ -261,8 +261,8 @@ const struct brw_tracked_state gen7_sol_state = {
> _NEW_LIGHT |
> _NEW_TRANSFORM_FEEDBACK),
> .brw = (BRW_NEW_BATCH |
> - BRW_NEW_VERTEX_PROGRAM),
> - .cache = CACHE_NEW_VS_PROG,
> + BRW_NEW_VERTEX_PROGRAM |
> + BRW_NEW_VUE_MAP_GEOM_OUT)
> },
> .emit = upload_sol_state,
> };
More information about the mesa-dev
mailing list