[Mesa-dev] [PATCH 2/2] i965/fs: Always set hannel 2 of texture headers in some stages

[Jason Ekstrand]

On Jan 14, 2016 9:30 PM, "Jordan Justen" <jordan.l.justen at intel.com> wrote:
>
> On 2016-01-14 20:43:17, Jason Ekstrand wrote:
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > index eebb485..70ca7cd 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > @@ -914,6 +914,22 @@ fs_generator::generate_tex(fs_inst *inst, struct
brw_reg dst, struct brw_reg src
> >              /* Set the offset bits in DWord 2. */
> >              brw_MOV(p, get_element_ud(header_reg, 2),
> >                         brw_imm_ud(inst->offset));
> > +         } else if (stage != MESA_SHADER_VERTEX &&
> > +                    stage != MESA_SHADER_FRAGMENT) {
> > +            /* In the vertex and fragment stages, the hardware is nice
to us
> > +             * and leaves g0.2 zerod out for us so we can use it for
headers.
> > +             * However, in compute, geometry, and tessellation stages,
the
> > +             * hardware is not so nice.  In particular, for compute
shaders on
> > +             * BDW, the hardware places some debug bits in 23:15.  As
it
> > +             * happens, bit 15 is the alpha channel mask.  This means
that if
> > +             * you use a texturing instruction with a header in a
compute
> > +             * shader, you may randomly get the alpha channel randomly
> > +             * disabled.  Since channel masks affect the return length
of the
> > +             * sampler message, this can lead the GPU to expect a
different
> > +             * mlen to the one you specified in the shader (probably 4
or 8)
> > +             * and this, in turn, hangs your GPU.
> > +             */
>
> This explanation is great, but my personal opinion is that it is
> better for the commit message than the comment. For a comment, what
> about something more terse, like:

That's fair.  I sometimes get a bit long-winded and ranty in my comments.

>             /* The vertex and fragment stages have g0.2 set to 0, so
>              * header0.2 is 0 when g0 is copied. Other stages may not, so
we
>              * must set it to 0 to avoid setting undesirable bits in the
>              * message.
>              */

I'll just copy+paste that in.

> For the series: Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

Thanks!

> > +            brw_MOV(p, get_element_ud(header_reg, 2), brw_imm_ud(0));
> >           }
> >
> >           brw_adjust_sampler_state_pointer(p, header_reg,
sampler_index);
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20160114/5f4568ad/attachment-0001.html>