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

[Eric Anholt]

On Tue, 19 Jul 2011 22:11:27 -0700, Kenneth Graunke <kenneth at whitecape.org> wrote:
> 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?

Agreed!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110720/cbe055db/attachment.pgp>