Hi All,
There is a problem in intel_pstate that if it works in the passive mode with
HWP enabled and the "powersave" governor is used on top of it, then changing
the policy max frequency doesn't cause the HWP max limit to be updated which
is quite confusing.
That happens because of two checks, one in the cpufreq core and one in the
driver itself, that are there to avoid unnecessary HW/FW updates when the
current frequency doesn't change. Of course, that is the case when the
policy max limit changes under the "powersave" governor (which sets the
current frequency to the policy min limit), but in that particular case,
the checks turn out to be harmful. This is dealt with by the first patch.
The second one is an optimization that can be done right away on top of the
first one.
Thanks!
From: Rafael J. Wysocki <[email protected]>
The restore_freq field in struct cpufreq_policy is only used by
__target_index() in one place and a local variable in that function
may as well be used instead of it, so drop it and modify
__target_index() accordingly.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/cpufreq/cpufreq.c | 10 +++++-----
include/linux/cpufreq.h | 5 -----
2 files changed, 5 insertions(+), 10 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -2112,7 +2112,7 @@ static int __target_intermediate(struct
static int __target_index(struct cpufreq_policy *policy, int index)
{
struct cpufreq_freqs freqs = {.old = policy->cur, .flags = 0};
- unsigned int intermediate_freq = 0;
+ unsigned int restore_freq, intermediate_freq = 0;
unsigned int newfreq = policy->freq_table[index].frequency;
int retval = -EINVAL;
bool notify;
@@ -2120,6 +2120,9 @@ static int __target_index(struct cpufreq
if (newfreq == policy->cur)
return 0;
+ /* Save last value to restore later on errors */
+ restore_freq = policy->cur;
+
notify = !(cpufreq_driver->flags & CPUFREQ_ASYNC_NOTIFICATION);
if (notify) {
/* Handle switching to intermediate frequency */
@@ -2157,7 +2160,7 @@ static int __target_index(struct cpufreq
*/
if (unlikely(retval && intermediate_freq)) {
freqs.old = intermediate_freq;
- freqs.new = policy->restore_freq;
+ freqs.new = restore_freq;
cpufreq_freq_transition_begin(policy, &freqs);
cpufreq_freq_transition_end(policy, &freqs, 0);
}
@@ -2194,9 +2197,6 @@ int __cpufreq_driver_target(struct cpufr
if (target_freq == policy->cur)
return 0;
- /* Save last value to restore later on errors */
- policy->restore_freq = policy->cur;
-
if (!cpufreq_driver->target_index)
return -EINVAL;
Index: linux-pm/include/linux/cpufreq.h
===================================================================
--- linux-pm.orig/include/linux/cpufreq.h
+++ linux-pm/include/linux/cpufreq.h
@@ -65,7 +65,6 @@ struct cpufreq_policy {
unsigned int max; /* in kHz */
unsigned int cur; /* in kHz, only needed if cpufreq
* governors are used */
- unsigned int restore_freq; /* = policy->cur before transition */
unsigned int suspend_freq; /* freq to set during suspend */
unsigned int policy; /* see above */
@@ -308,10 +307,6 @@ struct cpufreq_driver {
/* define one out of two */
int (*setpolicy)(struct cpufreq_policy *policy);
- /*
- * On failure, should always restore frequency to policy->restore_freq
- * (i.e. old freq).
- */
int (*target)(struct cpufreq_policy *policy,
unsigned int target_freq,
unsigned int relation); /* Deprecated */
On 22-10-20, 13:57, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The restore_freq field in struct cpufreq_policy is only used by
> __target_index() in one place and a local variable in that function
> may as well be used instead of it, so drop it and modify
> __target_index() accordingly.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/cpufreq/cpufreq.c | 10 +++++-----
> include/linux/cpufreq.h | 5 -----
> 2 files changed, 5 insertions(+), 10 deletions(-)
Acked-by: Viresh Kumar <[email protected]>
--
viresh