There is code in the routine intel_pstate_timer_func to "kick" the CPU out of the minimum pstate. The code was added back when several things were not working properly with the driver. It was added due to a "ping-pong" issue uncovered by the Phoronix test suite ffpmeg test. I have tried extensively to recreate the issue with the "kick" code removed and have not been able to. I am unable to detect any difference with the ffmpeg test between the normal code and code with the "kick removed" The assertion is that the "kick" code is not required and should be deleted. This bug has been extracted from bug 59481 which ended up discussing a few tangent issues. The main issue was solved and the bug closed. Now I am following up on some of the tangent issues. I am saying the kick portion of this code can be removed because it serves no purpose and makes the code harder to read and understand: static void intel_pstate_timer_func(unsigned long __data) { struct cpudata *cpu = (struct cpudata *) __data; intel_pstate_sample(cpu); intel_pstate_adjust_busy_pstate(cpu); if (cpu->pstate.current_pstate == cpu->pstate.min_pstate) { cpu->min_pstate_count++; if (!(cpu->min_pstate_count % 5)) { intel_pstate_set_pstate(cpu, cpu->pstate.max_pstate); } } else cpu->min_pstate_count = 0; intel_pstate_set_sample_time(cpu); } Where this was left on 2013.07.18 with Dirk Brandewie was that he wanted to go back and verify in his test environment. Which, of course, should be done.
Adding some phoronix ffmpeg test results: Kernel: 3.12RC7 with No Kick modification. Linux s15 3.12.0-rc7-nokick #49 SMP Sat Nov 2 21:40:01 PDT 2013 x86_64 x86_64 x86_64 GNU/Linux Mode: Powersave with default settings. Sample size 250 tests. (the limit seems to be 256) Test run 1 is rejected as it includes extra time to load the test file. For subsequent tests, the file is already cached. ave 13.475713 seconds max 13.70577598 seconds min 13.29449391 seconds stddev 0.073586378 seconds Kernel: 3.12RC7 Linux s15 3.12.0-rc7+ #48 SMP Fri Nov 1 21:09:51 PDT 2013 x86_64 x86_64 x86_64 GNU/Linux Mode: Performance with default settings. Sample size 250 tests. Test run 1 is NOT rejected as, due an error on my part, I restarted the test and the so the test file was already cached. ave 13.4565717 seconds max 13.66979098 seconds min 13.2799778 seconds stddev 0.070516137 seconds Kernel: 3.12RC7 Linux s15 3.12.0-rc7+ #48 SMP Fri Nov 1 21:09:51 PDT 2013 x86_64 x86_64 x86_64 GNU/Linux Mode: Powersave with default settings. Sample size 250 tests. Test run 1 is NOT rejected as the test had been just run under different conditions so the test file was already cached. ave 13.45689662 seconds min 13.64681292 seconds max 13.30776 seconds stddev 0.067261141 seconds System Information Hardware: Processor: Intel Core i7-2600K @ 3.80GHz (8 Cores), Motherboard: ASUS P8Z68-M PRO, Chipset: Intel 2nd Generation Core Family DRAM, Memory: 16384MB, Disk: 1000GB Seagate ST1000DM003-9YN1 + 1000GB Seagate ST1000DL002-9TT1 + 120GB INTEL SSDSA2CW12, Graphics: Intel 2nd Generation Core Family IGP, Audio: Realtek ALC892, Network: Intel 82574L Gigabit Connection Software: OS: Ubuntu 12.04, Kernel: 3.12.0-rc7+ (x86_64), Compiler: GCC 4.6, File-System: ext4, Screen Resolution: 1680x1050
Agreed. Do you want to submit the patch or should I?
Dirk: I do not know how to submit a patch. Which doesn't mean I shouldn't learn how, but I have also been cheating when I make my "no kick" test code by just deleting the 3 lines that decide to and do the kick. The real solution would be a bit more. I'll do it of you want (it would be a good way to get rid of me for awhile (just joking)).
Here is the patch I will send upstream it may not make it until v3.14-rc1 since the merge window is upon us :-) commit 7d9aba45dbd356686eca097b31028eccec0e11d6 Author: Dirk Brandewie <dirk.j.brandewie@intel.com> Date: Mon Oct 14 07:54:45 2013 -0700 cpufreq/intel_pstate: Remove periodic P state boost Remove the periodic P state boost. This code required for some corner case benchmark tests. The calculation of the required P state was incorrect/inaccurate and would not allow P state increase. This was fixed by a combination of commits: 2134ed4 cpufreq / intel_pstate: Change to scale off of max P-state d253d2a intel_pstate: Improve accuracy by not truncating until final result Fixes Bug: https://bugzilla.kernel.org/show_bug.cgi?id=64271 Reported-by: dsmythies@telus.net Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com> --- drivers/cpufreq/intel_pstate.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 26ab4b8..4d07ae2 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -83,8 +83,6 @@ struct cpudata { struct pstate_data pstate; struct _pid pid; - int min_pstate_count; - u64 prev_aperf; u64 prev_mperf; int sample_ptr; @@ -567,15 +565,6 @@ static void intel_pstate_timer_func(unsigned long __data) intel_pstate_sample(cpu); intel_pstate_adjust_busy_pstate(cpu); - - if (cpu->pstate.current_pstate == cpu->pstate.min_pstate) { - cpu->min_pstate_count++; - if (!(cpu->min_pstate_count % 5)) { - intel_pstate_set_pstate(cpu, cpu->pstate.max_pstate); - } - } else - cpu->min_pstate_count = 0; - intel_pstate_set_sample_time(cpu); }
Dirk: Thanks. Note that I have tested this to death already, and now I only run with a nokick version of intel_pstate.
commit 91a4cd4f3d8169d7398f9123683f64575927c682 Author: Dirk Brandewie <dirk.j.brandewie@intel.com> Date: Tue Dec 17 09:42:07 2013 -0800 intel_pstate: Remove periodic P state boost entered Linus' tree as v3.14 material.