From a33ce665a5827c598b85bb04d94b33e6a5e41c28 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Wed, 12 Sep 2012 11:13:49 -0700 Subject: [PATCH] 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 --- src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 9 +++++++-- 1 file 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 6e156d0f00a..034c7019573 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; -- 2.30.2