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

[Ben Widawsky]

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).

So take my:
Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

with a grain of salt.