Messages in this thread | | | Subject | Re: [PATCH v2 2/2] cpufreq: intel_pstate: Allow raw energy performance preference value | From | srinivas pandruvada <> | Date | Wed, 24 Jun 2020 11:02:01 -0700 |
| |
Hi Doug,
On Wed, 2020-06-24 at 08:37 -0700, Doug Smythies wrote: > Hi Srinivas, > > I have immediate need for this. I have been using a tool > I wrote myself for this which I can now retire. > (it wasn't very good anyway). > Yours remembers for each governor, and is way better. > Thanks. > I will incorporate your changes and re-post.
Thanks, Srinivas
> On 2020.06.23 11:27 Srinivas Pandruvada wrote: > > > Currently using attribute "energy_performance_preference", user > > space can > > write one of the four per-defined preference string. These > > preference > > strings gets mapped to a hard-coded Energy-Performance Preference > > (EPP) or > > Energy-Performance Bias (EPB) knob. > > > > These four values supposed to cover broad spectrum of use cases, > > but they > > are not uniformly distributed in the range. > > Suggest: > > These four values are supposed to cover broad spectrum of use cases, > but > are not uniformly distributed in the range. > > > There are number of cases, > > where this is not enough. For example: > > > > Suppose user wants more performance when connected to AC. Instead > > of using > > default "balance performance", the "performance" setting can be > > used. This > > changes EPP value from 0x80 to 0x00. But setting EPP to 0, results > > in > > electrical and thermal issues on some platforms. > > This results in CPU to do > > aggressive throttling, which causes drop in performance. > > Suggest: > > This results in aggressive throttling, which causes a drop in > performance. > > And: > > Tough. > I consider "performance mode" as sacrosanct, and have always > expected these to behave identically and at max CPU freq: > > intel_pstate no-hwp / performance > intel_cpufreq no-hwp / performance (a.k.a. passive) > acpi_cpufreq / performance > intel_pstate hwp / performance > intel_cpufreq hwp / performance (in future) > > as was always the case on my i7-2600K (no hwp) based computer > and is not the case on my i5-9600K (hwp capable) computer. > > But some value > > between 0x80 and 0x00 results in better performance. But that value > > can't > > be fixed as the power curve is not linear. In some cases just > > changing EPP > > from 0x80 to 0x75 is enough to get significant performance gain. > > > > Similarly on battery EPP 0x80 can be very aggressive in power > > consumption. > > But picking up the next choice "balance power" results in too much > > loss > > of performance, which cause bad user experience in use case like > > "Google > > Hangout". It was observed that some value between these two EPP is > > optimal. > > > > This change allows fine grain EPP tuning for platform like > > Chromebooks. > > Here based on the product and use cases, different EPP values can > > be set. > > This change is similar to the change done for: > > /sys/devices/system/cpu/cpu*/power/energy_perf_bias > > where user has choice to write a predefined string or raw value. > > > > The change itself is trivial. When user preference doesn't match > > predefined string preferences and value is an unsigned integer and > > in > > range, use that value for EPP. When the EPP feature is not prsent > ^^^^^^ > s/prsent/present > > > writing raw value is not supported. > > > > Suggested-by: Len Brown <lenb@kernel.org> > > Signed-off-by: Srinivas Pandruvada < > > srinivas.pandruvada@linux.intel.com> > > --- > > Documentation/admin-guide/pm/intel_pstate.rst | 6 ++- > > drivers/cpufreq/intel_pstate.c | 50 > > +++++++++++++++---- > > 2 files changed, 45 insertions(+), 11 deletions(-) > > > > diff --git a/Documentation/admin-guide/pm/intel_pstate.rst > > b/Documentation/admin- > > guide/pm/intel_pstate.rst > > index 939bfdc53f4f..5e209926e0ed 100644 > > --- a/Documentation/admin-guide/pm/intel_pstate.rst > > +++ b/Documentation/admin-guide/pm/intel_pstate.rst > > @@ -561,7 +561,11 @@ somewhere between the two extremes: > > Strings written to the ``energy_performance_preference`` attribute > > are > > internally translated to integer values written to the processor's > > Energy-Performance Preference (EPP) knob (if supported) or its > > -Energy-Performance Bias (EPB) knob. > > +Energy-Performance Bias (EPB) knob. It is also possible to write a > > positive > > +integer value between 0 to 255, if the EPP feature is present. If > > the EPP > > +feature is not present, writing integer value to this attribute is > > not > > +supported. In this case, user can use > > + "/sys/devices/system/cpu/cpu*/power/energy_perf_bias" interface. > > > > [Note that tasks may by migrated from one CPU to another by the > > scheduler's > > load-balancing algorithm and if different energy vs performance > > hints are > > diff --git a/drivers/cpufreq/intel_pstate.c > > b/drivers/cpufreq/intel_pstate.c > > index 1cf6d06f2314..d8f195c7a428 100644 > > --- a/drivers/cpufreq/intel_pstate.c > > +++ b/drivers/cpufreq/intel_pstate.c > > @@ -602,11 +602,12 @@ static const unsigned int epp_values[] = { > > HWP_EPP_POWERSAVE > > }; > > > > -static int intel_pstate_get_energy_pref_index(struct cpudata > > *cpu_data) > > +static int intel_pstate_get_energy_pref_index(struct cpudata > > *cpu_data, int *raw_epp) > > { > > s16 epp; > > int index = -EINVAL; > > > > + *raw_epp = 0; > > epp = intel_pstate_get_epp(cpu_data, 0); > > if (epp < 0) > > return epp; > > @@ -614,12 +615,14 @@ static int > > intel_pstate_get_energy_pref_index(struct cpudata *cpu_data) > > if (boot_cpu_has(X86_FEATURE_HWP_EPP)) { > > if (epp == HWP_EPP_PERFORMANCE) > > return 1; > > - if (epp <= HWP_EPP_BALANCE_PERFORMANCE) > > + if (epp == HWP_EPP_BALANCE_PERFORMANCE) > > return 2; > > - if (epp <= HWP_EPP_BALANCE_POWERSAVE) > > + if (epp == HWP_EPP_BALANCE_POWERSAVE) > > return 3; > > - else > > + if (epp == HWP_EPP_POWERSAVE) > > return 4; > > + *raw_epp = epp; > > + return 0; > > } else if (boot_cpu_has(X86_FEATURE_EPB)) { > > /* > > * Range: > > @@ -638,7 +641,8 @@ static int > > intel_pstate_get_energy_pref_index(struct cpudata *cpu_data) > > } > > > > static int intel_pstate_set_energy_pref_index(struct cpudata > > *cpu_data, > > - int pref_index) > > + int pref_index, bool > > use_raw, > > + u32 raw_epp) > > { > > int epp = -EINVAL; > > int ret; > > @@ -657,6 +661,16 @@ static int > > intel_pstate_set_energy_pref_index(struct cpudata *cpu_data, > > > > value &= ~GENMASK_ULL(31, 24); > > > > + if (use_raw) { > > + if (raw_epp > 255) { > > + ret = -EINVAL; > > + goto return_pref; > > + } > > + value |= (u64)raw_epp << 24; > > + ret = wrmsrl_on_cpu(cpu_data->cpu, > > MSR_HWP_REQUEST, value); > > + goto return_pref; > > + } > > + > > if (epp == -EINVAL) > > epp = epp_values[pref_index - 1]; > > > > @@ -694,6 +708,8 @@ static ssize_t > > store_energy_performance_preference( > > { > > struct cpudata *cpu_data = all_cpu_data[policy->cpu]; > > char str_preference[21]; > > + bool raw = false; > > + u32 epp; > > int ret; > > > > ret = sscanf(buf, "%20s", str_preference); > > @@ -701,10 +717,21 @@ static ssize_t > > store_energy_performance_preference( > > return -EINVAL; > > > > ret = match_string(energy_perf_strings, -1, str_preference); > > - if (ret < 0) > > + if (ret < 0) { > > + if (!boot_cpu_has(X86_FEATURE_HWP_EPP)) > > + return ret; > > + > > + ret = kstrtouint(buf, 10, &epp); > > + if (ret) > > + return ret; > > + > > + raw = true; > > + } > > + > > + ret = intel_pstate_set_energy_pref_index(cpu_data, ret, raw, > > epp); > > + if (ret) > > return ret; > > > > - intel_pstate_set_energy_pref_index(cpu_data, ret); > > return count; > > } > > > > @@ -712,13 +739,16 @@ static ssize_t > > show_energy_performance_preference( > > struct cpufreq_policy *policy, char > > *buf) > > { > > struct cpudata *cpu_data = all_cpu_data[policy->cpu]; > > - int preference; > > + int preference, raw_epp; > > > > - preference = intel_pstate_get_energy_pref_index(cpu_data); > > + preference = intel_pstate_get_energy_pref_index(cpu_data, > > &raw_epp); > > if (preference < 0) > > return preference; > > > > - return sprintf(buf, "%s\n", energy_perf_strings[preference]); > > + if (raw_epp) > > + return sprintf(buf, "%d\n", raw_epp); > > + else > > + return sprintf(buf, "%s\n", > > energy_perf_strings[preference]); > > } > > > > cpufreq_freq_attr_rw(energy_performance_preference); > > -- > > 2.25.4
| |