Bug 92975

(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.

(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

Beignet use the defaults MOCS table index 9, don't this commit change its value?

(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.

Oh, and please check that your dmesg *before* trying the patch above do *not* contain "MOCS failed to program: expect performance issues." error.

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

(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.

(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.

(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.

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.

Created attachment 120049 [details] [review]
0001-SKL-Use-kernel-defined-MOCS-values-instead-of-assumi.patch

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?

(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.

(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.

Get it. Can you send your patch to beignet mailing list beignet@lists.freedesktop.org? Then I will push it. Thanks.

(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.

Verified it.