[Mesa-dev] [PATCH 1/2] i965/fs: Fix gl_SampleID for 2x MSAA and SIMD16 mode.

[Matt Turner]

On Fri, Jul 18, 2014 at 1:19 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> We might be able to do this without an extra program key field, but this
> is non-invasive and fixes the bug, for now.
>
> This fixes the following Piglit tests on Broadwell:
> - ARB_sample_shading/builtin-gl-sample-id 2
> - ARB_sample_shading/builtin-gl-sample-position 2
> - EXT_framebuffer_multisample/multisample-blit 2 color
> - EXT_framebuffer_multisample/multisample-blit 2 color linear
> - EXT_framebuffer_multisample/multisample-blit 2 depth
> - EXT_framebuffer_multisample/no-color 2 depth combined
> - EXT_framebuffer_multisample/no-color 2 depth separate
> - EXT_framebuffer_multisample/no-color 2 depth single
> - EXT_framebuffer_multisample/no-color 2 depth-computed combined
> - EXT_framebuffer_multisample/no-color 2 depth-computed separate
> - EXT_framebuffer_multisample/no-color 2 depth-computed single
> - EXT_framebuffer_multisample/unaligned-blit 2 color msaa
> - EXT_framebuffer_multisample/unaligned-blit 2 depth msaa
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80991
> Cc: "10.2" <mesa-stable at lists.freedesktop.org>
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 7 ++++++-
>  src/mesa/drivers/dri/i965/brw_wm.c   | 4 ++++
>  src/mesa/drivers/dri/i965/brw_wm.h   | 1 +
>  3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 1a5d987..a749e89 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -1304,6 +1304,11 @@ fs_visitor::emit_sampleid_setup(ir_variable *ir)
>         * populating a temporary variable with the sequence (0, 1, 2, 3),
>         * and then reading from it using vstride=1, width=4, hstride=0.
>         * These computations hold good for 4x multisampling as well.
> +       *
> +       * For 2x MSAA and SIMD16, we want to use the sequence (0, 1, 0, 1):
> +       * the first four slots are sample 0 of subspan 0; the next four
> +       * are sample 1 of subspan 0; the third group is sample 0 of
> +       * subspan 1, and finally sample 1 of subspan 1.
>         */
>        fs_inst *inst;
>        inst = emit(BRW_OPCODE_AND, t1,
> @@ -1313,7 +1318,7 @@ fs_visitor::emit_sampleid_setup(ir_variable *ir)
>        inst = emit(BRW_OPCODE_SHR, t1, t1, fs_reg(5));
>        inst->force_writemask_all = true;
>        /* This works for both SIMD8 and SIMD16 */
> -      inst = emit(MOV(t2, brw_imm_v(0x3210)));
> +      inst = emit(MOV(t2, brw_imm_v(key->persample_2x ? 0x1010 : 0x3210)));

Is this still safe for SIMD8?

If so,

Reviewed-by: Matt Turner <mattst88 at gmail.com>