On Wed, Jun 3, 2015 at 9:35 PM, Ben Widawsky
<benjamin.widaw...@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 <b...@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
LGTM.
Reviewed-by: Anuj Phogat <anuj.pho...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
