Mesa (master): i965/blorp: Increase Y alignment for multisampled stencil blits.

Paul Berry stereotype441 at kemper.freedesktop.org
Mon Sep 24 16:28:24 UTC 2012


Module: Mesa
Branch: master
Commit: a33ce665a5827c598b85bb04d94b33e6a5e41c28
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=a33ce665a5827c598b85bb04d94b33e6a5e41c28

Author: Paul Berry <stereotype441 at gmail.com>
Date:   Wed Sep 12 11:13:49 2012 -0700

i965/blorp: Increase Y alignment for multisampled stencil blits.

This patch is a band-aid fix for a bug in commit 5fd67fa (i965/blorp:
Reduce alignment restrictions for stencil blits), which causes
multisampled stencil blits to work incorrectly on Sandy Bridge.

When blitting to or from a normal stencil buffer, we have to use a
coordinate transformation that swizzles coordinates to account for the
fact that stencil buffers use W tiling, but the most similar tiling
format available for textures and render targets is Y tiling.  The
differences between W and Y tiling cause pixels to be scrambled within
a block of size 8x4 (width x height) as measured relative to a W tile,
or 16x2 as measured relative to a Y tile.  So in order to make sure
that pixels at the edges of the blit aren't lost, we need to align the
rendering rectangle (and the buffer sizes) to multiples of the 8x4
block size.  This alignment happens in the brw_blorp_blit_params
constructor, whereas the determination of how to swizzle the
coordinates happens during code generation, in the
brw_blorp_blit_program class.

When blitting to or from a multisampled stencil buffer, the coordinate
swizzling is more complex, because it has to account for the
interleaving pattern of samples, which uses 4x4 blocks for 4x MSAA and
8x4 blocks for 8x MSAA.  The end result is that if multisampling is in
use, the 16x2 block size (relative so a Y tile) needs to be expanded
to 16x4, and the corresponding size relative to a W tile expands to
8x8.

The problem doesn't affect Ivy Bridge severely enough to crop up in
Piglit tests because on Ivy Bridge we have to disable multisampling
when blitting *to* a multisampled stencil buffer (the blorp compiler
generates code to compensate for the fact that multisampling is
disabled).  However I suspect a bug is still present because we don't
disable multisampling when blitting *from* a multisampled stencil
buffer.

This patch fixes the problem by doubling the vertical alignment
requirement when blitting to or from a multisampled stencil buffer,
and multisampling has not been disabled.

In the long run I would like to rework the brw_blorp_blit_params
constructor--it's difficult to follow and has had several subtle bugs
like this one.  However this band-aid fix should be suitable for
cherry-picking to release branches.

Fixes Piglit tests "unaligned-blit {2,4} stencil {msaa,upsample}" on
Sandy Bridge.

NOTE: This is a candidate for stable release branches.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

---

 src/mesa/drivers/dri/i965/brw_blorp_blit.cpp |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
index 6e156d0..034c701 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
@@ -1800,6 +1800,11 @@ brw_blorp_blit_params::brw_blorp_blit_params(struct brw_context *brw,
        * account for the differences in aspect ratio between the Y and W
        * sub-tiles.  We need to modify the layer width and height similarly.
        *
+       * A correction needs to be applied when MSAA is in use: since
+       * INTEL_MSAA_LAYOUT_IMS uses an interleaving pattern whose height is 4,
+       * we need to align the Y coordinates to multiples of 8, so that when
+       * they are divided by two they are still multiples of 4.
+       *
        * Note: Since the x/y offset of the surface will be applied using the
        * SURFACE_STATE command packet, it will be invisible to the swizzling
        * code in the shader; therefore it needs to be in a multiple of the
@@ -1821,7 +1826,7 @@ brw_blorp_blit_params::brw_blorp_blit_params(struct brw_context *brw,
        * TODO: what if this makes the coordinates (or the texture size) too
        * large?
        */
-      const unsigned x_align = 8, y_align = 4;
+      const unsigned x_align = 8, y_align = dst.num_samples != 0 ? 8 : 4;
       x0 = ROUND_DOWN_TO(x0, x_align) * 2;
       y0 = ROUND_DOWN_TO(y0, y_align) / 2;
       x1 = ALIGN(x1, x_align) * 2;
@@ -1843,7 +1848,7 @@ brw_blorp_blit_params::brw_blorp_blit_params(struct brw_context *brw,
        *
        * TODO: what if this makes the texture size too large?
        */
-      const unsigned x_align = 8, y_align = 4;
+      const unsigned x_align = 8, y_align = src.num_samples != 0 ? 8 : 4;
       src.width = ALIGN(src.width, x_align) * 2;
       src.height = ALIGN(src.height, y_align) / 2;
       src.x_offset *= 2;




More information about the mesa-commit mailing list