[Mesa-dev] [PATCH 04/12] i965/cnl: Implement new pipe control workaround

[Kenneth Graunke]

On Friday, April 14, 2017 8:21:03 PM PDT Ilia Mirkin wrote:
> On Fri, Apr 14, 2017 at 8:35 PM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
> > From: Ben Widawsky <benjamin.widawsky at intel.com>
> >
> > GEN10 requires flushing all previous pipe controls before issuing a render
> > target cache flush. The docs seem to fairly explicitly say this is gen10 only.
> >
> > v2: Rebased on
> > commit 04f74d66293222d5e1905cfb930bfa083e30463c
> > Author: Francisco Jerez <currojerez at riseup.net>
> > Date:   Thu Jun 30 19:39:24 2016 -0700
> >
> >     i965: Emit SNB write cache flush W/A from brw_emit_pipe_control_flush.
> >
> > Cc: Francisco Jerez <currojerez at riseup.net>
> > Signed-off-by: Ben Widawsky <benjamin.widawsky at intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/brw_pipe_control.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> > index b8f7406..b921fe7 100644
> > --- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> > +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> > @@ -128,6 +128,24 @@ brw_emit_pipe_control_flush(struct brw_context *brw, uint32_t flags)
> >           brw_emit_pipe_control_flush(brw, 0);
> >        }
> >
> > +      if (brw->gen == 10) {
> 
> Should this only be if flags & PIPE_CONTROL_RENDER_TARGET_FLUSH ?

Yes it should.

> > +        /* Hardware workaround: CNL
> > +         *
> > +         * "Before sending a PIPE_CONTROL command with bit 12 set, SW
> > +         * must issue another PIPE_CONTROL with Render Target Cache
> > +         * Flush Enable (bit 12) = 0 and Pipe Control Flush Enable (bit
> > +         * 7) = 1."
> > +         */
> > +         BEGIN_BATCH(6);
> > +         OUT_BATCH(_3DSTATE_PIPE_CONTROL | (6 - 2));
> > +         OUT_BATCH(PIPE_CONTROL_FLUSH_ENABLE);
> 
> Based on the comment above, shouldn't this also be |
> PIPE_CONTROL_RENDER_TARGET_FLUSH?

Nope - it actually says "Render Target Cache Flush Enable (bit 12) = 0"
...the "= 0" being the key part.  Not great wording :/  I'm guessing
they're trying to make it clear that you need a separate PIPE_CONTROL
for the workaround - you can't just add (bit 7) to the existing one.

> Also, this tends to be done as a brw_emit_pipe_control_flush(brw,
> fooflags) call above for gen9, makes sense to do the same thing here,
> no?

Yes it does.  That would cause infinite recursion, unless you fix the
bug you mentioned above...at which point it should be fine. :)

> > +         OUT_BATCH(0);
> > +         OUT_BATCH(0);
> > +         OUT_BATCH(0);
> > +         OUT_BATCH(0);
> > +         ADVANCE_BATCH();
> > +      }
> > +
> >        BEGIN_BATCH(6);
> >        OUT_BATCH(_3DSTATE_PIPE_CONTROL | (6 - 2));
> >        OUT_BATCH(flags);
> > --
> > 2.9.3
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170416/c5b8caa2/attachment.sig>