On Fri, May 29, 2015 at 12:26:39PM -0700, Kenneth Graunke wrote: > We used to store the GS dispatch mode in brw_gs_prog_data while > separately storing the VS dispatch mode in brw_vue_prog_data::simd8. > > This patch introduces an enum to represent all possible dispatch modes, > and stores it in brw_vue_prog_data::dispatch_mode, unifying the two. > > Based on a suggestion by Matt Turner. > > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > --- > src/mesa/drivers/dri/i965/brw_context.h | 16 +++++++--------- > src/mesa/drivers/dri/i965/brw_defines.h | 5 ++--- > src/mesa/drivers/dri/i965/brw_vec4.cpp | 5 ++++- > src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 8 ++++---- > src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 4 ++-- > src/mesa/drivers/dri/i965/gen7_gs_state.c | 2 +- > src/mesa/drivers/dri/i965/gen8_gs_state.c | 3 ++- > src/mesa/drivers/dri/i965/gen8_vs_state.c | 3 ++- > 8 files changed, 24 insertions(+), 22 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > b/src/mesa/drivers/dri/i965/brw_context.h > index abc11f6..01c4283 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.h > +++ b/src/mesa/drivers/dri/i965/brw_context.h > @@ -605,6 +605,12 @@ struct brw_ff_gs_prog_data { > unsigned svbi_postincrement_value; > }; > > +enum shader_dispatch_mode { > + DISPATCH_MODE_4X1_SINGLE = 0, > + DISPATCH_MODE_4X2_DUAL_INSTANCE = 1, > + DISPATCH_MODE_4X2_DUAL_OBJECT = 2, > + DISPATCH_MODE_SIMD8 = 3, > +}; > > /* Note: brw_vue_prog_data_compare() must be updated when adding fields to > * this struct! > @@ -622,7 +628,7 @@ struct brw_vue_prog_data { > */ > GLuint urb_entry_size; > > - bool simd8; > + enum shader_dispatch_mode dispatch_mode; > }; > > > @@ -720,14 +726,6 @@ struct brw_gs_prog_data > int invocations; > > /** > - * Dispatch mode, can be any of: > - * GEN7_GS_DISPATCH_MODE_DUAL_OBJECT > - * GEN7_GS_DISPATCH_MODE_DUAL_INSTANCE > - * GEN7_GS_DISPATCH_MODE_SINGLE > - */ > - int dispatch_mode; > - > - /** > * Gen6 transform feedback enabled flag. > */ > bool gen6_xfb_enabled; > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h > b/src/mesa/drivers/dri/i965/brw_defines.h > index dedc381..f6da305 100644 > --- a/src/mesa/drivers/dri/i965/brw_defines.h > +++ b/src/mesa/drivers/dri/i965/brw_defines.h > @@ -1773,9 +1773,8 @@ enum brw_message_target { > # define GEN7_GS_CONTROL_DATA_FORMAT_GSCTL_SID 1 > # define GEN7_GS_CONTROL_DATA_HEADER_SIZE_SHIFT 20 > # define GEN7_GS_INSTANCE_CONTROL_SHIFT 15 > -# define GEN7_GS_DISPATCH_MODE_SINGLE (0 << 11) > -# define GEN7_GS_DISPATCH_MODE_DUAL_INSTANCE (1 << 11) > -# define GEN7_GS_DISPATCH_MODE_DUAL_OBJECT (2 << 11) > +# define GEN7_GS_DISPATCH_MODE_SHIFT 11 > +# define GEN7_GS_DISPATCH_MODE_MASK INTEL_MASK(12, 11) > # define GEN6_GS_STATISTICS_ENABLE (1 << 10) > # define GEN6_GS_SO_STATISTICS_ENABLE (1 << 9) > # define GEN6_GS_RENDERING_ENABLE (1 << 8) > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp > b/src/mesa/drivers/dri/i965/brw_vec4.cpp > index a324798..5a9c3f5 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp > @@ -1894,6 +1894,8 @@ brw_vs_emit(struct brw_context *brw, > brw_create_nir(brw, NULL, &c->vp->program.Base, > MESA_SHADER_VERTEX); > } > > + prog_data->base.dispatch_mode = DISPATCH_MODE_SIMD8; > + > fs_visitor v(brw, mem_ctx, MESA_SHADER_VERTEX, &c->key, > &prog_data->base.base, prog, &c->vp->program.Base, 8); > if (!v.run_vs()) { > @@ -1926,11 +1928,12 @@ brw_vs_emit(struct brw_context *brw, > g.generate_code(v.cfg, 8); > assembly = g.get_assembly(final_assembly_size); > > - prog_data->base.simd8 = true; > c->base.last_scratch = v.last_scratch;
for whatever it's worth, it would have made more sense to put the dispatch_mode change here. > } > > if (!assembly) { > + prog_data->base.dispatch_mode = DISPATCH_MODE_4X2_DUAL_OBJECT; > + > vec4_vs_visitor v(brw, c, prog_data, prog, mem_ctx); > if (!v.run()) { > if (prog) { > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > index 363e30e..eacb2f5 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > @@ -106,7 +106,7 @@ vec4_gs_visitor::setup_payload() > * to be interleaved, so one register contains two attribute slots. > */ > int attributes_per_reg = > - c->prog_data.dispatch_mode == GEN7_GS_DISPATCH_MODE_DUAL_OBJECT ? 1 : > 2; > + c->prog_data.base.dispatch_mode == DISPATCH_MODE_4X2_DUAL_OBJECT ? 1 : > 2; > > /* If a geometry shader tries to read from an input that wasn't written by > * the vertex shader, that produces undefined results, but it shouldn't > @@ -655,7 +655,7 @@ brw_gs_emit(struct brw_context *brw, > */ > if (c->prog_data.invocations <= 1 && > likely(!(INTEL_DEBUG & DEBUG_NO_DUAL_OBJECT_GS))) { > - c->prog_data.dispatch_mode = GEN7_GS_DISPATCH_MODE_DUAL_OBJECT; > + c->prog_data.base.dispatch_mode = DISPATCH_MODE_4X2_DUAL_OBJECT; > > vec4_gs_visitor v(brw, c, prog, mem_ctx, true /* no_spills */); > if (v.run()) { > @@ -690,9 +690,9 @@ brw_gs_emit(struct brw_context *brw, > * SINGLE mode. > */ > if (c->prog_data.invocations <= 1 || brw->gen < 7) > - c->prog_data.dispatch_mode = GEN7_GS_DISPATCH_MODE_SINGLE; > + c->prog_data.base.dispatch_mode = DISPATCH_MODE_4X1_SINGLE; > else > - c->prog_data.dispatch_mode = GEN7_GS_DISPATCH_MODE_DUAL_INSTANCE; > + c->prog_data.base.dispatch_mode = DISPATCH_MODE_4X2_DUAL_INSTANCE; > > vec4_gs_visitor *gs = NULL; > const unsigned *ret = NULL; > 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 f82a62b..b2f91bd 100644 > --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c > +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c > @@ -121,7 +121,7 @@ brw_upload_vs_pull_constants(struct brw_context *brw) > /* BRW_NEW_VS_PROG_DATA */ > const struct brw_stage_prog_data *prog_data = > &brw->vs.prog_data->base.base; > > - dword_pitch = brw->vs.prog_data->base.simd8; > + dword_pitch = brw->vs.prog_data->base.dispatch_mode == > DISPATCH_MODE_SIMD8; > > /* _NEW_PROGRAM_CONSTANTS */ > brw_upload_pull_constants(brw, BRW_NEW_VS_CONSTBUF, &vp->program.Base, > @@ -151,7 +151,7 @@ brw_upload_vs_ubo_surfaces(struct brw_context *brw) > return; > > /* BRW_NEW_VS_PROG_DATA */ > - dword_pitch = brw->vs.prog_data->base.simd8; > + dword_pitch = brw->vs.prog_data->base.dispatch_mode == > DISPATCH_MODE_SIMD8; > brw_upload_ubo_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_VERTEX], > &brw->vs.base, &brw->vs.prog_data->base.base, > dword_pitch); Seems like it might be worthwhile to remove dword_pitch altogether now and pass down the vue by itself, but I'm fine with either. > diff --git a/src/mesa/drivers/dri/i965/gen7_gs_state.c > b/src/mesa/drivers/dri/i965/gen7_gs_state.c > index e1c4f8b..8d6d3fe 100644 > --- a/src/mesa/drivers/dri/i965/gen7_gs_state.c > +++ b/src/mesa/drivers/dri/i965/gen7_gs_state.c > @@ -112,7 +112,7 @@ upload_gs_state(struct brw_context *brw) > GEN7_GS_CONTROL_DATA_HEADER_SIZE_SHIFT) | > ((brw->gs.prog_data->invocations - 1) << > GEN7_GS_INSTANCE_CONTROL_SHIFT) | > - brw->gs.prog_data->dispatch_mode | > + SET_FIELD(prog_data->dispatch_mode, GEN7_GS_DISPATCH_MODE) | > GEN6_GS_STATISTICS_ENABLE | > (brw->gs.prog_data->include_primitive_id ? > GEN7_GS_INCLUDE_PRIMITIVE_ID : 0) | > diff --git a/src/mesa/drivers/dri/i965/gen8_gs_state.c > b/src/mesa/drivers/dri/i965/gen8_gs_state.c > index 0763e91..26a02d3 100644 > --- a/src/mesa/drivers/dri/i965/gen8_gs_state.c > +++ b/src/mesa/drivers/dri/i965/gen8_gs_state.c > @@ -76,7 +76,8 @@ gen8_upload_gs_state(struct brw_context *brw) > > uint32_t dw7 = (brw->gs.prog_data->control_data_header_size_hwords << > GEN7_GS_CONTROL_DATA_HEADER_SIZE_SHIFT) | > - brw->gs.prog_data->dispatch_mode | > + SET_FIELD(prog_data->dispatch_mode, > + GEN7_GS_DISPATCH_MODE) | > ((brw->gs.prog_data->invocations - 1) << > GEN7_GS_INSTANCE_CONTROL_SHIFT) | > GEN6_GS_STATISTICS_ENABLE | > diff --git a/src/mesa/drivers/dri/i965/gen8_vs_state.c > b/src/mesa/drivers/dri/i965/gen8_vs_state.c > index f92af55..9bfde38 100644 > --- a/src/mesa/drivers/dri/i965/gen8_vs_state.c > +++ b/src/mesa/drivers/dri/i965/gen8_vs_state.c > @@ -66,7 +66,8 @@ upload_vs_state(struct brw_context *brw) > (prog_data->urb_read_length << GEN6_VS_URB_READ_LENGTH_SHIFT) | > (0 << GEN6_VS_URB_ENTRY_READ_OFFSET_SHIFT)); > > - uint32_t simd8_enable = prog_data->simd8 ? GEN8_VS_SIMD8_ENABLE : 0; > + uint32_t simd8_enable = prog_data->dispatch_mode == DISPATCH_MODE_SIMD8 ? > + GEN8_VS_SIMD8_ENABLE : 0; I wouldn't mind an assert here that it's not one of those modes which shouldn't be used by VS, but I am assert happy... > OUT_BATCH(((brw->max_vs_threads - 1) << HSW_VS_MAX_THREADS_SHIFT) | > GEN6_VS_STATISTICS_ENABLE | > simd8_enable | Reviewed-by: Ben Widawsky <b...@bwidawsk.net> -- Ben Widawsky, Intel Open Source Technology Center _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev