Bug 106708 - [SKL/KBL/GLK] 2-3% performance drop in SynMark DrvState and 5-9% drop on SynMark Multithread
Summary: [SKL/KBL/GLK] 2-3% performance drop in SynMark DrvState and 5-9% drop on SynM...
Status: VERIFIED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Intel 3D Bugs Mailing List
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-29 11:27 UTC by Eero Tamminen
Modified: 2018-05-30 10:24 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

Attachments

Description Eero Tamminen 2018-05-29 11:27:58 UTC 
Mesa performance with SynMark v7 DrvState performance dropped by 2-3% between following Mesa commits:
2018-05-21 11:57:46 73df16dcee: radv: fix centroid interpolation
2018-05-22 17:02:28 92f01fc5f9: i965: Emit VF cache invalidates for 48-bit addressing bugs with softpin.

This performance drop is clearly visible on SKL GT2, GT3e, GT4e, KBL GT2, GT3e and GKL B0.  On BXT devices there's too much variance in this test to be sure whether they're affected.

In the same interval there's even larger drop in SynMark v7 Multithread test on the same machines.  That drop isn't visible on BXT.

These regressions are GEN9+ specific, they aren't visible on GEN6-8 machines.


Only commits in that range which could have impact on these machines are:
92f01fc5f9 i965: Emit VF cache invalidates for 48-bit addressing bugs with softpin.
c7259259d4 i965: Introduce a "memory zone" concept on BO allocation.
417b9e5770 intel/eu: Set EXECUTE_1 when setting the rounding mode in cr0
fe2edb25dd dri3: Stricter SBC wraparound handling

First one affects GEN8+, next two are more generic ones, and last one is X hang fix.  None of them is GEN9+ specific, so somebody would need to bisect these two regressions further.
Comment 1 Chris Wilson 2018-05-29 16:24:12 UTC 
From inspection, 92f01fc5f914fd500497d0c3aed75f3ac8dc054d:

+static void
+vf_invalidate_for_vb_48bit_transitions(struct brw_context *brw)
+{
+#if GEN_GEN >= 8
+   bool need_invalidate = true;
+   unsigned i;

that need_invalidate should be initialised to false
Comment 2 Kenneth Graunke 2018-05-29 19:19:15 UTC 
Should be fixed by:

commit 3ac5fbadfd8644d30fce9ff267cb811ad157996a
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue May 29 17:28:07 2018 +0100

    i965: Only emit VF cache invalidations when the high bits changes
    
    Commit 92f01fc5f914 ("i965: Emit VF cache invalidates for 48-bit
    addressing bugs with softpin.") tried to only emit the VF invalidate if
    the high bits changed, but it accidentally always set need_invalidate to
    true; causing it to emit unconditionally emit the pipe control before
    every primitive.
    
    Fixes: 92f01fc5f914 ("i965: Emit VF cache invalidates for 48-bit addressing bugs with softpin.")
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106708
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Comment 3 Eero Tamminen 2018-05-30 10:24:17 UTC 
Verified.  Both performance regressions go away with Mesa git.

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.