[Mesa-dev] [PATCH 2/3] i965: Create a shader_dispatch_mode enum to replace VS/GS fields.
Ben Widawsky
ben at bwidawsk.net
Fri May 29 15:58:03 PDT 2015
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 <kenneth at 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 <ben at bwidawsk.net>
--
Ben Widawsky, Intel Open Source Technology Center
More information about the mesa-dev
mailing list