Bug 92975 - [SKL][v4.3 bisected]SKL OpenCL performance reduced hugely
Summary: [SKL][v4.3 bisected]SKL OpenCL performance reduced hugely
Status: VERIFIED FIXED
Alias: None
Product: Beignet
Classification: Unclassified
Component: Beignet (show other bugs)
Version: unspecified
Hardware: All Linux (All)
: highest critical
Assignee: Francisco Jerez
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-17 06:31 UTC by meng
Modified: 2015-12-16 00:44 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
0001-SKL-Use-kernel-defined-MOCS-values-instead-of-assumi.patch (982 bytes, patch)
2015-11-23 14:59 UTC, Francisco Jerez
Details | Splinter Review

Description meng 2015-11-17 06:31:38 UTC
==Regression==
--------------------------
Regression: Yes 
Ubuntu: 14.04

==kernel==
--------------------------
bad commit: v4.3-rc1 
good commit: v4.2.6 

==Test cases==
OpenCV: git://code.opencv.org/opencv.git
OpenCL 1.2 Code Samples: https://software.intel.com/en-us/intel-opencl-support/code-samples
Luxmark v2.1: http://www.luxrender.net/wiki/LuxMark

==Bug detailed description==
Compared kernel v4.3 v.s. v4.1, many OpenCL performance (e.g. OpenCV 3.0, Luxmark v2.1) reduced hugely(>50%) on SKL. The issue can't be reproduced on BDW.

I can't find out the first bad commit because SKL can't boot with some bisected kernels.

==Reproduce steps==
---------------------------- 
./luxmark --scene=SALA  --mode=BENCHMARK_OCL_GPU --single-run (Luxmark v2.1 SALA)
Comment 1 Jani Nikula 2015-11-17 10:19:38 UTC
(In reply to meng from comment #0)
> I can't find out the first bad commit because SKL can't boot with some
> bisected kernels.

Please try 'git bisect skip' or choose commits yourself to try to proceed with the bisect.

I presume you have i915.preliminary_hw_support=1 param set for pre-v4.3 kernels.
Comment 2 meng 2015-11-18 08:08:02 UTC
(In reply to Jani Nikula from comment #1)
> (In reply to meng from comment #0)
> > I can't find out the first bad commit because SKL can't boot with some
> > bisected kernels.
> 
> Please try 'git bisect skip' or choose commits yourself to try to proceed
> with the bisect.
> 
> I presume you have i915.preliminary_hw_support=1 param set for pre-v4.3
> kernels.

No "i915.preliminary_hw_support=1" set in grub.
All our tests are in the same environment e.g. the same kernel parameter.


By skipping many commits, found out the first bad commit:
commit 3bbaba0ceaa254c9ee261e329bfd92e4ba4fe32a
Author: Peter Antoine <peter.antoine@intel.com>
Date:   Fri Jul 10 20:13:11 2015 +0300

    drm/i915: Added Programming of the MOCS
Comment 3 rongyang 2015-11-18 08:29:30 UTC
Beignet use the defaults MOCS table index 9, don't this commit change its value?
Comment 4 Jani Nikula 2015-11-18 09:05:07 UTC
(In reply to meng from comment #2)
> No "i915.preliminary_hw_support=1" set in grub.

Before v4.3 you don't get SKL gfx support without that, or CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT=y.

When running pre-v4.3, do you see something along these lines in the dmesg:

"""
This hardware requires preliminary hardware support.
See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support
"""

> By skipping many commits, found out the first bad commit:
> commit 3bbaba0ceaa254c9ee261e329bfd92e4ba4fe32a
> Author: Peter Antoine <peter.antoine@intel.com>
> Date:   Fri Jul 10 20:13:11 2015 +0300
> 
>     drm/i915: Added Programming of the MOCS

Please pick a badly performing kernel (verify that it does!) and then try this on top:

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 06180dce954e..0eaeccdc5a49 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1867,7 +1867,7 @@ static int gen8_init_rcs_context(struct drm_i915_gem_request *req)
 	if (ret)
 		return ret;
 
-	ret = intel_rcs_context_init_mocs(req);
+	ret = 1;
 	/*
 	 * Failing to program the MOCS is non-fatal.The system will not
 	 * run at peak performance. So generate an error and carry on.
Comment 5 Jani Nikula 2015-11-18 09:23:44 UTC
Oh, and please check that your dmesg *before* trying the patch above do *not* contain "MOCS failed to program: expect performance issues." error.
Comment 6 meng 2015-11-18 13:10:08 UTC
Just check, "CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT=y" is in our .config(both v4.3 and pre_v4.3).

try the above patch on the first bad commit, the OpenCL performance issues can't be reproduced. 
BTW, no the patch, no "*ERROR* MOCS failed" in dmesg.
if with the patch, there is "*ERROR* MOCS failed
Comment 7 Jani Nikula 2015-11-18 13:39:27 UTC
(In reply to meng from comment #6)
> Just check, "CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT=y" is in our
> .config(both v4.3 and pre_v4.3).

Ah, okay.

> try the above patch on the first bad commit, the OpenCL performance issues
> can't be reproduced. 

Thanks for testing; may I ask for two more test rounds, first current drm-intel-nightly branch of http://cgit.freedesktop.org/drm-intel, and then drm-intel-nightly + the patch from comment #4.

> BTW, no the patch, no "*ERROR* MOCS failed" in dmesg.
> if with the patch, there is "*ERROR* MOCS failed

This is expected, thanks for checking.
Comment 8 meng 2015-11-19 01:35:43 UTC
(In reply to Jani Nikula from comment #7)
The issue also exists on drm-intel-nightly latest commit:git-2e0db751(2015y-11m-18d-19h-48m-48s).
And it should be fixed with patch.
Comment 9 Jani Nikula 2015-11-23 10:18:45 UTC
(In reply to meng from comment #8)
> (In reply to Jani Nikula from comment #7)
> The issue also exists on drm-intel-nightly latest
> commit:git-2e0db751(2015y-11m-18d-19h-48m-48s).
> And it should be fixed with patch.

It's not "fixed". The patch from comment #4 merely bypasses MOCS initialization, i.e. confirms that MOCS init causes the perf regression.
Comment 10 Francisco Jerez 2015-11-23 14:58:32 UTC
Changing product category of the bug to Beignet.  Beignet shouldn't assume that the MOCS tables are set to the hardware defaults, but instead use the MOCS values exposed by the kernel.  The kernel-defined MOCS entry 2 is roughly equivalent to the hardware-default entry you seemed to be using previously, the (completely untested) attached patch should fix the issue.
Comment 11 Francisco Jerez 2015-11-23 14:59:25 UTC
Created attachment 120049 [details] [review]
0001-SKL-Use-kernel-defined-MOCS-values-instead-of-assumi.patch
Comment 12 rongyang 2015-12-02 05:32:31 UTC
Thanks for your patch.
But how could beignet got the MOCS values exposed by the kernel, if kernel will change these values in the future, how could beignet get the needed one?
Comment 13 meng 2015-12-02 05:51:49 UTC
(In reply to Francisco Jerez from comment #11)
> Created attachment 120049 [details] [review] [review]
> 0001-SKL-Use-kernel-defined-MOCS-values-instead-of-assumi.patch

Yes, Beignet with the patch, the performance is OK.
Comment 14 Francisco Jerez 2015-12-02 15:19:48 UTC
(In reply to rongyang from comment #12)
> Thanks for your patch.
> But how could beignet got the MOCS values exposed by the kernel, if kernel
> will change these values in the future, how could beignet get the needed one?

The meaning of the MOCS entries already exposed by the kernel shouldn't ever change in a non-backwards-compatible way, only new entries will be added at the end of the table.  The exposed MOCS entries are considered part of the kernel ABI so in principle you can rely on them staying the same in future kernel versions.
Comment 15 rongyang 2015-12-04 03:20:50 UTC
Get it. Can you send your patch to beignet mailing list beignet@lists.freedesktop.org? Then I will push it. Thanks.
Comment 16 Francisco Jerez 2015-12-04 13:31:09 UTC
(In reply to rongyang from comment #15)
> Get it. Can you send your patch to beignet mailing list
> beignet@lists.freedesktop.org? Then I will push it. Thanks.

Sure, done.
Comment 17 meng 2015-12-16 00:44:53 UTC
Verified it.


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.