[Mesa-dev] [PATCH] i965: Don't try to use the hardware blitter for multisampled miptrees.

[Kenneth Graunke]

On 02/21/2014 08:20 PM, Paul Berry wrote:
> On 21 February 2014 19:15, Kenneth Graunke <kenneth at whitecape.org
> <mailto:kenneth at whitecape.org>> wrote:
> 
>     The blitter is completely ignorant of MSAA buffer layouts, so any
>     attempt to use BLT paths with MSAA buffers is likely to break
>     spectacularly.
> 
>     In most cases, BLORP handles MSAA blits, so we never hit this bug.
>     Until recently, it also wasn't worth fixing, since Meta couldn't handle
>     MSAA either, so there was nothing to fall back to.  But now there is.
> 
>     +143 piglit tests on Broadwell (which doesn't have BLORP support).
>     Surprisingly, three also start failing.  No changes on Ivybridge.
> 
> 
> That actually doesn't surprise me too much.  Since color buffers use an
> array-like sample layout (sample N in the Nth layer), trying to use the
> blitter on a multisampled buffer just gets you sample 0 from each pixel.
>  Which for some tests is good enough to pass (even though the blit is
> actually doing the wrong thing).  So it's possible that a few of these
> tests that were passing by luck with the blitter don't yet have proper
> Meta support :)

That makes a lot of sense.  For UMS buffers, it got sample 0, which was
enough to fool a few not-very-picky tests.  But it still wrongly ignored
samples >= 1.  And it would probably break horribly for IMS buffers.

I can definitely believe the meta code needs more baking. :)

>     Signed-off-by: Kenneth Graunke <kenneth at whitecape.org
>     <mailto:kenneth at whitecape.org>>
>     ---
>      src/mesa/drivers/dri/i965/intel_blit.c | 4 ++++
>      1 file changed, 4 insertions(+)
> 
>     diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
>     b/src/mesa/drivers/dri/i965/intel_blit.c
>     index df85dc9..cedb230 100644
>     --- a/src/mesa/drivers/dri/i965/intel_blit.c
>     +++ b/src/mesa/drivers/dri/i965/intel_blit.c
>     @@ -165,6 +165,10 @@ intel_miptree_blit(struct brw_context *brw,
>         mesa_format src_format =
>     _mesa_get_srgb_format_linear(src_mt->format);
>         mesa_format dst_format =
>     _mesa_get_srgb_format_linear(dst_mt->format);
> 
>     +   /* The blitter doesn't understand multisampling at all. */
>     +   if (src_mt->num_samples > 0 || dst_mt->num_samples > 0)
>     +      return false;
>     +
> 
> 
> You might consider moving this check up to the top of the function,
> since it doesn't depend on src_format or dst_format.  But it's not a big
> deal.  Either way, the patch is:
> 
> Reviewed-by: Paul Berry <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>>

Done.  Thanks a ton for the review, Paul!

--Ken

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140223/6322b0d0/attachment-0001.pgp>