Bug 91513

Summary: [IVB/HSW/BDW/SKL Bisected] Lightsmark performance reduced by 7%-10%
Product: Mesa Reporter: wendy.wang
Component: Drivers/DRI/i965Assignee: Matt Turner <mattst88>
Status: RESOLVED FIXED QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: major    
Priority: medium CC: eero.t.tamminen
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: Xorg.0.log info
untested patch

Description wendy.wang 2015-07-31 02:09:58 UTC 
System Environment: 
Failed platform: IVB-M, HSW-GT3e, BDW-U F0 stepping CPU, SKL-Y
Mesa Regression: Yes
2015-07-29 unstable GFX SW stack still can reproduce this bug:
 Libdrm:		(master)libdrm-2.4.62-11-g1a6efaf68e207302cd9423051b8091fa663bbabe
 Mesa:		(master)e17056f5a20beb752a530180fce1aba0e68877b6
 Xserver:		(master)xorg-server-1.17.0-284-ga8a0f6464a33c12c1de495d74fd478c0d952643e
 Xf86_video_intel:		(master)2.99.917-401-g4f0a58c9db4d1ac32a79c87392454ad859912f47
 Kernel:   (drm-intel-nightly)bae71bf8fb9a4975ac22f2df756d478856764f82


Bug detailed description:
Lightsmark performacne reduced by 7% --10% because of the Meas commit: 
	BDW dropped by -7.75%
	IVB-M dropped by -7.17%
	HSW-GT3e dropped by -5.51%
	SKL-Y dropped by -10.74%

Bisect result show the first bad commit is 3a318766.

3a31876600cb5c4d90c998ecb5635c602eeb2bd1 is the first bad commit
commit 3a31876600cb5c4d90c998ecb5635c602eeb2bd1
Author: Ben Widawsky <benjamin.widawsky@intel.com>
Date:   Tue Jul 14 09:56:09 2015 -0700

    i965: Push miptree tiling request into flags

    With the last few patches a way was provided to influence lower layer miptre                                                                                                                     e
    layout and allocation decisions via flags (replacing bools). For simplicity,                                                                                                                      I
    chose not to touch the tiling requests because the change was slightly less
    mechanical than replacing the bools.

    The goal is to organize the code so we can continue to add new parameters an                                                                                                                     d
    tiling types while minimizing risk to the existing code, and not having to
    constantly add new function parameters.

    v2: Rebased on Anuj's recent Yf/Ys changes
    Fix non-msrt MCS allocation (was only happening in gen8 case before)

    v3: small fix in assertion requested by Chad

    v4: Use parens to get the order right from v3.

    Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
    Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>




Reproduce Steps:
1. cd /usr/local/games/OpenGL/lightsmark-1.2.0/Lightsmark2008.2.0/bin/pc-linux64
2. vblank_mode=0 ./backend silent 1920x1080
Comment 1 ye.tian 2015-07-31 02:31:22 UTC 
Created attachment 117470 [details]
Xorg.0.log info
Comment 2 Matt Turner 2015-07-31 21:28:12 UTC 
Created attachment 117475 [details] [review]
untested patch

I see one problem:

-   switch (requested) {
-   case INTEL_MIPTREE_TILING_ANY:
+   switch (layout_flags & MIPTREE_LAYOUT_ALLOC_ANY_TILED) {
+   case MIPTREE_LAYOUT_ALLOC_ANY_TILED:
       break;
-   case INTEL_MIPTREE_TILING_Y:
+   case MIPTREE_LAYOUT_ALLOC_YTILED:
       return I915_TILING_Y;
-   case INTEL_MIPTREE_TILING_NONE:
+   case MIPTREE_LAYOUT_ALLOC_LINEAR:
       return I915_TILING_NONE;
    }
 
but we have

+   MIPTREE_LAYOUT_ALLOC_YTILED             = 1 << 5,
+   MIPTREE_LAYOUT_ALLOC_XTILED             = 1 << 6,
+   MIPTREE_LAYOUT_ALLOC_LINEAR             = 1 << 7,
 };
 
+#define MIPTREE_LAYOUT_ALLOC_ANY_TILED (MIPTREE_LAYOUT_ALLOC_YTILED | \
+                                        MIPTREE_LAYOUT_ALLOC_XTILED)

the third case (MIPTREE_LAYOUT_ALLOC_LINEAR) in the switch is now impossible to reach. I think it is supposed to be "case 0:". Please test the (untested) attached patch.
Comment 3 ye.tian 2015-08-03 07:58:18 UTC 
Test it on the latest mesa with the patch, this problem does not exist.
Comment 4 Matt Turner 2015-08-06 19:31:34 UTC 
Fixed by

commit 1c175fc2e3a685b531920dec247086463ab9a154
Author: Matt Turner <mattst88@gmail.com>
Date:   Tue Aug 4 22:58:08 2015 -0700

    i965: Correct a mistake that always forced texture tiling.

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.