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

[Jordan Justen]

On 2016-02-06 10:25:59, Ben Widawsky wrote:
> 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.
> 

On top? I usually add to the bottom.

Alejandro,

I ran the patch through jenkins, and it seemed to fix Sandy Bridge,
Ivy Bridge and Haswell for
piglit.spec.arb_texture_multisample.arb_texture_multisample-sample-depth

Tested-by: Jordan Justen <jordan.l.justen at intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

> (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