[Mesa-dev] [PATCH v3] i965/blorp: Fix hiz ops on MSAA surfaces

[Ben Widawsky]

On Sat, Feb 06, 2016 at 12:01:50PM +0100, Alejandro Piñeiro wrote:
> From: Chris Forbes <chrisf at ijw.co.nz>
> 
> Two things were broken here:
> - The depth/stencil surface dimensions were broken for MSAA.
> - Sample count was programmed incorrectly.
> 
> Result was the depth resolve didn't work correctly on MSAA surfaces, and
> so sampling the surface later produced garbage.
> 
> Fixes the new piglit test arb_texture_multisample-sample-depth, and
> various artifacts in 'tesseract' with msaa=4 glineardepth=0.
> 
> Fixes freedesktop bug #76396.
> 
> Not observed any piglit regressions on Haswell.
> 
> v2: Just set brw_hiz_op_params::dst.num_samples rather than adding a
>     helper function (Ken).
> 
> v3: moved the alignment needed for hiz+msaa to brw_blorp.cpp, as
>     suggested by Chad Versace (Alejandro Piñeiro on behalf of Chris
>     Forbes)
> ---
> 
> While taking a look to bug #76396, I found that the git commit message
> on arb_texture_multisample/sample-depth.c mentions that MSAA+hiz not
> working on Haswell was an already know bug. After pinging on IRC Ben
> Widawsky pointed me this around one year old review thread.
> 
> It seems that the original patch got not updated after Chad Versace
> review. Hoping to not stomp on any toe, I made the update myself (so
> this v3 update). Even if this update is not accepted, hopefully this
> will resume the review process and the bug will be fixed soon.
> 
> I re-run piglit on Haswell, so "Not observed any piglit regressions on
> Haswell." still applies.
> 
> I also slightly changed the commit message to reflect that this patch
> fixes bug #76396. Also removed the "Signed-off-by: Chris Forbes
> <chrisf at ijw.co.nz>" because is not technically true anymore. Chris can
> re-add that again when he push the patch, if accepted.

Thanks for doing this Alejandro. Like Ilia said though, the common process is to
just add your SoB on top if there was one there to begin with.

(I actually didn't look at this one too closely, but I did review Chris' patch
in case nobody bothered to clean it up):
Reviewed-by: Ben Widawsky <benjamin.widawsky at intel.com>

[snip]


-- 
Ben Widawsky, Intel Open Source Technology Center