[Mesa-dev] [PATCH 1/2] i965/gen9: Implement Push Constant Buffer workaround

[Rantala, Valtteri]

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.rantala at intel.com>

> -----Original Message-----
> From: mesa-dev [mailto:mesa-dev-bounces at 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.widawsky at 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.deak at intel.com>
> > Cc: Neil Roberts <neil at linux.intel.com>
> > Cc: Anuj Phogat <anuj.phogat at intel.com>
> > Signed-off-by: Ben Widawsky <ben at 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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> Verified with the spec. LGTM.
> 
> Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at 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.