[Mesa-dev] [PATCH] intel/compiler/gen9: Pixel shader header only workaround

[Kenneth Graunke]

On Friday, October 27, 2017 4:26:32 AM PDT Pohjolainen, Topi wrote:
> On Fri, Oct 27, 2017 at 03:02:59AM -0700, Kenneth Graunke wrote:
> > On Wednesday, October 25, 2017 10:37:37 AM PDT Topi Pohjolainen wrote:
> > > Fixes intermittent GPU hangs on Broxton with an Intel internal
> > > test case.
> > > 
> > > There are plenty of similar fragment shaders in piglit that do
> > > not use any varyings and any uniforms. According to the
> > > documentation special timing is needed between pipeline stages.
> > > Apparently we just don't hit that with piglit. Even with the
> > > failing test case one doesn't always get the hang.
> > > 
> > > Moreover, according to the error states the hang happens
> > > significantly later than the execution of the problematic shader.
> > > There are multiple render cycles (primitive submissions) in between.
> > > I've also seen error states where the ACTHD points outside the
> > > batch. Almost as if the hardware writes somewhere that gets used
> > > later on. That would also explain why piglit doesn't suffer from
> > > this - most tests kick off one render cycle and any corruption
> > > is left unseen.
> > > 
> > > v2 (Ken): Instead of enabling push constants, enable one of the
> > >           inputs (PSIZ).
> > > v3 (Ken, Jason): Use LAYER instead making vulkan emit_3dstate_sbe()
> > >                  happy.
> > > 
> > > CC: Kenneth Graunke <kenneth at whitecape.org>
> > > CC: Jason Ekstrand <jason at jlekstrand.net>
> > > CC: Eero Tamminen <eero.t.tamminen at intel.com>
> > > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > > ---
> > >  src/intel/compiler/brw_fs.cpp | 29 +++++++++++++++++++++++++++++
> > >  1 file changed, 29 insertions(+)
> > 
> > This looks great, thanks a ton for fixing this, Topi!
> > 
> > Cc: "17.3 17.2" <mesa-stable at lists.freedesktop.org>
> 
> We just need to make sure 17.2/3 contain also Iago's:
> 
> 
> commit 566a0c43f0b9fbf5106161471dd5061c7275f761
> Author: Iago Toral Quiroga <itoral at igalia.com>
> Date:   Thu Jan 5 13:17:53 2017 +0100
> 
>     anv: don't skip the VUE header if we are reading gl_Layer in a fragment shader
>     
>     This is the same we do in the GL driver: the hardware provides gl_Layer
>     in the VUE header, so when the fragment shader reads it we can't skip it.
> 
> 
> otherwise it'll assert.

Fortunately, both the 17.2 and 17.3 branches already contain that
commit.  I thought you might also need this one:

commit 70cd05d6ac533977f96aa832bbb2886172019f35
Author: Kenneth Graunke <kenneth at whitecape.org>
Date:   Wed Oct 25 09:37:09 2017 -0700

    anv: Fix assert about source attrs.
    
    Asserting slot >= 2 made sense when the URB read offset was always 1
    (pair of slots).  Commit 566a0c43f0b9fbf5106161471dd5061c7275f761 made
    it possible to read from the VUE header in slot 0, by adjusting the
    offset to be 0.  So, this assert is now bogus.  Use the one from GL.
    
    Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

But it looks like you technically don't, since with VARYING_SLOT_LAYER
it'll "continue" and skip over the slot >= 2 assert.

So I think we're fine.
-------------- 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/20171027/ffcc0e5c/attachment.sig>