This patch is Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
On 12/02/2014 03:51 AM, Kenneth Graunke wrote: > We use IEEE mode for GLSL programs, but need to use ALT mode for ARB > programs so that 0^0 == 1. The choice is based entirely on the shader > source language. > > Previously, our code to determine which mode we wanted was duplicated > in 8 different places (VS and FS for Gen4-5, Gen6, Gen7, and Gen8). > The ctx->_Shader->CurrentProgram[stage] == NULL check was confusing > as well - we use CurrentProgram (non-derived state), but _Shader > (derived state). It also relies on knowing that ARB programs don't > use gl_shader_program structures today. The compiler already makes > this assumption in a few places, but I'd rather keep that assumption > out of the state upload code. > > With this patch, we select the mode at compile time, and store that > choice in prog_data. The state upload code simply uses that decision. > > This eliminates a BRW_NEW_*_PROGRAM dependency in the state upload code. > > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > --- > src/mesa/drivers/dri/i965/brw_context.h | 2 ++ > src/mesa/drivers/dri/i965/brw_vs.c | 4 ++++ > src/mesa/drivers/dri/i965/brw_vs_state.c | 5 +---- > src/mesa/drivers/dri/i965/brw_wm.c | 4 ++++ > src/mesa/drivers/dri/i965/brw_wm_state.c | 7 +------ > src/mesa/drivers/dri/i965/gen6_vs_state.c | 6 +----- > src/mesa/drivers/dri/i965/gen6_wm_state.c | 7 +------ > src/mesa/drivers/dri/i965/gen7_vs_state.c | 6 +----- > src/mesa/drivers/dri/i965/gen7_wm_state.c | 8 +------- > src/mesa/drivers/dri/i965/gen8_ps_state.c | 7 +------ > src/mesa/drivers/dri/i965/gen8_vs_state.c | 5 +---- > 11 files changed, 18 insertions(+), 43 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > b/src/mesa/drivers/dri/i965/brw_context.h > index b4ddc17..4bb3b17 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.h > +++ b/src/mesa/drivers/dri/i965/brw_context.h > @@ -355,6 +355,8 @@ struct brw_stage_prog_data { > */ > unsigned dispatch_grf_start_reg; > > + bool use_alt_mode; /**< Use ALT floating point mode? Otherwise, IEEE. */ > + > /* Pointers to tracked values (only valid once > * _mesa_load_state_parameters has been called at runtime). > * > diff --git a/src/mesa/drivers/dri/i965/brw_vs.c > b/src/mesa/drivers/dri/i965/brw_vs.c > index 2f628e5..970d86c 100644 > --- a/src/mesa/drivers/dri/i965/brw_vs.c > +++ b/src/mesa/drivers/dri/i965/brw_vs.c > @@ -209,6 +209,10 @@ do_vs_prog(struct brw_context *brw, > memcpy(&c.key, key, sizeof(*key)); > memset(&prog_data, 0, sizeof(prog_data)); > > + /* Use ALT floating point mode for ARB programs so that 0^0 == 1. */ > + if (!prog) > + stage_prog_data->use_alt_mode = true; > + > mem_ctx = ralloc_context(NULL); > > c.vp = vp; > diff --git a/src/mesa/drivers/dri/i965/brw_vs_state.c > b/src/mesa/drivers/dri/i965/brw_vs_state.c > index abd6771..5371f71 100644 > --- a/src/mesa/drivers/dri/i965/brw_vs_state.c > +++ b/src/mesa/drivers/dri/i965/brw_vs_state.c > @@ -57,10 +57,7 @@ brw_upload_vs_unit(struct brw_context *brw) > stage_state->prog_offset + > (vs->thread0.grf_reg_count << 1)) >> 6; > > - /* Use ALT floating point mode for ARB vertex programs, because they > - * require 0^0 == 1. > - */ > - if (brw->ctx._Shader->CurrentProgram[MESA_SHADER_VERTEX] == NULL) > + if (brw->vs.prog_data->base.base.use_alt_mode) > vs->thread1.floating_point_mode = BRW_FLOATING_POINT_NON_IEEE_754; > else > vs->thread1.floating_point_mode = BRW_FLOATING_POINT_IEEE_754; > diff --git a/src/mesa/drivers/dri/i965/brw_wm.c > b/src/mesa/drivers/dri/i965/brw_wm.c > index 7badb23..e7939f0 100644 > --- a/src/mesa/drivers/dri/i965/brw_wm.c > +++ b/src/mesa/drivers/dri/i965/brw_wm.c > @@ -182,6 +182,10 @@ bool do_wm_prog(struct brw_context *brw, > > prog_data.computed_depth_mode = computed_depth_mode(&fp->program); > > + /* Use ALT floating point mode for ARB programs so that 0^0 == 1. */ > + if (!prog) > + prog_data.base.use_alt_mode = true; > + > /* Allocate the references to the uniforms that will end up in the > * prog_data associated with the compiled program, and which will be freed > * by the state cache. > diff --git a/src/mesa/drivers/dri/i965/brw_wm_state.c > b/src/mesa/drivers/dri/i965/brw_wm_state.c > index d2903c7..0dee1f8 100644 > --- a/src/mesa/drivers/dri/i965/brw_wm_state.c > +++ b/src/mesa/drivers/dri/i965/brw_wm_state.c > @@ -114,12 +114,7 @@ brw_upload_wm_unit(struct brw_context *brw) > (wm->wm9.grf_reg_count_2 << 1)) >> 6; > > wm->thread1.depth_coef_urb_read_offset = 1; > - /* Use ALT floating point mode for ARB fragment programs, because they > - * require 0^0 == 1. Even though _CurrentFragmentProgram is used for > - * rendering, CurrentProgram[MESA_SHADER_FRAGMENT] is used for this check > - * to differentiate between the GLSL and non-GLSL cases. > - */ > - if (ctx->_Shader->CurrentProgram[MESA_SHADER_FRAGMENT] == NULL) > + if (prog_data->base.use_alt_mode) > wm->thread1.floating_point_mode = BRW_FLOATING_POINT_NON_IEEE_754; > else > wm->thread1.floating_point_mode = BRW_FLOATING_POINT_IEEE_754; > diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c > b/src/mesa/drivers/dri/i965/gen6_vs_state.c > index fc0fd1d..e365cc6 100644 > --- a/src/mesa/drivers/dri/i965/gen6_vs_state.c > +++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c > @@ -160,7 +160,6 @@ const struct brw_tracked_state gen6_vs_push_constants = { > static void > upload_vs_state(struct brw_context *brw) > { > - struct gl_context *ctx = &brw->ctx; > const struct brw_stage_state *stage_state = &brw->vs.base; > uint32_t floating_point_mode = 0; > > @@ -202,10 +201,7 @@ upload_vs_state(struct brw_context *brw) > ADVANCE_BATCH(); > } > > - /* Use ALT floating point mode for ARB vertex programs, because they > - * require 0^0 == 1. > - */ > - if (ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX] == NULL) > + if (brw->vs.prog_data->base.base.use_alt_mode) > floating_point_mode = GEN6_VS_FLOATING_POINT_MODE_ALT; > > BEGIN_BATCH(6); > diff --git a/src/mesa/drivers/dri/i965/gen6_wm_state.c > b/src/mesa/drivers/dri/i965/gen6_wm_state.c > index 4ac91af..e57b7f6 100644 > --- a/src/mesa/drivers/dri/i965/gen6_wm_state.c > +++ b/src/mesa/drivers/dri/i965/gen6_wm_state.c > @@ -115,12 +115,7 @@ upload_wm_state(struct brw_context *brw) > dw5 |= GEN6_WM_LINE_AA_WIDTH_1_0; > dw5 |= GEN6_WM_LINE_END_CAP_AA_WIDTH_0_5; > > - /* Use ALT floating point mode for ARB fragment programs, because they > - * require 0^0 == 1. Even though _CurrentFragmentProgram is used for > - * rendering, CurrentProgram[MESA_SHADER_FRAGMENT] is used for this check > - * to differentiate between the GLSL and non-GLSL cases. > - */ > - if (ctx->_Shader->CurrentProgram[MESA_SHADER_FRAGMENT] == NULL) > + if (prog_data->base.use_alt_mode) > dw2 |= GEN6_WM_FLOATING_POINT_MODE_ALT; > > dw2 |= (ALIGN(brw->wm.base.sampler_count, 4) / 4) << > diff --git a/src/mesa/drivers/dri/i965/gen7_vs_state.c > b/src/mesa/drivers/dri/i965/gen7_vs_state.c > index 3b0126d..5a11588 100644 > --- a/src/mesa/drivers/dri/i965/gen7_vs_state.c > +++ b/src/mesa/drivers/dri/i965/gen7_vs_state.c > @@ -66,7 +66,6 @@ gen7_upload_constant_state(struct brw_context *brw, > static void > upload_vs_state(struct brw_context *brw) > { > - struct gl_context *ctx = &brw->ctx; > const struct brw_stage_state *stage_state = &brw->vs.base; > uint32_t floating_point_mode = 0; > const int max_threads_shift = brw->is_haswell ? > @@ -75,10 +74,7 @@ upload_vs_state(struct brw_context *brw) > if (!brw->is_haswell && !brw->is_baytrail) > gen7_emit_vs_workaround_flush(brw); > > - /* Use ALT floating point mode for ARB vertex programs, because they > - * require 0^0 == 1. > - */ > - if (ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX] == NULL) > + if (brw->vs.prog_data->base.base.use_alt_mode) > floating_point_mode = GEN6_VS_FLOATING_POINT_MODE_ALT; > > BEGIN_BATCH(6); > diff --git a/src/mesa/drivers/dri/i965/gen7_wm_state.c > b/src/mesa/drivers/dri/i965/gen7_wm_state.c > index 5a5c726..923414e 100644 > --- a/src/mesa/drivers/dri/i965/gen7_wm_state.c > +++ b/src/mesa/drivers/dri/i965/gen7_wm_state.c > @@ -141,13 +141,7 @@ upload_ps_state(struct brw_context *brw) > dw2 |= ((prog_data->base.binding_table.size_bytes / 4) << > GEN7_PS_BINDING_TABLE_ENTRY_COUNT_SHIFT); > > - /* Use ALT floating point mode for ARB fragment programs, because they > - * require 0^0 == 1. Even though _CurrentFragmentProgram is used for > - * rendering, CurrentProgram[MESA_SHADER_FRAGMENT] is used for this check > - * to differentiate between the GLSL and non-GLSL cases. > - */ > - /* BRW_NEW_FRAGMENT_PROGRAM */ > - if (ctx->_Shader->CurrentProgram[MESA_SHADER_FRAGMENT] == NULL) > + if (prog_data->base.use_alt_mode) > dw2 |= GEN7_PS_FLOATING_POINT_MODE_ALT; > > /* Haswell requires the sample mask to be set in this packet as well as > diff --git a/src/mesa/drivers/dri/i965/gen8_ps_state.c > b/src/mesa/drivers/dri/i965/gen8_ps_state.c > index a3ce1d4..d4a58e4 100644 > --- a/src/mesa/drivers/dri/i965/gen8_ps_state.c > +++ b/src/mesa/drivers/dri/i965/gen8_ps_state.c > @@ -141,12 +141,7 @@ upload_ps_state(struct brw_context *brw) > ((prog_data->base.binding_table.size_bytes / 4) << > GEN7_PS_BINDING_TABLE_ENTRY_COUNT_SHIFT); > > - /* Use ALT floating point mode for ARB fragment programs, because they > - * require 0^0 == 1. Even though _CurrentFragmentProgram is used for > - * rendering, CurrentFragmentProgram is used for this check to > - * differentiate between the GLSL and non-GLSL cases. > - */ > - if (ctx->_Shader->CurrentProgram[MESA_SHADER_FRAGMENT] == NULL) > + if (prog_data->base.use_alt_mode) > dw3 |= GEN7_PS_FLOATING_POINT_MODE_ALT; > > /* 3DSTATE_PS expects the number of threads per PSD, which is always 64; > diff --git a/src/mesa/drivers/dri/i965/gen8_vs_state.c > b/src/mesa/drivers/dri/i965/gen8_vs_state.c > index 5a2021f..e5b7e03 100644 > --- a/src/mesa/drivers/dri/i965/gen8_vs_state.c > +++ b/src/mesa/drivers/dri/i965/gen8_vs_state.c > @@ -39,10 +39,7 @@ upload_vs_state(struct brw_context *brw) > /* BRW_NEW_VS_PROG_DATA */ > const struct brw_vec4_prog_data *prog_data = &brw->vs.prog_data->base; > > - /* Use ALT floating point mode for ARB vertex programs, because they > - * require 0^0 == 1. > - */ > - if (ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX] == NULL) > + if (prog_data->base.use_alt_mode) > floating_point_mode = GEN6_VS_FLOATING_POINT_MODE_ALT; > > BEGIN_BATCH(9); > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev