On Monday, February 25, 2019 11:01:33 AM PST Pohjolainen, Topi wrote: > On Mon, Feb 25, 2019 at 10:32:27AM -0800, Kenneth Graunke wrote: > > On Monday, February 25, 2019 6:33:11 AM PST Pohjolainen, Topi wrote: > > > On Thu, Nov 01, 2018 at 08:04:21PM -0700, Kenneth Graunke wrote: > > > > - if (GEN_GEN >= 9) { > > > > - /* THE PIPE_CONTROL "VF Cache Invalidation Enable" docs > > > > continue: > > > > - * > > > > - * "Project: BDW+ > > > > - * > > > > - * When VF Cache Invalidate is set “Post Sync > > > > Operation” must > > > > - * be enabled to “Write Immediate Data” or “Write PS > > > > Depth > > > > - * Count” or “Write Timestamp”." > > > > - * > > > > - * If there's a BO, we're already doing some kind of write. > > > > - * If not, add a write to the workaround BO. > > > > - * > > > > - * XXX: This causes GPU hangs on Broadwell, so restrict it > > > > to > > > > - * Gen9+ for now...see this bug for more information: > > > > - * https://bugs.freedesktop.org/show_bug.cgi?id=103787 > > > > > > In "Flush Types" workarounds later on you apply this for gen8 as well. > > > > Yes, that's intentional - we're supposed to according to the docs. > > I re-tested the Piglit test from bug 103787 on my Broadwell, and it > > works fine - no GPU hangs. I think we were just doing it wrong before. > > > > Trying to figure out an ordering for the workarounds is awful... :( > > What would you think about another patch just before this to enable that for > gen8? Just in case it causes problems it would bisect to much smaller patch.
It isn't simply enabling it though...in the old code, we had: if (devinfo->gen == 8) gen8_add_cs_stall_workaround_bits(&flags); if (flags & PIPE_CONTROL_VF_CACHE_INVALIDATE) { if (devinfo->gen >= 9) { ... if (!bo) { flags |= PIPE_CONTROL_WRITE_IMMEDIATE; bo = brw->workaround_bo; } } } Which adds a WRITE_IMMEDIATE to the current PIPE_CONTROL, but does so after the call to gen8_add_cs_stall_workaround_bits - and that function would have added a CS_STALL had it seen the WRITE_IMMEDIATE. I suspect this bad ordering is why we saw hangs on Broadwell - we missed a stall. The new code performs these in the opposite order, correctly adding the necessary CS_STALL. I could probably write a patch to swap those and enable it on Gen8+. --Ken
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