[Mesa-dev] [PATCH] [v2] i965: Split out gen8 push constant state upload

[Ben Widawsky]

On Thu, Jul 09, 2015 at 09:44:52AM -0700, Ben Widawsky wrote:
> While implementing the workaround in the previous patch I noticed things were
> starting to get a bit messy. Since gen8 works differently enough from gen7, I
> thought splitting it out with be good.
> 
> While here, get rid of gen8 MOCS which does nothing and was in the wrong place
> anyway.
> 
> This patch is totally optional. I'd be willing to just always use buffer #2 on
> gen8+. Pre-HSW this wasn't allowed, but it looks like it's okay for gen8 too.
> 
> v2: Move inactive batch generation to the top of the function in order to make
> the rest of the code easier to read.
> 
> Jenkins results (still a bunch of spurious failures, I miss Mark):
> http://otc-mesa-ci.jf.intel.com/job/bwidawsk/169/
> 
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com> (v1)
> ---
> 
> I had a minor bug in v1 which prevented me from pushing this sooner. I'd like to
> merge this patch unless anyone has complaints?
> 
> ---
>  src/mesa/drivers/dri/i965/brw_state.h     |  6 +-
>  src/mesa/drivers/dri/i965/gen6_gs_state.c |  2 +-
>  src/mesa/drivers/dri/i965/gen6_vs_state.c |  3 +-
>  src/mesa/drivers/dri/i965/gen6_wm_state.c |  3 +-
>  src/mesa/drivers/dri/i965/gen7_vs_state.c | 93 ++++++++++++++++++++-----------
>  5 files changed, 68 insertions(+), 39 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
> index 987672f..f45459d 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -368,9 +368,9 @@ brw_upload_pull_constants(struct brw_context *brw,
>  
>  /* gen7_vs_state.c */
>  void
> -gen7_upload_constant_state(struct brw_context *brw,
> -                           const struct brw_stage_state *stage_state,
> -                           bool active, unsigned opcode);
> +brw_upload_constant_state(struct brw_context *brw,
> +                          const struct brw_stage_state *stage_state,
> +                          bool active, unsigned opcode);
>  
>  #ifdef __cplusplus
>  }
> diff --git a/src/mesa/drivers/dri/i965/gen6_gs_state.c b/src/mesa/drivers/dri/i965/gen6_gs_state.c
> index eb4c586..19568b0 100644
> --- a/src/mesa/drivers/dri/i965/gen6_gs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_gs_state.c
> @@ -48,7 +48,7 @@ gen6_upload_gs_push_constants(struct brw_context *brw)
>     }
>  
>     if (brw->gen >= 7)
> -      gen7_upload_constant_state(brw, stage_state, gp, _3DSTATE_CONSTANT_GS);
> +      brw_upload_constant_state(brw, stage_state, gp, _3DSTATE_CONSTANT_GS);
>  }
>  
>  const struct brw_tracked_state gen6_gs_push_constants = {
> diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> index 35d10ef..c33607d 100644
> --- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> @@ -140,8 +140,7 @@ gen6_upload_vs_push_constants(struct brw_context *brw)
>        if (brw->gen == 7 && !brw->is_haswell && !brw->is_baytrail)
>           gen7_emit_vs_workaround_flush(brw);
>  
> -      gen7_upload_constant_state(brw, stage_state, true /* active */,
> -                                 _3DSTATE_CONSTANT_VS);
> +      brw_upload_constant_state(brw, stage_state, true, _3DSTATE_CONSTANT_VS);
>     }
>  }
>  
> diff --git a/src/mesa/drivers/dri/i965/gen6_wm_state.c b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> index d1748ba..ced4ad6 100644
> --- a/src/mesa/drivers/dri/i965/gen6_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> @@ -50,8 +50,7 @@ gen6_upload_wm_push_constants(struct brw_context *brw)
>                                stage_state, AUB_TRACE_WM_CONSTANTS);
>  
>     if (brw->gen >= 7) {
> -      gen7_upload_constant_state(brw, &brw->wm.base, true,
> -                                 _3DSTATE_CONSTANT_PS);
> +      brw_upload_constant_state(brw, &brw->wm.base, true, _3DSTATE_CONSTANT_PS);
>     }
>  }
>  
> diff --git a/src/mesa/drivers/dri/i965/gen7_vs_state.c b/src/mesa/drivers/dri/i965/gen7_vs_state.c
> index 4b17d06..6a51934 100644
> --- a/src/mesa/drivers/dri/i965/gen7_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_vs_state.c
> @@ -29,20 +29,23 @@
>  #include "program/prog_statevars.h"
>  #include "intel_batchbuffer.h"
>  
> -
> -void
> -gen7_upload_constant_state(struct brw_context *brw,
> +static void
> +gen8_upload_constant_state(struct brw_context *brw,
>                             const struct brw_stage_state *stage_state,
>                             bool active, unsigned opcode)
>  {
> -   uint32_t mocs = brw->gen < 8 ? GEN7_MOCS_L3 : 0;
>  
> -   /* Disable if the shader stage is inactive or there are no push constants. */
> -   active = active && stage_state->push_const_size != 0;
> +   /* FINISHME: determine if we should use mocs on gen9 */
>  
> -   int dwords = brw->gen >= 8 ? 11 : 7;
> -   BEGIN_BATCH(dwords);
> -   OUT_BATCH(opcode << 16 | (dwords - 2));
> +   BEGIN_BATCH(11);
> +   OUT_BATCH(opcode << 16 | (11 - 2));
> +
> +   if (!active) {
> +      for (int i = 0; i < 11; i++)
> +         OUT_BATCH(0);
> +
> +      return;
> +   }

I just noticed this bug. I wonder why nothing failed? v3 coming up after another
jenkins run.