[Mesa-dev] [PATCH] i965: Disable the unlit centroid workaround on Gen7.

[Kenneth Graunke]

On Monday, August 1, 2016 12:48:47 PM PDT Matt Turner wrote:
> Once upon a time (commit 8313f44409) Paul added code for the unlit
> centroid workaround (WaCopyUnlitCentroidBarys). His commit message
> claims it fixed the EXT_framebuffer_multisample/interpolation {2,4}
> {centroid-deriv,centroid-deriv-disabled} piglit tests but does not say
> on which platform, though he cites the IVB PRM.
> 
> "3DSTATE_WM [DevIVB, DevHSW]" says
> 
>    "[DevIVB]: Workaround: When Centroid Barycentric mode is required, HW
>     may produce incorrect interpolation results when a 2X2 pixels have
>     unlit pixels."
> 
> I later disabled it for Haswell (commit f6db414f3c) with no known ill
> effects.
> 
> The Sandybridge page does not have this text, but the workarounds
> database (see WaCopyUnlitCentroidBarys) says the issues applies *only*
> to Sandybridge, and in fact in commit 1a2de7dce8fc I note that disabling
> the workaround on Sandybridge causes the tests Paul originally mentioned
> to fail.
> 
> So this is, and always has been, a huge confusing mess.
> 
> Disabling the workaround indeed causes the tests Paul originally
> mentioned to fail on Sandybridge but not on Ivybridge/Baytrail.
> 
> On Ivybridge:
> 
>    total instructions in shared programs: 6914901 -> 6909599 (-0.08%)
>    instructions in affected programs: 106766 -> 101464 (-4.97%)
>    helped: 884
> 
>    total cycles in shared programs: 70874764 -> 70813774 (-0.09%)
>    cycles in affected programs: 794144 -> 733154 (-7.68%)
>    helped: 688
>    HURT: 186
> 
>    LOST:   1
>    GAINED: 6
> ---
> TODO: test with the test suites not enabled in the developer builds
> 
> TODO: see if disabling this workaround fixes the hang Ken mentions
>       in commit ebdc82d06532. I suspect it will.
> 
>  src/mesa/drivers/dri/i965/brw_device_info.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_device_info.c b/src/mesa/drivers/dri/i965/brw_device_info.c
> index 77bbe78..5620977 100644
> --- a/src/mesa/drivers/dri/i965/brw_device_info.c
> +++ b/src/mesa/drivers/dri/i965/brw_device_info.c
> @@ -116,7 +116,6 @@ static const struct brw_device_info brw_device_info_snb_gt2 = {
>  
>  static const struct brw_device_info brw_device_info_ivb_gt1 = {
>     GEN7_FEATURES, .is_ivybridge = true, .gt = 1,
> -   .needs_unlit_centroid_workaround = true,
>     .num_slices = 1,
>     .max_vs_threads = 36,
>     .max_hs_threads = 36,
> @@ -137,7 +136,6 @@ static const struct brw_device_info brw_device_info_ivb_gt1 = {
>  
>  static const struct brw_device_info brw_device_info_ivb_gt2 = {
>     GEN7_FEATURES, .is_ivybridge = true, .gt = 2,
> -   .needs_unlit_centroid_workaround = true,
>     .num_slices = 1,
>     .max_vs_threads = 128,
>     .max_hs_threads = 128,
> @@ -158,7 +156,6 @@ static const struct brw_device_info brw_device_info_ivb_gt2 = {
>  
>  static const struct brw_device_info brw_device_info_byt = {
>     GEN7_FEATURES, .is_baytrail = true, .gt = 1,
> -   .needs_unlit_centroid_workaround = true,
>     .num_slices = 1,
>     .has_llc = false,
>     .max_vs_threads = 36,
> 

Sounds good to me.  The fact that the Piglit tests pass on Ivybridge
but not on Sandybridge is pretty compelling.  Plus, Sandybridge was
the first platform with multisampling, so it sounds very plausible
that they would have fixed it for round 2 with Ivybridge.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160801/b078840c/attachment-0001.sig>