<p dir="ltr"><br>
On Jan 14, 2016 9:30 PM, "Jordan Justen" <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>> wrote:<br>
><br>
> On 2016-01-14 20:43:17, Jason Ekstrand wrote:<br>
> > ---<br>
> > src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 16 ++++++++++++++++<br>
> > 1 file changed, 16 insertions(+)<br>
> ><br>
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp<br>
> > index eebb485..70ca7cd 100644<br>
> > --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp<br>
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp<br>
> > @@ -914,6 +914,22 @@ fs_generator::generate_tex(fs_inst *inst, struct brw_reg dst, struct brw_reg src<br>
> > /* Set the offset bits in DWord 2. */<br>
> > brw_MOV(p, get_element_ud(header_reg, 2),<br>
> > brw_imm_ud(inst->offset));<br>
> > + } else if (stage != MESA_SHADER_VERTEX &&<br>
> > + stage != MESA_SHADER_FRAGMENT) {<br>
> > + /* In the vertex and fragment stages, the hardware is nice to us<br>
> > + * and leaves g0.2 zerod out for us so we can use it for headers.<br>
> > + * However, in compute, geometry, and tessellation stages, the<br>
> > + * hardware is not so nice. In particular, for compute shaders on<br>
> > + * BDW, the hardware places some debug bits in 23:15. As it<br>
> > + * happens, bit 15 is the alpha channel mask. This means that if<br>
> > + * you use a texturing instruction with a header in a compute<br>
> > + * shader, you may randomly get the alpha channel randomly<br>
> > + * disabled. Since channel masks affect the return length of the<br>
> > + * sampler message, this can lead the GPU to expect a different<br>
> > + * mlen to the one you specified in the shader (probably 4 or 8)<br>
> > + * and this, in turn, hangs your GPU.<br>
> > + */<br>
><br>
> This explanation is great, but my personal opinion is that it is<br>
> better for the commit message than the comment. For a comment, what<br>
> about something more terse, like:</p>
<p dir="ltr">That's fair. I sometimes get a bit long-winded and ranty in my comments.</p>
<p dir="ltr">> /* The vertex and fragment stages have g0.2 set to 0, so<br>
> * header0.2 is 0 when g0 is copied. Other stages may not, so we<br>
> * must set it to 0 to avoid setting undesirable bits in the<br>
> * message.<br>
> */</p>
<p dir="ltr">I'll just copy+paste that in.</p>
<p dir="ltr">> For the series: Reviewed-by: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>></p>
<p dir="ltr">Thanks!</p>
<p dir="ltr">> > + brw_MOV(p, get_element_ud(header_reg, 2), brw_imm_ud(0));<br>
> > }<br>
> ><br>
> > brw_adjust_sampler_state_pointer(p, header_reg, sampler_index);<br>
> > --<br>
> > 2.5.0.400.gff86faf<br>
> ><br>
> > _______________________________________________<br>
> > mesa-dev mailing list<br>
> > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</p>