On Wednesday, January 24, 2018 4:33:56 PM PST Rafael Antognolli wrote:
> These packets were causing GPU hangs when the context was restored,
> possibly because they were pointing to BO's that were already
> unreferenced. So we tell the hardware to ignore such packets after the
> batch buffer ends, since we know those BO's are not around anymore.
>
> This change fixes GPU hangs on CNL. The (partial) solution to this
> problem so far was to entirely disable push constants on this platform.
>
> Signed-off-by: Rafael Antognolli <rafael.antogno...@intel.com>
> Cc: Kenneth Graunke <kenn...@whitecape.org>
> Cc: "18.0" <mesa-sta...@lists.freedesktop.org>
> ---
> src/mesa/drivers/dri/i965/brw_pipe_control.c | 49
> +++++++++++++++++++++++++++
> src/mesa/drivers/dri/i965/brw_pipe_control.h | 1 +
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 4 +++
> 3 files changed, 54 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> index e28be34c8e8..eb8ada63129 100644
> --- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> @@ -317,6 +317,55 @@ gen7_emit_vs_workaround_flush(struct brw_context *brw)
> brw->workaround_bo, 0, 0);
> }
>
> +/**
> + * From the PRM, Volume 2a:
> + *
> + * "Indirect State Pointers Disable
> + *
> + * At the completion of the post-sync operation associated with this pipe
> + * control packet, the indirect state pointers in the hardware are
> + * considered invalid; the indirect pointers are not saved in the context.
> + * If any new indirect state commands are executed in the command stream
> + * while the pipe control is pending, the new indirect state commands are
> + * preserved.
> + *
> + * [DevIVB+]: Using Invalidate State Pointer (ISP) only inhibits context
> + * restoring of Push Constant (3DSTATE_CONSTANT_*) commands. Push Constant
> + * commands are only considered as Indirect State Pointers. Once ISP is
> + * issued in a context, SW must initialize by programming push constant
> + * commands for all the shaders (at least to zero length) before
> attempting
> + * any rendering operation for the same context."
> + *
> + * 3DSTATE_CONSTANT_* packets are restored during a context restore,
> + * even though they point to a BO that has been already unreferenced at
> + * the end of the previous batch buffer. This has been fine so far since
> + * we are protected by these scratch page (every address not covered by
> + * a BO should be pointing to the scratch page). But on CNL, it is
> + * causing a GPU hang during context restore at the 3DSTATE_CONSTANT_*
> + * instruction.
> + *
> + * The flag "Indirect State Pointers Disable" in PIPE_CONTROL tells the
> + * hardware to ignore previous 3DSTATE_CONSTANT_* packets during a
> + * context restore, so the mentioned hang doesn't happen. However,
> + * software must program push constant commands for all stages prior to
> + * rendering anything, so we flag them as dirty.
> + */
> +void
> +gen10_emit_isp_disable(struct brw_context *brw)
> +{
> + const struct gen_device_info *devinfo = &brw->screen->devinfo;
> +
> + brw_emit_pipe_control_write(brw,
> + PIPE_CONTROL_ISP_DIS |
> + PIPE_CONTROL_WRITE_IMMEDIATE,
> + brw->workaround_bo, 0, 0);
> +
> + brw->vs.base.push_constants_dirty = true;
> + brw->tcs.base.push_constants_dirty = true;
> + brw->tes.base.push_constants_dirty = true;
> + brw->gs.base.push_constants_dirty = true;
> + brw->wm.base.push_constants_dirty = true;
> +}
>
> /**
> * Emit a PIPE_CONTROL command for gen7 with the CS Stall bit set.
> diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.h
> b/src/mesa/drivers/dri/i965/brw_pipe_control.h
> index 6e9a404870d..651cd4d3e70 100644
> --- a/src/mesa/drivers/dri/i965/brw_pipe_control.h
> +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.h
> @@ -85,5 +85,6 @@ void brw_emit_post_sync_nonzero_flush(struct brw_context
> *brw);
> void brw_emit_depth_stall_flushes(struct brw_context *brw);
> void gen7_emit_vs_workaround_flush(struct brw_context *brw);
> void gen7_emit_cs_stall_flush(struct brw_context *brw);
> +void gen10_emit_isp_disable(struct brw_context *brw);
>
> #endif
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 02bfd3f333c..86d88a701f8 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -764,6 +764,10 @@ brw_finish_batch(struct brw_context *brw)
> brw_emit_pipe_control_flush(brw, PIPE_CONTROL_RENDER_TARGET_FLUSH |
> PIPE_CONTROL_CS_STALL);
> }
> +
> + /* Do not restore push constant packets during context restore. */
> + if (devinfo->gen == 10)
> + gen10_emit_isp_disable(brw);
> }
>
> /* Emit MI_BATCH_BUFFER_END to finish our batch. Note that execbuf2
> We might consider doing this on gen >= 7, as I think it should be safe and possibly save some unnecessary garbage reads during context restore. But, we can always do that later. I'm happy to take your patches as-is for now. Series is: Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
