[Mesa-dev] [PATCH 2/3] i965: Enable the PIPE_CONTROL workaround workaround out of paranoia.

[Kenneth Graunke]

On 07/19/2011 03:44 PM, Eric Anholt wrote:
> There's scary stuff going on in PIPE_CONTROL internals, and if the
> BSpec says to do this to make PIPE_CONTROL work, I'll go ahead and do
> it because we'll probably never be able to debug it after the fact.
> ---
>  src/mesa/drivers/dri/intel/intel_batchbuffer.c |   32 +++++++++++++++++++++--
>  1 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> index 9c97ef2..6a5f934 100644
> --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> @@ -308,12 +308,30 @@ emit:
>   * [Dev-SNB{W/A}]: Before a PIPE_CONTROL with Write Cache Flush Enable
>   * =1, a PIPE_CONTROL with any non-zero post-sync-op is required.
>   *
> - * XXX: There is also a workaround that would appear to apply to this
> - * workaround, but it doesn't appear to be necessary so far:
> + * And the workaround for these two requires this workaround first:
>   *
> - * Dev-SNB{W/A}]: Pipe-control with CS-stall bit set must be sent
> + * [Dev-SNB{W/A}]: Pipe-control with CS-stall bit set must be sent
>   * BEFORE the pipe-control with a post-sync op and no write-cache
>   * flushes.
> + *
> + * And this last workaround is tricky because of the requirements on
> + * that bit.  From section 1.4.7.2.3 "Stall" of the Sandy Bridge PRM
> + * volume 2 part 1:

You mean 1.7.4.2.3.  For that matter, I think the earlier reference
("implementing two workarounds on gen6. From section 1.4.7.1") should be
1.7.4.1 as well.

> + *
> + *     "1 of the following must also be set:
> + *      - Render Target Cache Flush Enable ([12] of DW1)
> + *      - Depth Cache Flush Enable ([0] of DW1)
> + *      - Stall at Pixel Scoreboard ([1] of DW1)
> + *      - Depth Stall ([13] of DW1)
> + *      - Post-Sync Operation ([13] of DW1)
> + *      - Notify Enable ([8] of DW1)"
> + *
> + * The cache flushes require the workaround flush that triggered this
> + * one, so we can't use it.  Post-sync nonzero is what triggered this
> + * workaround, so we can't use that one either.  Notify enable is
> + * IRQs, which aren't really our business.  That leaves depth stall or
> + * stall at scoreboard.  It shouldn't matter which we choose because
> + * we're supposed to be stalling the entire pipeline anyway.

It seems like depth stall should be eliminated, too.  Workaround 1:
"Before any depth stall flush [...] send a PIPE_CONTROL with no bits set
except Post-Sync Operation != 0".  So we'd have to do that.  But then we
hit Workaround 3 again---this very text says we need a CS stall...with
depth or scoreboard stall.  If a depth stall...refer to rule 1.
Repeat...infinitely.

Perhaps I'm misreading...or it doesn't apply since we don't care about
the effects of the depth stall (we're just required to set the bit).

Anyway, it seems like Stall at Pixel Scoreboard might be the safer bet, no?

>   */
>  void
>  intel_emit_post_sync_nonzero_flush(struct intel_context *intel)
> @@ -323,6 +341,14 @@ intel_emit_post_sync_nonzero_flush(struct intel_context *intel)
>  
>     BEGIN_BATCH(4);
>     OUT_BATCH(_3DSTATE_PIPE_CONTROL);
> +   OUT_BATCH(PIPE_CONTROL_CS_STALL |
> +	     PIPE_CONTROL_DEPTH_STALL);
> +   OUT_BATCH(0); /* address */
> +   OUT_BATCH(0); /* write data */
> +   ADVANCE_BATCH();
> +
> +   BEGIN_BATCH(4);
> +   OUT_BATCH(_3DSTATE_PIPE_CONTROL);
>     OUT_BATCH(PIPE_CONTROL_WRITE_IMMEDIATE);
>     OUT_RELOC(intel->batch.workaround_bo,
>  	     I915_GEM_DOMAIN_INSTRUCTION, I915_GEM_DOMAIN_INSTRUCTION, 0);