On 09/02/16 02:29, Jordan Justen wrote: > 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 <chr...@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 >>> <chr...@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.
That was also what Illia suggested. > 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.jus...@intel.com> > Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com> Two reviews, so I think that it is safe to push without waiting for Chad Versace. I have just pushed the commit, adding the Signed-off as you all suggested. Thanks everybody for the patience and the reviews. BR _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev