[Mesa-dev] [PATCH 09/12] i965/fs/skl+: Fix calculating gl_SampleID for 16x MSAA

[Anuj Phogat]

On Tue, Sep 22, 2015 at 4:18 PM, Ben Widawsky <ben at bwidawsk.net> wrote:

> On Thu, Sep 17, 2015 at 05:00:11PM +0100, Neil Roberts wrote:
> > In order to accomodate 16x MSAA, the starting sample pair index is now
> > 3 bits rather than 2 on SKL+.
>
> Hooray for bad docs. "PS Thread Payload for Normal Dispatch": SSPI is 2
> bits
> (R0.0.7:6) with 1 reserved for expansion.
>
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs.cpp | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > index 82f49b4..bd9bcdd 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > @@ -1300,9 +1300,16 @@ fs_visitor::emit_sampleid_setup()
> >         * are sample 1 of subspan 0; the third group is sample 0 of
> >         * subspan 1, and finally sample 1 of subspan 1.
> >         */
> > +
> > +      /* SKL+ has an extra bit for the Starting Sample Pair Index to
> > +       * accomodate 16x MSAA.
> > +       */
> > +      unsigned sspi_bits = devinfo->gen >= 9 ? 3 : 2;
> > +      unsigned sspi_mask = ((1 << sspi_bits) - 1) << 6;
> > +
>
> I think using the bits makes this unnecessarily hard to read.
> unsigned sspi_mask = devinfo->gen >= 9 ? 0x1c0 : 0xc0;
>
> >        abld.exec_all()
> >            .AND(t1, fs_reg(retype(brw_vec1_grf(0, 0),
> BRW_REGISTER_TYPE_UD)),
> > -               fs_reg(0xc0));
> > +               fs_reg(sspi_mask));
> >        abld.exec_all().SHR(t1, t1, fs_reg(5));
> >
> >        /* This works for both SIMD8 and SIMD16 */
>
> I'm really sketchy on the details of how this actually works. Mostly, I
> get what
> this code is doing, and I agree that the only difference for 16x should be
> the
> extra bit (because SSPI needs to go up to 7 for SIMD8).
>
As per docs we're supposed to get the per slot SampleID written to 15:0
bits in
R1.0. I used SSPI to compute the SampleID because I never got anything
useful
in these bits on IVB. Things might have changed on later platforms. So, I
think
it's worth trying to do what docs say. I'll send out a patch to add above
reasoning
as a comment in this code. For now I'm happy with this patch:

Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>


>
> So take my:
> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>
>
> with a grain of salt.
> _______________________________________________
> 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/20150924/16d35fe9/attachment-0001.html>