Jordan Justen <jordan.l.jus...@intel.com> writes: > On 2015-02-19 21:40:37, Ben Widawsky wrote: >> On Thu, Feb 19, 2015 at 03:42:05PM -0800, Jordan Justen wrote: >> > For fragment programs, we pull this mask from the payload header. The same >> > mask doesn't exist for compute shaders, so we set all bits to enabled. >> > >> > Note: this mask is ANDed with the execution mask, so some channels may not >> > end >> > up issuing the atomic operation. >> > >> > Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com> >> > Cc: Ben Widawsky <b...@bwidawsk.net> >> > Cc: Francisco Jerez <curroje...@riseup.net> >> >> Just add to the commit message that this is needed specifically because >> compute >> is invoked as SIMD16 (and perhaps reference the other commits?) and it's: >> Reviewed-by: Ben Widawsky <b...@bwidawsk.net> > > Good idea. I'll add those. > >> Sorry it advance... >> we may as well just go for 0xffffffff in case we ever support SIMD32. > > I had been setting all 32-bits previously. I mentioned to you that I > thought this was needed for SIMD32. I wanted to double check it before > sending the patch out. I think I found the field for IVB in the PRM: > > IVB Vol 4 Part 1 3.9.9.9 Message Header > Pixel/Sample Mask > > ...and it looks like it is only 16-bits. Maybe Francisco can confirm > that I got it right. > > I couldn't find this same information in the HSW PRMs. > > I'm not sure what that means for SIMD32. > Yeah, it's only 16 bits. For SIMD32 it means that, *sigh*, we'll have to split up the message in several SIMD16 chunks (my image load store branch has to do something similar to do typed surface operations in SIMD16 mode because there are only 8-wide variants of those messages). To initialize the header we would just copy the first or second 16-bit half of the sample mask, and set the quarter control bits appropriately for each half so the execution mask is taken into account correctly.
With Ben's and Matt's suggestions this patch is: Reviewed-by: Francisco Jerez <curroje...@riseup.net> > -Jordan > >> > --- >> > While it's fresh in our minds. :) >> > >> > This seems to work for gen7 & gen8 CS. For CS simd16, we need the >> > 0xffff change, but it seems to work fine for simd8 as well. >> > >> > I also tested gen8 (simd8vs), and there were no piglit regressions. >> > >> > src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 8 ++++---- >> > 1 file changed, 4 insertions(+), 4 deletions(-) >> > >> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp >> > b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp >> > index 24cc118..960a0aa 100644 >> > --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp >> > +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp >> > @@ -2998,9 +2998,9 @@ fs_visitor::emit_untyped_atomic(unsigned atomic_op, >> > unsigned surf_index, >> > * mask sent in the header to compute the actual set of channels >> > that execute >> > * the atomic operation. >> > */ >> > - assert(stage == MESA_SHADER_VERTEX); >> > + assert(stage == MESA_SHADER_VERTEX || stage == MESA_SHADER_COMPUTE); >> > emit(MOV(component(sources[0], 7), >> > - brw_imm_ud(0xff)))->force_writemask_all = true; >> > + brw_imm_ud(0xffff)))->force_writemask_all = true; >> > } >> > length++; >> > >> > @@ -3061,9 +3061,9 @@ fs_visitor::emit_untyped_surface_read(unsigned >> > surf_index, fs_reg dst, >> > * mask sent in the header to compute the actual set of channels >> > that execute >> > * the atomic operation. >> > */ >> > - assert(stage == MESA_SHADER_VERTEX); >> > + assert(stage == MESA_SHADER_VERTEX || stage == MESA_SHADER_COMPUTE); >> > emit(MOV(component(sources[0], 7), >> > - brw_imm_ud(0xff)))->force_writemask_all = true; >> > + brw_imm_ud(0xffff)))->force_writemask_all = true; >> > } >> > >> > /* Set the surface read offset. */ >> > -- >> > 2.1.4 >> >
pgpppOi6R6AAi.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
