* [PATCH] cpufreq: intel_pstate: Fix policy data management in passive mode @ 2017-03-18 1:36 Rafael J. Wysocki 2017-03-18 15:05 ` [PATCH v2] " Rafael J. Wysocki 0 siblings, 1 reply; 3+ messages in thread From: Rafael J. Wysocki @ 2017-03-18 1:36 UTC (permalink / raw) To: Linux PM; +Cc: LKML, Srinivas Pandruvada From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> The policy->cpuinfo.max_freq and policy->max updates in intel_cpufreq_turbo_update() are excessive as they are done for no good reason and may lead to problems in principle, so drop them. Moreover, fix intel_cpufreq_verify_policy() so that it checks global.no_turbo in addition to global.turbo_disabled when updating policy->cpuinfo.max_freq to make it consistent with intel_pstate_verify_policy(). Fixes: 001c76f05b01 (cpufreq: intel_pstate: Generic governors support) Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- One more fix related to the passive mode. --- drivers/cpufreq/intel_pstate.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) Index: linux-pm/drivers/cpufreq/intel_pstate.c =================================================================== --- linux-pm.orig/drivers/cpufreq/intel_pstate.c +++ linux-pm/drivers/cpufreq/intel_pstate.c @@ -2247,7 +2247,7 @@ static int intel_cpufreq_verify_policy(s struct cpudata *cpu = all_cpu_data[policy->cpu]; update_turbo_state(); - policy->cpuinfo.max_freq = global.turbo_disabled ? + policy->cpuinfo.max_freq = global.no_turbo || global.turbo_disabled ? cpu->pstate.max_freq : cpu->pstate.turbo_freq; cpufreq_verify_within_cpu_limits(policy); @@ -2265,10 +2265,6 @@ static unsigned int intel_cpufreq_turbo_ max_freq = global.no_turbo || global.turbo_disabled ? cpu->pstate.max_freq : cpu->pstate.turbo_freq; - policy->cpuinfo.max_freq = max_freq; - if (policy->max > max_freq) - policy->max = max_freq; - if (target_freq > max_freq) target_freq = max_freq; ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2] cpufreq: intel_pstate: Fix policy data management in passive mode 2017-03-18 1:36 [PATCH] cpufreq: intel_pstate: Fix policy data management in passive mode Rafael J. Wysocki @ 2017-03-18 15:05 ` Rafael J. Wysocki 2017-03-21 0:56 ` [PATCH v3] " Rafael J. Wysocki 0 siblings, 1 reply; 3+ messages in thread From: Rafael J. Wysocki @ 2017-03-18 15:05 UTC (permalink / raw) To: Linux PM; +Cc: LKML, Srinivas Pandruvada From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> The policy->cpuinfo.max_freq and policy->max updates in intel_cpufreq_turbo_update() are excessive as they are done for no good reason and may lead to problems in principle, so they should be dropped. However, after dropping them intel_cpufreq_turbo_update() becomes entirely pointless, because the check made by it is made again down the road in intel_pstate_prepare_request(), so drop the entire intel_cpufreq_turbo_update(). In addition to that, fix intel_cpufreq_verify_policy() so that it checks global.no_turbo in addition to global.turbo_disabled when updating policy->cpuinfo.max_freq to make it consistent with intel_pstate_verify_policy(). Fixes: 001c76f05b01 (cpufreq: intel_pstate: Generic governors support) Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- -> v2: Drop intel_cpufreq_turbo_update() as it is pointless without the excessive updates that need to be dropped. --- drivers/cpufreq/intel_pstate.c | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) Index: linux-pm/drivers/cpufreq/intel_pstate.c =================================================================== --- linux-pm.orig/drivers/cpufreq/intel_pstate.c +++ linux-pm/drivers/cpufreq/intel_pstate.c @@ -2247,7 +2247,7 @@ static int intel_cpufreq_verify_policy(s struct cpudata *cpu = all_cpu_data[policy->cpu]; update_turbo_state(); - policy->cpuinfo.max_freq = global.turbo_disabled ? + policy->cpuinfo.max_freq = global.no_turbo || global.turbo_disabled ? cpu->pstate.max_freq : cpu->pstate.turbo_freq; cpufreq_verify_within_cpu_limits(policy); @@ -2255,26 +2255,6 @@ static int intel_cpufreq_verify_policy(s return 0; } -static unsigned int intel_cpufreq_turbo_update(struct cpudata *cpu, - struct cpufreq_policy *policy, - unsigned int target_freq) -{ - unsigned int max_freq; - - update_turbo_state(); - - max_freq = global.no_turbo || global.turbo_disabled ? - cpu->pstate.max_freq : cpu->pstate.turbo_freq; - policy->cpuinfo.max_freq = max_freq; - if (policy->max > max_freq) - policy->max = max_freq; - - if (target_freq > max_freq) - target_freq = max_freq; - - return target_freq; -} - static int intel_cpufreq_target(struct cpufreq_policy *policy, unsigned int target_freq, unsigned int relation) @@ -2284,7 +2264,7 @@ static int intel_cpufreq_target(struct c int target_pstate; freqs.old = policy->cur; - freqs.new = intel_cpufreq_turbo_update(cpu, policy, target_freq); + freqs.new = target_freq; cpufreq_freq_transition_begin(policy, &freqs); switch (relation) { @@ -2316,7 +2296,6 @@ static unsigned int intel_cpufreq_fast_s struct cpudata *cpu = all_cpu_data[policy->cpu]; int target_pstate; - target_freq = intel_cpufreq_turbo_update(cpu, policy, target_freq); target_pstate = DIV_ROUND_UP(target_freq, cpu->pstate.scaling); target_pstate = intel_pstate_prepare_request(cpu, target_pstate); intel_pstate_update_pstate(cpu, target_pstate); ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v3] cpufreq: intel_pstate: Fix policy data management in passive mode 2017-03-18 15:05 ` [PATCH v2] " Rafael J. Wysocki @ 2017-03-21 0:56 ` Rafael J. Wysocki 0 siblings, 0 replies; 3+ messages in thread From: Rafael J. Wysocki @ 2017-03-21 0:56 UTC (permalink / raw) To: Linux PM; +Cc: LKML, Srinivas Pandruvada From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> The policy->cpuinfo.max_freq and policy->max updates in intel_cpufreq_turbo_update() are excessive as they are done for no good reason and may lead to problems in principle, so they should be dropped. However, after dropping them intel_cpufreq_turbo_update() becomes almost entirely pointless, because the check made by it is made again down the road in intel_pstate_prepare_request(). The only thing in it that still needs to be done is the call to update_turbo_state(), so drop intel_cpufreq_turbo_update() altogether and make its callers invoke update_turbo_state() directly instead of it. In addition to that, fix intel_cpufreq_verify_policy() so that it checks global.no_turbo in addition to global.turbo_disabled when updating policy->cpuinfo.max_freq to make it consistent with intel_pstate_verify_policy(). Fixes: 001c76f05b01 (cpufreq: intel_pstate: Generic governors support) Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- v2 -> v3: Make the previous callers of intel_cpufreq_turbo_update() invoke update_turbo_state() directly (since it was called by the former). -> v2: Drop intel_cpufreq_turbo_update() as it is pointless without the excessive updates that need to be dropped. --- drivers/cpufreq/intel_pstate.c | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) Index: linux-pm/drivers/cpufreq/intel_pstate.c =================================================================== --- linux-pm.orig/drivers/cpufreq/intel_pstate.c +++ linux-pm/drivers/cpufreq/intel_pstate.c @@ -2247,7 +2247,7 @@ static int intel_cpufreq_verify_policy(s struct cpudata *cpu = all_cpu_data[policy->cpu]; update_turbo_state(); - policy->cpuinfo.max_freq = global.turbo_disabled ? + policy->cpuinfo.max_freq = global.no_turbo || global.turbo_disabled ? cpu->pstate.max_freq : cpu->pstate.turbo_freq; cpufreq_verify_within_cpu_limits(policy); @@ -2255,26 +2255,6 @@ static int intel_cpufreq_verify_policy(s return 0; } -static unsigned int intel_cpufreq_turbo_update(struct cpudata *cpu, - struct cpufreq_policy *policy, - unsigned int target_freq) -{ - unsigned int max_freq; - - update_turbo_state(); - - max_freq = global.no_turbo || global.turbo_disabled ? - cpu->pstate.max_freq : cpu->pstate.turbo_freq; - policy->cpuinfo.max_freq = max_freq; - if (policy->max > max_freq) - policy->max = max_freq; - - if (target_freq > max_freq) - target_freq = max_freq; - - return target_freq; -} - static int intel_cpufreq_target(struct cpufreq_policy *policy, unsigned int target_freq, unsigned int relation) @@ -2283,8 +2263,10 @@ static int intel_cpufreq_target(struct c struct cpufreq_freqs freqs; int target_pstate; + update_turbo_state(); + freqs.old = policy->cur; - freqs.new = intel_cpufreq_turbo_update(cpu, policy, target_freq); + freqs.new = target_freq; cpufreq_freq_transition_begin(policy, &freqs); switch (relation) { @@ -2316,7 +2298,8 @@ static unsigned int intel_cpufreq_fast_s struct cpudata *cpu = all_cpu_data[policy->cpu]; int target_pstate; - target_freq = intel_cpufreq_turbo_update(cpu, policy, target_freq); + update_turbo_state(); + target_pstate = DIV_ROUND_UP(target_freq, cpu->pstate.scaling); target_pstate = intel_pstate_prepare_request(cpu, target_pstate); intel_pstate_update_pstate(cpu, target_pstate); ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-03-21 1:08 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-18 1:36 [PATCH] cpufreq: intel_pstate: Fix policy data management in passive mode Rafael J. Wysocki 2017-03-18 15:05 ` [PATCH v2] " Rafael J. Wysocki 2017-03-21 0:56 ` [PATCH v3] " Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®