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

Anuj Phogat anuj.phogat at gmail.com
Tue Jun 23 10:50:35 PDT 2015


On Wed, Jun 3, 2015 at 9:35 PM, Ben Widawsky
<benjamin.widawsky at intel.com> 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.
>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  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 | 82 ++++++++++++++++++++-----------
>  5 files changed, 59 insertions(+), 37 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 7081eb7..1cfd1ad 100644
> --- a/src/mesa/drivers/dri/i965/gen6_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> @@ -49,8 +49,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..091df53 100644
> --- a/src/mesa/drivers/dri/i965/gen7_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_vs_state.c
> @@ -29,20 +29,16 @@
>  #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;
> +   /* MOCS is optional for GEN9, but it isn't allowed for GEN8 */
>
> -   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));
>
>     /* Workaround for SKL+ (we use option #2 until we have a need for more
>      * constant buffers). This comes from the documentation for 3DSTATE_CONSTANT_*
> @@ -57,42 +53,40 @@ gen7_upload_constant_state(struct brw_context *brw,
>      */
>     if (brw->gen >= 9 && active) {
>        OUT_BATCH(0);
> -      OUT_BATCH(stage_state->push_const_size);
> +      OUT_BATCH(stage_state->push_const_size); /* buffer 3, 2 */
>     } else {
> -      OUT_BATCH(active ? stage_state->push_const_size : 0);
> +      OUT_BATCH(active ? stage_state->push_const_size : 0); /* buffer 1, 0 */
>        OUT_BATCH(0);
>     }
> -   /* Pointer to the constant buffer.  Covered by the set of state flags
> -    * from gen6_prepare_wm_contants
> -    */
> -   if (brw->gen >= 9 && active) {
> -      OUT_BATCH(0);
> +
> +   /* Push constant buffer #0 */
> +   if (brw->gen >= 9 || !active) {
>        OUT_BATCH(0);
>        OUT_BATCH(0);
> +   } else {
> +      OUT_BATCH(stage_state->push_const_offset);
>        OUT_BATCH(0);
> +   }
> +
> +   /* Push constant buffer #1 */
> +   OUT_BATCH(0);
> +   OUT_BATCH(0);
> +
> +   /* Push constant buffer #2 */
> +   if (brw->gen >= 9 && active) {
>        /* XXX: When using buffers other than 0, you need to specify the
>         * graphics virtual address regardless of INSPM/debug bits
>         */
>        OUT_RELOC64(brw->batch.bo, I915_GEM_DOMAIN_RENDER, 0,
>                    stage_state->push_const_offset);
> -      OUT_BATCH(0);
> -      OUT_BATCH(0);
> -   } else if (brw->gen>= 8) {
> -      OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0);
> -      OUT_BATCH(0);
> -      OUT_BATCH(0);
> -      OUT_BATCH(0);
> -      OUT_BATCH(0);
> -      OUT_BATCH(0);
> -      OUT_BATCH(0);
> -      OUT_BATCH(0);
>     } else {
> -      OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0);
> -      OUT_BATCH(0);
>        OUT_BATCH(0);
>        OUT_BATCH(0);
>     }
>
> +   /* Push constant buffer #3 */
> +   OUT_BATCH(0);
> +   OUT_BATCH(0);
>     ADVANCE_BATCH();
>
>    /* On SKL+ the new constants don't take effect until the next corresponding
> @@ -103,6 +97,36 @@ gen7_upload_constant_state(struct brw_context *brw,
>        brw->ctx.NewDriverState |= BRW_NEW_SURFACES;
>  }
>
> +static void
> +gen7_upload_constant_state(struct brw_context *brw,
> +                           const struct brw_stage_state *stage_state,
> +                           bool active, unsigned opcode)
> +{
> +   BEGIN_BATCH(7);
> +   OUT_BATCH(opcode << 16 | (7 - 2));
> +   OUT_BATCH(active ? stage_state->push_const_size : 0);
> +   OUT_BATCH(0);
> +   OUT_BATCH(active ? (stage_state->push_const_offset | GEN7_MOCS_L3) : 0);
> +   OUT_BATCH(0);
> +   OUT_BATCH(0);
> +   OUT_BATCH(0);
> +   ADVANCE_BATCH();
> +}
> +
> +void
> +brw_upload_constant_state(struct brw_context *brw,
> +                          const struct brw_stage_state *stage_state,
> +                          bool active, unsigned opcode)
> +{
> +   /* Disable if the shader stage is inactive or there are no push constants. */
> +   active = active && stage_state->push_const_size != 0;
> +
> +   if (brw->gen >= 8)
> +      gen8_upload_constant_state(brw, stage_state, active, opcode);
> +   else
> +      gen7_upload_constant_state(brw, stage_state, active, opcode);
> +}
> +
>
>  static void
>  upload_vs_state(struct brw_context *brw)
> --
> 2.4.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

LGTM.
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>


More information about the mesa-dev mailing list