Ran multiple test cases multiple times that were introducing GPU hangs. Applying this patch fixed the GPU hang issues on SKL.
Tested-by: Valtteri Rantala <valtteri.rant...@intel.com> > -----Original Message----- > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf > Of Anuj Phogat > Sent: Friday, June 19, 2015 4:27 AM > To: Widawsky, Benjamin > Cc: mesa-dev; Deak, Imre; Phogat, Anuj; Ben Widawsky > Subject: Re: [Mesa-dev] [PATCH 1/2] i965/gen9: Implement Push Constant > Buffer workaround > > On Wed, Jun 3, 2015 at 9:35 PM, Ben Widawsky > <benjamin.widaw...@intel.com> wrote: > > This implements a workaround (exact excerpt as a comment in the code). > > The docs specify [clearly, after you struggle for a while] that the > > offset isn't relative to state base. This actually makes sense. > > > > Buffer #0 is meant to be used for normal uniforms. > > Buffer #1 is typically used for gather constants when using RS. > > Buffer #1-#3 could be used to push a bunch of UBO data which would just be > > somewhere in memory, and not relative to the dynamic state. > > > > NOTE: I've moved away from the ternary operator for the new gen9 > conditions. > > Admittedly it's probably not great to do this, but I really want to > > fix this all up in the subsequent patch and doing it here makes that > > diff a lot nicer. I want to split out the gen8/9 code to make the > > function a bit more readable, but to keep this easily cherry-pickable > > I am doing this fix first. If we decide not to merge the cleanup patch then > > I can > revisit this. > > > > Anuj ran this on his SKL and said there were no fixes on regressions. > > There is some hope it fixes BXT issues. > > > > Cc: Imre Deak <imre.d...@intel.com> > > Cc: Neil Roberts <n...@linux.intel.com> > > Cc: Anuj Phogat <anuj.pho...@intel.com> > > Signed-off-by: Ben Widawsky <b...@bwidawsk.net> > > --- > > src/mesa/drivers/dri/i965/gen7_vs_state.c | 48 > > ++++++++++++++++++++++++++----- > > 1 file changed, 41 insertions(+), 7 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/gen7_vs_state.c > > b/src/mesa/drivers/dri/i965/gen7_vs_state.c > > index 278b3ec..4b17d06 100644 > > --- a/src/mesa/drivers/dri/i965/gen7_vs_state.c > > +++ b/src/mesa/drivers/dri/i965/gen7_vs_state.c > > @@ -43,18 +43,52 @@ gen7_upload_constant_state(struct brw_context > *brw, > > int dwords = brw->gen >= 8 ? 11 : 7; > > BEGIN_BATCH(dwords); > > OUT_BATCH(opcode << 16 | (dwords - 2)); > > - OUT_BATCH(active ? stage_state->push_const_size : 0); > > - OUT_BATCH(0); > > + > > + /* Workaround for SKL+ (we use option #2 until we have a need for more > > + * constant buffers). This comes from the documentation for > 3DSTATE_CONSTANT_* > > + * > > + * The driver must ensure The following case does not occur without a > > flush > > + * to the 3D engine: 3DSTATE_CONSTANT_* with buffer 3 read length equal > to > > + * zero committed followed by a 3DSTATE_CONSTANT_* with buffer 0 read > length > > + * not equal to zero committed. Possible ways to avoid this condition > > + * include: > > + * 1. always force buffer 3 to have a non zero read length > > + * 2. always force buffer 0 to a zero read length > > + */ > > + if (brw->gen >= 9 && active) { > > + OUT_BATCH(0); > > + OUT_BATCH(stage_state->push_const_size); > > + } else { > > + OUT_BATCH(active ? stage_state->push_const_size : 0); > > + OUT_BATCH(0); > > + } > > /* Pointer to the constant buffer. Covered by the set of state flags > > * from gen6_prepare_wm_contants > > */ > > - OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0); > > - OUT_BATCH(0); > > - OUT_BATCH(0); > > - OUT_BATCH(0); > > - if (brw->gen >= 8) { > > + if (brw->gen >= 9 && active) { > > + OUT_BATCH(0); > > + OUT_BATCH(0); > > + OUT_BATCH(0); > > + OUT_BATCH(0); > > + /* XXX: When using buffers other than 0, you need to specify the > > + * graphics virtual address regardless of INSPM/debug bits > INSTPM > > + */ > > + OUT_RELOC64(brw->batch.bo, I915_GEM_DOMAIN_RENDER, 0, > > + stage_state->push_const_offset); > > OUT_BATCH(0); > > OUT_BATCH(0); > > + } else if (brw->gen>= 8) { > > + OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0); > > + OUT_BATCH(0); > > + OUT_BATCH(0); > > + OUT_BATCH(0); > > + OUT_BATCH(0); > > + OUT_BATCH(0); > > + OUT_BATCH(0); > > + OUT_BATCH(0); > > + } else { > > + OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0); > > + OUT_BATCH(0); > > OUT_BATCH(0); > > OUT_BATCH(0); > > } > > -- > > 2.4.2 > > > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > Verified with the spec. LGTM. > > Reviewed-by: Anuj Phogat <anuj.pho...@gmail.com> > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
