Bug 103258 - [GEN9+] 5% performance drop + misrendering in GfxBench Manhattan 3.0 & CarChase from "i965: Disable auxiliary buffers when there are self-dependencies"
Summary: [GEN9+] 5% performance drop + misrendering in GfxBench Manhattan 3.0 & CarCha...
Status: VERIFIED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Kenneth Graunke
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
: 103247 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-10-13 11:45 UTC by Eero Tamminen
Modified: 2017-11-09 16:39 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:

Attachments
Example of Manhattan 3.0 misrendering (134.18 KB, image/jpeg)
2017-10-13 11:45 UTC, Eero Tamminen 		Details
CarChase rendering before commit (2.03 MB, image/png)
2017-10-17 08:44 UTC, Eero Tamminen 		Details
CarChase rendering before commit (352.58 KB, image/jpeg)
2017-10-17 08:46 UTC, Eero Tamminen 		Details
CarChase rendering after commit (336.76 KB, image/jpeg)
2017-10-17 08:47 UTC, Eero Tamminen 		Details
Show Obsolete (1) View All

Description Eero Tamminen 2017-10-13 11:45:33 UTC 
Created attachment 134830 [details]
Example of Manhattan 3.0 misrendering

Between following Mesa commits:
e676434856 2017-10-10 17:45:22 UTC Implement a new GL_MESA_tile_raster_order extension
3c66a461f3 2017-10-11 17:45:31 UTC meson: fix glx test

Manhattan 3.0 rendering has broken, there are now two transparent horizontal bands on top of the screen bottom.  See the attachment.

Additionally there was (values are from KLB GT3e):
* 5-6% perf drop in CarChase
* 4% perf drop in Manhattan 3.1

These regressions happen on all GEN9+ platforms.

Martin's bisecting this, I'll add the exact commit once the bisecting finishes.


(Additionally, with Mesa Git tip, I'm also getting GfxBench NULL pointer segfault/aborts when testfw_app starts, at least on SKL GT2.)
Comment 1 Eero Tamminen 2017-10-13 13:06:30 UTC 
Martin's ezBench bisected the performance regression to following commit:
------------------------------------------------------
commit ea0d2e98ecb369ab84e78c84709c0930ea8c293a
Author:     Kenneth Graunke <kenneth@whitecape.org>
AuthorDate: Thu Oct 5 20:31:01 2017 -0700
Commit:     Kenneth Graunke <kenneth@whitecape.org>
CommitDate: Tue Oct 10 14:57:04 2017 -0700

    i965: Disable auxiliary buffers when there are self-dependencies.
    
    Jason and I investigated several OpenGL CTS failures where the tests
    bind the same texture for rendering and texturing, at the same time.
    This has defined results as long as the reads happen before writes,
    or the regions are non-overlapping.  Normally, this just works out.
    
    However, CCS can cause problems.  If the shader is reading one set of
    pixels, and writing to different pixels that are adjacent, they may end
    up being covered by the same CCS block.  So rendering may be writing a
    CCS block, while the sampler is trying to read it.  Corruption ensues.
    
    Disabling CCS is unfortunate, but safe.
    
    Fixes several KHR-GL45.texture_barrier.* subtests.
    
    Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>
    Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
------------------------------------------------------

(On SKL GT2, both tests drop ~4%.)

There isn't yet render validation for GfxBench, so bisecting that will take a bit more work.
Comment 2 Eero Tamminen 2017-10-13 13:17:00 UTC 
Checked manually, same commit is also the cause for the misrendering.
Comment 3 Mark Janes 2017-10-13 15:47:36 UTC 
possible duplicate of 103247
Comment 4 Eero Tamminen 2017-10-16 11:06:40 UTC 
There's a very small drop also in Manhattan 3.1.
Comment 5 Mark Janes 2017-10-16 21:33:11 UTC 
*** Bug 103247 has been marked as a duplicate of this bug. ***
Comment 6 Eero Tamminen 2017-10-17 08:44:46 UTC 
Created attachment 134877 [details]
CarChase rendering before commit
Comment 7 Eero Tamminen 2017-10-17 08:46:26 UTC 
Created attachment 134878 [details]
CarChase rendering before commit
Comment 8 Eero Tamminen 2017-10-17 08:47:49 UTC 
Created attachment 134879 [details]
CarChase rendering after commit

The commit changes also rendering of all CarChase frames, it's just harder to see than in Manhattan without comparing the before & after frames.
Comment 9 Kenneth Graunke 2017-10-19 05:44:26 UTC 
Patches on the mailing list:
https://patchwork.freedesktop.org/series/32263/
Comment 10 Eero Tamminen 2017-10-19 14:17:59 UTC 
(In reply to Kenneth Graunke from comment #9)
> Patches on the mailing list:
> https://patchwork.freedesktop.org/series/32263/

Thanks, this patch series fixes GfxBench performance regressions and (at least Manhattan) mis-rendering.

Additionally, it seems to help GpuTest v0.7 FurMark performance by 3-6% (depending on platform).
Comment 11 Eero Tamminen 2017-10-19 14:30:50 UTC 
(In reply to Eero Tamminen from comment #10)
> Additionally, it seems to help GpuTest v0.7 FurMark performance by 3-6%
> (depending on platform).

Only 2% on SKL i5 GT2.
Comment 12 Kenneth Graunke 2017-10-19 18:11:39 UTC 
The performance regression should be fixed by the series ending in:

commit 82144b7392d1651167cbfd4746aea6a14d5aff46
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Thu Oct 12 20:47:41 2017 -0700

    i965: Don't disable aux buffers for non-overlapping miplevels.
    
    Meta's GenerateMipmap implementation binds the same image for both
    sampling and rendering - but it samples from one miplevel while
    rendering the next.  This is a false self-dependency, and there's
    no need to disable auxiliary buffers in this case.  In fact, we really
    want to leave it enabled so the new miplevels gain color compression.
    
    Thankfully, the texture object's _MaxLevel is always one shy of the
    miplevel being rendered.  So we can simply check if irb->mt_level is
    overlaps with the texture's defined levels.  If not, there's no self-
    dependency and we can leave the auxiliary buffers enabled.
    
    Fixes a performance regression in GFXBench4 Car Chase, which apparently
    calls glGenerateMipmap() on every frame.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103247
    Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
    Reviewed-by; Jason Ekstrand <jason@jlekstrand.net>

I'm still not sure why the rendering changed - there's probably a bug there still - but I don't have time to investigate that right now.
Comment 13 Eero Tamminen 2017-10-24 14:14:54 UTC 
Verified, perf is back at previous level and Manhattan rendering is fine.

(During same day there was also several percent increase to FurMark, I haven't yet bisected where that comes from.)
Comment 14 Eero Tamminen 2017-11-09 16:39:41 UTC 
Verified that CarChase rendering got also fixed (Martin's ezBench instance  bisection finally reached this).

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.