* [RFC PATCH v3 0/2] add SW BOOST support for CPPC @ 2020-05-19 11:41 Xiongfeng Wang 2020-05-19 11:41 ` [RFC PATCH v3 1/2] cpufreq: change '.set_boost' to act on only one policy Xiongfeng Wang 2020-05-19 11:41 ` [RFC PATCH v3 2/2] CPPC: add support for SW BOOST Xiongfeng Wang 0 siblings, 2 replies; 9+ messages in thread From: Xiongfeng Wang @ 2020-05-19 11:41 UTC (permalink / raw) To: rjw, viresh.kumar, Souvik.Chakravarty, Thanu.Rangarajan Cc: Sudeep.Holla, guohanjun, john.garry, jonathan.cameron, linux-pm, linux-kernel, wangxiongfeng2 ACPI spec 6.2 section 8.4.7.1 provide the following two CPC registers. "Highest performance is the absolute maximum performance an individual processor may reach, assuming ideal conditions. This performance level may not be sustainable for long durations, and may only be achievable if other platform components are in a specific state; for example, it may require other processors be in an idle state. Nominal Performance is the maximum sustained performance level of the processor, assuming ideal operating conditions. In absence of an external constraint (power, thermal, etc.) this is the performance level the platform is expected to be able to maintain continuously. All processors are expected to be able to sustain their nominal performance state simultaneously." We can use Highest Performance as the max performance in boost mode and Nomial Performance as the max performance in non-boost mode. If the Highest Performance is greater than the Nominal Performance, we assume SW BOOST is supported. Xiongfeng Wang (2): cpufreq: change '.set_boost' to act on only one policy CPPC: add support for SW BOOST drivers/cpufreq/acpi-cpufreq.c | 4 ++-- drivers/cpufreq/cppc_cpufreq.c | 39 +++++++++++++++++++++++++++++-- drivers/cpufreq/cpufreq.c | 53 +++++++++++++++++++++--------------------- include/linux/cpufreq.h | 2 +- 4 files changed, 67 insertions(+), 31 deletions(-) -- 1.7.12.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC PATCH v3 1/2] cpufreq: change '.set_boost' to act on only one policy 2020-05-19 11:41 [RFC PATCH v3 0/2] add SW BOOST support for CPPC Xiongfeng Wang @ 2020-05-19 11:41 ` Xiongfeng Wang 2020-05-19 11:57 ` Xiongfeng Wang 2020-05-20 4:59 ` Viresh Kumar 2020-05-19 11:41 ` [RFC PATCH v3 2/2] CPPC: add support for SW BOOST Xiongfeng Wang 1 sibling, 2 replies; 9+ messages in thread From: Xiongfeng Wang @ 2020-05-19 11:41 UTC (permalink / raw) To: rjw, viresh.kumar, Souvik.Chakravarty, Thanu.Rangarajan Cc: Sudeep.Holla, guohanjun, john.garry, jonathan.cameron, linux-pm, linux-kernel, wangxiongfeng2 Macro 'for_each_active_policy()' is defined internally. To avoid some cpufreq driver needing this macro to iterate over all the policies in '.set_boost' callback, we redefine '.set_boost' to act on only one policy and pass the policy as an argument. 'cpufreq_boost_trigger_state()' iterate over all the policies to set boost for the system. This is preparation for adding SW BOOST support for CPPC. Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> --- drivers/cpufreq/acpi-cpufreq.c | 4 ++-- drivers/cpufreq/cpufreq.c | 53 +++++++++++++++++++++--------------------- include/linux/cpufreq.h | 2 +- 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c index 289e8ce..b0a9eb5 100644 --- a/drivers/cpufreq/acpi-cpufreq.c +++ b/drivers/cpufreq/acpi-cpufreq.c @@ -126,7 +126,7 @@ static void boost_set_msr_each(void *p_en) boost_set_msr(enable); } -static int set_boost(int val) +static int set_boost(struct cpufreq_policy *policy, int val) { get_online_cpus(); on_each_cpu(boost_set_msr_each, (void *)(long)val, 1); @@ -162,7 +162,7 @@ static ssize_t store_cpb(struct cpufreq_policy *policy, const char *buf, if (ret || val > 1) return -EINVAL; - set_boost(val); + set_boost(policy, val); return count; } diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index d03f250..d0d86b1 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2532,34 +2532,29 @@ void cpufreq_update_limits(unsigned int cpu) /********************************************************************* * BOOST * *********************************************************************/ -static int cpufreq_boost_set_sw(int state) +static int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state) { - struct cpufreq_policy *policy; - - for_each_active_policy(policy) { - int ret; - - if (!policy->freq_table) - return -ENXIO; + int ret; - ret = cpufreq_frequency_table_cpuinfo(policy, - policy->freq_table); - if (ret) { - pr_err("%s: Policy frequency update failed\n", - __func__); - return ret; - } + if (!policy->freq_table) + return -ENXIO; - ret = freq_qos_update_request(policy->max_freq_req, policy->max); - if (ret < 0) - return ret; + ret = cpufreq_frequency_table_cpuinfo(policy, policy->freq_table); + if (ret) { + pr_err("%s: Policy frequency update failed\n", __func__); + return ret; } + ret = freq_qos_update_request(policy->max_freq_req, policy->max); + if (ret < 0) + return ret; + return 0; } int cpufreq_boost_trigger_state(int state) { + struct cpufreq_policy *policy; unsigned long flags; int ret = 0; @@ -2570,16 +2565,22 @@ int cpufreq_boost_trigger_state(int state) cpufreq_driver->boost_enabled = state; write_unlock_irqrestore(&cpufreq_driver_lock, flags); - ret = cpufreq_driver->set_boost(state); - if (ret) { - write_lock_irqsave(&cpufreq_driver_lock, flags); - cpufreq_driver->boost_enabled = !state; - write_unlock_irqrestore(&cpufreq_driver_lock, flags); - - pr_err("%s: Cannot %s BOOST\n", - __func__, state ? "enable" : "disable"); + for_each_active_policy(policy) { + ret = cpufreq_driver->set_boost(policy, state); + if (ret) + goto err_reset_state; } + return 0; + +err_reset_state: + write_lock_irqsave(&cpufreq_driver_lock, flags); + cpufreq_driver->boost_enabled = !state; + write_unlock_irqrestore(&cpufreq_driver_lock, flags); + + pr_err("%s: Cannot %s BOOST\n", + __func__, state ? "enable" : "disable"); + return ret; } diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 67d5950..3494f67 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -367,7 +367,7 @@ struct cpufreq_driver { /* platform specific boost support code */ bool boost_enabled; - int (*set_boost)(int state); + int (*set_boost)(struct cpufreq_policy *policy, int state); }; /* flags */ -- 1.7.12.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC PATCH v3 1/2] cpufreq: change '.set_boost' to act on only one policy 2020-05-19 11:41 ` [RFC PATCH v3 1/2] cpufreq: change '.set_boost' to act on only one policy Xiongfeng Wang @ 2020-05-19 11:57 ` Xiongfeng Wang 2020-05-20 4:59 ` Viresh Kumar 1 sibling, 0 replies; 9+ messages in thread From: Xiongfeng Wang @ 2020-05-19 11:57 UTC (permalink / raw) To: rjw, viresh.kumar, Souvik.Chakravarty, Thanu.Rangarajan Cc: Sudeep.Holla, guohanjun, john.garry, jonathan.cameron, linux-pm, linux-kernel On 2020/5/19 19:41, Xiongfeng Wang wrote: > Macro 'for_each_active_policy()' is defined internally. To avoid some > cpufreq driver needing this macro to iterate over all the policies in > '.set_boost' callback, we redefine '.set_boost' to act on only one > policy and pass the policy as an argument. > 'cpufreq_boost_trigger_state()' iterate over all the policies to set > boost for the system. This is preparation for adding SW BOOST support > for CPPC. > > Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> > --- > drivers/cpufreq/acpi-cpufreq.c | 4 ++-- > drivers/cpufreq/cpufreq.c | 53 +++++++++++++++++++++--------------------- > include/linux/cpufreq.h | 2 +- > 3 files changed, 30 insertions(+), 29 deletions(-) > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > index 289e8ce..b0a9eb5 100644 > --- a/drivers/cpufreq/acpi-cpufreq.c > +++ b/drivers/cpufreq/acpi-cpufreq.c > @@ -126,7 +126,7 @@ static void boost_set_msr_each(void *p_en) > boost_set_msr(enable); > } > > -static int set_boost(int val) > +static int set_boost(struct cpufreq_policy *policy, int val) > { > get_online_cpus(); > on_each_cpu(boost_set_msr_each, (void *)(long)val, 1); > @@ -162,7 +162,7 @@ static ssize_t store_cpb(struct cpufreq_policy *policy, const char *buf, > if (ret || val > 1) > return -EINVAL; > > - set_boost(val); > + set_boost(policy, val); > > return count; > } My original thought is as below. Since '/sys/devices/system/cpu/cpufreq/boost' can be used to set boost for the system. The file 'cpb' below each policy can be used to set boost for the policy. But this will change the existing logic. --- a/drivers/cpufreq/acpi-cpufreq.c +++ b/drivers/cpufreq/acpi-cpufreq.c @@ -126,12 +126,14 @@ static void boost_set_msr_each(void *p_en) boost_set_msr(enable); } -static int set_boost(int val) +static int set_boost(struct cpufreq_policy *policy, int val) { get_online_cpus(); - on_each_cpu(boost_set_msr_each, (void *)(long)val, 1); + on_each_cpu_mask(policy->cpus, boost_set_msr_each, + (void *)(long)val, 1); put_online_cpus(); - pr_debug("Core Boosting %sabled.\n", val ? "en" : "dis"); + pr_debug("CPU %*pbl: Core Boosting %sabled.\n", + cpumask_pr_args(policy->cpus), val ? "en" : "dis"); return 0; } @@ -162,7 +164,7 @@ static ssize_t store_cpb(struct cpufreq_policy *policy, const char *buf, if (ret || val > 1) return -EINVAL; - set_boost(val); + set_boost(policy, val); return count; } > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index d03f250..d0d86b1 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > > int cpufreq_boost_trigger_state(int state) > { > + struct cpufreq_policy *policy; > unsigned long flags; > int ret = 0; > > @@ -2570,16 +2565,22 @@ int cpufreq_boost_trigger_state(int state) > cpufreq_driver->boost_enabled = state; > write_unlock_irqrestore(&cpufreq_driver_lock, flags); > > - ret = cpufreq_driver->set_boost(state); > - if (ret) { > - write_lock_irqsave(&cpufreq_driver_lock, flags); > - cpufreq_driver->boost_enabled = !state; > - write_unlock_irqrestore(&cpufreq_driver_lock, flags); > - > - pr_err("%s: Cannot %s BOOST\n", > - __func__, state ? "enable" : "disable"); > + for_each_active_policy(policy) { > + ret = cpufreq_driver->set_boost(policy, state); > + if (ret) > + goto err_reset_state; > } > > + return 0; > + > +err_reset_state: > + write_lock_irqsave(&cpufreq_driver_lock, flags); > + cpufreq_driver->boost_enabled = !state; > + write_unlock_irqrestore(&cpufreq_driver_lock, flags); > + > + pr_err("%s: Cannot %s BOOST\n", > + __func__, state ? "enable" : "disable"); > + > return ret; > } Is it better to set 'boost_enabled' after set boost successfully rather than setting it in the begining and resetting it if setting boost failed. Thanks, Xiongfeng > > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > index 67d5950..3494f67 100644 > --- a/include/linux/cpufreq.h > +++ b/include/linux/cpufreq.h > @@ -367,7 +367,7 @@ struct cpufreq_driver { > > /* platform specific boost support code */ > bool boost_enabled; > - int (*set_boost)(int state); > + int (*set_boost)(struct cpufreq_policy *policy, int state); > }; > > /* flags */ > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH v3 1/2] cpufreq: change '.set_boost' to act on only one policy 2020-05-19 11:41 ` [RFC PATCH v3 1/2] cpufreq: change '.set_boost' to act on only one policy Xiongfeng Wang 2020-05-19 11:57 ` Xiongfeng Wang @ 2020-05-20 4:59 ` Viresh Kumar 2020-05-20 10:36 ` Rafael J. Wysocki 1 sibling, 1 reply; 9+ messages in thread From: Viresh Kumar @ 2020-05-20 4:59 UTC (permalink / raw) To: Xiongfeng Wang Cc: rjw, Souvik.Chakravarty, Thanu.Rangarajan, Sudeep.Holla, guohanjun, john.garry, jonathan.cameron, linux-pm, linux-kernel On 19-05-20, 19:41, Xiongfeng Wang wrote: > Macro 'for_each_active_policy()' is defined internally. To avoid some > cpufreq driver needing this macro to iterate over all the policies in > '.set_boost' callback, we redefine '.set_boost' to act on only one > policy and pass the policy as an argument. > 'cpufreq_boost_trigger_state()' iterate over all the policies to set > boost for the system. This is preparation for adding SW BOOST support > for CPPC. > > Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> > --- > drivers/cpufreq/acpi-cpufreq.c | 4 ++-- > drivers/cpufreq/cpufreq.c | 53 +++++++++++++++++++++--------------------- > include/linux/cpufreq.h | 2 +- > 3 files changed, 30 insertions(+), 29 deletions(-) > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > index 289e8ce..b0a9eb5 100644 > --- a/drivers/cpufreq/acpi-cpufreq.c > +++ b/drivers/cpufreq/acpi-cpufreq.c > @@ -126,7 +126,7 @@ static void boost_set_msr_each(void *p_en) > boost_set_msr(enable); > } > > -static int set_boost(int val) > +static int set_boost(struct cpufreq_policy *policy, int val) > { > get_online_cpus(); > on_each_cpu(boost_set_msr_each, (void *)(long)val, 1); I think (Rafael can confirm), that you need to update this as well. You don't need to run for each cpu now, but for each CPU in the policy. > @@ -162,7 +162,7 @@ static ssize_t store_cpb(struct cpufreq_policy *policy, const char *buf, > if (ret || val > 1) > return -EINVAL; > > - set_boost(val); > + set_boost(policy, val); > > return count; > } > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index d03f250..d0d86b1 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -2532,34 +2532,29 @@ void cpufreq_update_limits(unsigned int cpu) > /********************************************************************* > * BOOST * > *********************************************************************/ > -static int cpufreq_boost_set_sw(int state) > +static int cpufreq_boost_set_sw(struct cpufreq_policy *policy, int state) > { > - struct cpufreq_policy *policy; > - > - for_each_active_policy(policy) { > - int ret; > - > - if (!policy->freq_table) > - return -ENXIO; > + int ret; > > - ret = cpufreq_frequency_table_cpuinfo(policy, > - policy->freq_table); > - if (ret) { > - pr_err("%s: Policy frequency update failed\n", > - __func__); > - return ret; > - } > + if (!policy->freq_table) > + return -ENXIO; > > - ret = freq_qos_update_request(policy->max_freq_req, policy->max); > - if (ret < 0) > - return ret; > + ret = cpufreq_frequency_table_cpuinfo(policy, policy->freq_table); > + if (ret) { > + pr_err("%s: Policy frequency update failed\n", __func__); > + return ret; > } > > + ret = freq_qos_update_request(policy->max_freq_req, policy->max); > + if (ret < 0) > + return ret; > + > return 0; > } > > int cpufreq_boost_trigger_state(int state) > { > + struct cpufreq_policy *policy; > unsigned long flags; > int ret = 0; > > @@ -2570,16 +2565,22 @@ int cpufreq_boost_trigger_state(int state) > cpufreq_driver->boost_enabled = state; > write_unlock_irqrestore(&cpufreq_driver_lock, flags); > > - ret = cpufreq_driver->set_boost(state); > - if (ret) { > - write_lock_irqsave(&cpufreq_driver_lock, flags); > - cpufreq_driver->boost_enabled = !state; > - write_unlock_irqrestore(&cpufreq_driver_lock, flags); > - > - pr_err("%s: Cannot %s BOOST\n", > - __func__, state ? "enable" : "disable"); > + for_each_active_policy(policy) { > + ret = cpufreq_driver->set_boost(policy, state); > + if (ret) > + goto err_reset_state; > } > > + return 0; > + > +err_reset_state: > + write_lock_irqsave(&cpufreq_driver_lock, flags); > + cpufreq_driver->boost_enabled = !state; > + write_unlock_irqrestore(&cpufreq_driver_lock, flags); > + > + pr_err("%s: Cannot %s BOOST\n", > + __func__, state ? "enable" : "disable"); > + > return ret; > } > > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > index 67d5950..3494f67 100644 > --- a/include/linux/cpufreq.h > +++ b/include/linux/cpufreq.h > @@ -367,7 +367,7 @@ struct cpufreq_driver { > > /* platform specific boost support code */ > bool boost_enabled; > - int (*set_boost)(int state); > + int (*set_boost)(struct cpufreq_policy *policy, int state); > }; > > /* flags */ > -- > 1.7.12.4 -- viresh ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH v3 1/2] cpufreq: change '.set_boost' to act on only one policy 2020-05-20 4:59 ` Viresh Kumar @ 2020-05-20 10:36 ` Rafael J. Wysocki 2020-05-20 10:41 ` Viresh Kumar 0 siblings, 1 reply; 9+ messages in thread From: Rafael J. Wysocki @ 2020-05-20 10:36 UTC (permalink / raw) To: Viresh Kumar Cc: Xiongfeng Wang, Rafael J. Wysocki, Souvik Chakravarty, Thanu.Rangarajan, Sudeep Holla, Hanjun Guo, John Garry, Jonathan Cameron, Linux PM, Linux Kernel Mailing List On Wed, May 20, 2020 at 6:59 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 19-05-20, 19:41, Xiongfeng Wang wrote: > > Macro 'for_each_active_policy()' is defined internally. To avoid some > > cpufreq driver needing this macro to iterate over all the policies in > > '.set_boost' callback, we redefine '.set_boost' to act on only one > > policy and pass the policy as an argument. > > 'cpufreq_boost_trigger_state()' iterate over all the policies to set > > boost for the system. This is preparation for adding SW BOOST support > > for CPPC. > > > > Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> > > --- > > drivers/cpufreq/acpi-cpufreq.c | 4 ++-- > > drivers/cpufreq/cpufreq.c | 53 +++++++++++++++++++++--------------------- > > include/linux/cpufreq.h | 2 +- > > 3 files changed, 30 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > > index 289e8ce..b0a9eb5 100644 > > --- a/drivers/cpufreq/acpi-cpufreq.c > > +++ b/drivers/cpufreq/acpi-cpufreq.c > > @@ -126,7 +126,7 @@ static void boost_set_msr_each(void *p_en) > > boost_set_msr(enable); > > } > > > > -static int set_boost(int val) > > +static int set_boost(struct cpufreq_policy *policy, int val) > > { > > get_online_cpus(); > > on_each_cpu(boost_set_msr_each, (void *)(long)val, 1); > > I think (Rafael can confirm), that you need to update this as well. You don't > need to run for each cpu now, but for each CPU in the policy. Right, the caller will iterate over policies. Accordingly, the CPU hotplug locking needs to go to the caller too. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH v3 1/2] cpufreq: change '.set_boost' to act on only one policy 2020-05-20 10:36 ` Rafael J. Wysocki @ 2020-05-20 10:41 ` Viresh Kumar 0 siblings, 0 replies; 9+ messages in thread From: Viresh Kumar @ 2020-05-20 10:41 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Xiongfeng Wang, Rafael J. Wysocki, Souvik Chakravarty, Thanu.Rangarajan, Sudeep Holla, Hanjun Guo, John Garry, Jonathan Cameron, Linux PM, Linux Kernel Mailing List On 20-05-20, 12:36, Rafael J. Wysocki wrote: > On Wed, May 20, 2020 at 6:59 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 19-05-20, 19:41, Xiongfeng Wang wrote: > > > Macro 'for_each_active_policy()' is defined internally. To avoid some > > > cpufreq driver needing this macro to iterate over all the policies in > > > '.set_boost' callback, we redefine '.set_boost' to act on only one > > > policy and pass the policy as an argument. > > > 'cpufreq_boost_trigger_state()' iterate over all the policies to set > > > boost for the system. This is preparation for adding SW BOOST support > > > for CPPC. > > > > > > Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> > > > --- > > > drivers/cpufreq/acpi-cpufreq.c | 4 ++-- > > > drivers/cpufreq/cpufreq.c | 53 +++++++++++++++++++++--------------------- > > > include/linux/cpufreq.h | 2 +- > > > 3 files changed, 30 insertions(+), 29 deletions(-) > > > > > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > > > index 289e8ce..b0a9eb5 100644 > > > --- a/drivers/cpufreq/acpi-cpufreq.c > > > +++ b/drivers/cpufreq/acpi-cpufreq.c > > > @@ -126,7 +126,7 @@ static void boost_set_msr_each(void *p_en) > > > boost_set_msr(enable); > > > } > > > > > > -static int set_boost(int val) > > > +static int set_boost(struct cpufreq_policy *policy, int val) > > > { > > > get_online_cpus(); > > > on_each_cpu(boost_set_msr_each, (void *)(long)val, 1); > > > > I think (Rafael can confirm), that you need to update this as well. You don't > > need to run for each cpu now, but for each CPU in the policy. > > Right, the caller will iterate over policies. > > Accordingly, the CPU hotplug locking needs to go to the caller too. Hmm, why is that required ? Can't we call boost_set_msr_each() for all CPUs of a policy under the locks ? And then let the next call take the lock again ? I thought we don't want a CPU to disappear while we are trying to run boost_set_msr_each() for it (or miss one that just got added) and that should work with the locks being there in this routine. -- viresh ^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC PATCH v3 2/2] CPPC: add support for SW BOOST 2020-05-19 11:41 [RFC PATCH v3 0/2] add SW BOOST support for CPPC Xiongfeng Wang 2020-05-19 11:41 ` [RFC PATCH v3 1/2] cpufreq: change '.set_boost' to act on only one policy Xiongfeng Wang @ 2020-05-19 11:41 ` Xiongfeng Wang 2020-05-20 5:00 ` Viresh Kumar 1 sibling, 1 reply; 9+ messages in thread From: Xiongfeng Wang @ 2020-05-19 11:41 UTC (permalink / raw) To: rjw, viresh.kumar, Souvik.Chakravarty, Thanu.Rangarajan Cc: Sudeep.Holla, guohanjun, john.garry, jonathan.cameron, linux-pm, linux-kernel, wangxiongfeng2 To add SW BOOST support for CPPC, we need to get the max frequency of boost mode and non-boost mode. ACPI spec 6.2 section 8.4.7.1 describe the following two CPC registers. "Highest performance is the absolute maximum performance an individual processor may reach, assuming ideal conditions. This performance level may not be sustainable for long durations, and may only be achievable if other platform components are in a specific state; for example, it may require other processors be in an idle state. Nominal Performance is the maximum sustained performance level of the processor, assuming ideal operating conditions. In absence of an external constraint (power, thermal, etc.) this is the performance level the platform is expected to be able to maintain continuously. All processors are expected to be able to sustain their nominal performance state simultaneously." To add SW BOOST support for CPPC, we can use Highest Performance as the max performance in boost mode and Nominal Performance as the max performance in non-boost mode. If the Highest Performance is greater than the Nominal Performance, we assume SW BOOST is supported. The current CPPC driver does not support SW BOOST and use 'Highest Performance' as the max performance the CPU can achieve. 'Nominal Performance' is used to convert 'performance' to 'frequency'. That means, if firmware enable boost and provide a value for Highest Performance which is greater than Nominal Performance, boost feature is enabled by default. Because SW BOOST is disabled by default, so, after this patch, boost feature is disabled by default even if boost is enabled by firmware. Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> --- drivers/cpufreq/cppc_cpufreq.c | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index bda0b24..792ed9e 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -37,6 +37,7 @@ * requested etc. */ static struct cppc_cpudata **all_cpu_data; +static bool boost_supported; struct cppc_workaround_oem_info { char oem_id[ACPI_OEM_ID_SIZE + 1]; @@ -310,7 +311,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) * Section 8.4.7.1.1.5 of ACPI 6.1 spec) */ policy->min = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.lowest_nonlinear_perf); - policy->max = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.highest_perf); + policy->max = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.nominal_perf); /* * Set cpuinfo.min_freq to Lowest to make the full range of performance @@ -318,7 +319,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) * nonlinear perf */ policy->cpuinfo.min_freq = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.lowest_perf); - policy->cpuinfo.max_freq = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.highest_perf); + policy->cpuinfo.max_freq = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.nominal_perf); policy->transition_delay_us = cppc_cpufreq_get_transition_delay_us(cpu_num); policy->shared_type = cpu->shared_type; @@ -343,6 +344,13 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) cpu->cur_policy = policy; + /* + * If 'highest_perf' is greater than 'nominal_perf', we assume CPU Boost + * is supported. + */ + if (cpu->perf_caps.highest_perf > cpu->perf_caps.nominal_perf) + boost_supported = true; + /* Set policy->cur to max now. The governors will adjust later. */ policy->cur = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.highest_perf); @@ -410,6 +418,32 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum) return cppc_get_rate_from_fbctrs(cpu, fb_ctrs_t0, fb_ctrs_t1); } +static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state) +{ + struct cppc_cpudata *cpudata; + int ret = 0; + + if (!boost_supported) { + pr_err("BOOST not supported by CPU or firmware\n"); + return -EINVAL; + } + + cpudata = all_cpu_data[policy->cpu]; + if (state) + policy->max = cppc_cpufreq_perf_to_khz(cpudata, + cpudata->perf_caps.highest_perf); + else + policy->max = cppc_cpufreq_perf_to_khz(cpudata, + cpudata->perf_caps.nominal_perf); + policy->cpuinfo.max_freq = policy->max; + + ret = freq_qos_update_request(policy->max_freq_req, policy->max); + if (ret < 0) + return ret; + + return 0; +} + static struct cpufreq_driver cppc_cpufreq_driver = { .flags = CPUFREQ_CONST_LOOPS, .verify = cppc_verify_policy, @@ -417,6 +451,7 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum) .get = cppc_cpufreq_get_rate, .init = cppc_cpufreq_cpu_init, .stop_cpu = cppc_cpufreq_stop_cpu, + .set_boost = cppc_cpufreq_set_boost, .name = "cppc_cpufreq", }; -- 1.7.12.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC PATCH v3 2/2] CPPC: add support for SW BOOST 2020-05-19 11:41 ` [RFC PATCH v3 2/2] CPPC: add support for SW BOOST Xiongfeng Wang @ 2020-05-20 5:00 ` Viresh Kumar 2020-05-21 6:25 ` Xiongfeng Wang 0 siblings, 1 reply; 9+ messages in thread From: Viresh Kumar @ 2020-05-20 5:00 UTC (permalink / raw) To: Xiongfeng Wang Cc: rjw, Souvik.Chakravarty, Thanu.Rangarajan, Sudeep.Holla, guohanjun, john.garry, jonathan.cameron, linux-pm, linux-kernel On 19-05-20, 19:41, Xiongfeng Wang wrote: > To add SW BOOST support for CPPC, we need to get the max frequency of > boost mode and non-boost mode. ACPI spec 6.2 section 8.4.7.1 describe > the following two CPC registers. > > "Highest performance is the absolute maximum performance an individual > processor may reach, assuming ideal conditions. This performance level > may not be sustainable for long durations, and may only be achievable if > other platform components are in a specific state; for example, it may > require other processors be in an idle state. > > Nominal Performance is the maximum sustained performance level of the > processor, assuming ideal operating conditions. In absence of an > external constraint (power, thermal, etc.) this is the performance level > the platform is expected to be able to maintain continuously. All > processors are expected to be able to sustain their nominal performance > state simultaneously." > > To add SW BOOST support for CPPC, we can use Highest Performance as the > max performance in boost mode and Nominal Performance as the max > performance in non-boost mode. If the Highest Performance is greater > than the Nominal Performance, we assume SW BOOST is supported. > > The current CPPC driver does not support SW BOOST and use 'Highest > Performance' as the max performance the CPU can achieve. 'Nominal > Performance' is used to convert 'performance' to 'frequency'. That > means, if firmware enable boost and provide a value for Highest > Performance which is greater than Nominal Performance, boost feature is > enabled by default. > > Because SW BOOST is disabled by default, so, after this patch, boost > feature is disabled by default even if boost is enabled by firmware. > > Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> > --- > drivers/cpufreq/cppc_cpufreq.c | 39 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 37 insertions(+), 2 deletions(-) > > diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c > index bda0b24..792ed9e 100644 > --- a/drivers/cpufreq/cppc_cpufreq.c > +++ b/drivers/cpufreq/cppc_cpufreq.c > @@ -37,6 +37,7 @@ > * requested etc. > */ > static struct cppc_cpudata **all_cpu_data; > +static bool boost_supported; > > struct cppc_workaround_oem_info { > char oem_id[ACPI_OEM_ID_SIZE + 1]; > @@ -310,7 +311,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) > * Section 8.4.7.1.1.5 of ACPI 6.1 spec) > */ > policy->min = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.lowest_nonlinear_perf); > - policy->max = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.highest_perf); > + policy->max = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.nominal_perf); > > /* > * Set cpuinfo.min_freq to Lowest to make the full range of performance > @@ -318,7 +319,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) > * nonlinear perf > */ > policy->cpuinfo.min_freq = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.lowest_perf); > - policy->cpuinfo.max_freq = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.highest_perf); > + policy->cpuinfo.max_freq = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.nominal_perf); > > policy->transition_delay_us = cppc_cpufreq_get_transition_delay_us(cpu_num); > policy->shared_type = cpu->shared_type; > @@ -343,6 +344,13 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) > > cpu->cur_policy = policy; > > + /* > + * If 'highest_perf' is greater than 'nominal_perf', we assume CPU Boost > + * is supported. > + */ > + if (cpu->perf_caps.highest_perf > cpu->perf_caps.nominal_perf) > + boost_supported = true; > + > /* Set policy->cur to max now. The governors will adjust later. */ > policy->cur = cppc_cpufreq_perf_to_khz(cpu, > cpu->perf_caps.highest_perf); > @@ -410,6 +418,32 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum) > return cppc_get_rate_from_fbctrs(cpu, fb_ctrs_t0, fb_ctrs_t1); > } > > +static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state) > +{ > + struct cppc_cpudata *cpudata; > + int ret = 0; No need to initialize this. > + > + if (!boost_supported) { > + pr_err("BOOST not supported by CPU or firmware\n"); > + return -EINVAL; > + } > + > + cpudata = all_cpu_data[policy->cpu]; > + if (state) > + policy->max = cppc_cpufreq_perf_to_khz(cpudata, > + cpudata->perf_caps.highest_perf); > + else > + policy->max = cppc_cpufreq_perf_to_khz(cpudata, > + cpudata->perf_caps.nominal_perf); > + policy->cpuinfo.max_freq = policy->max; > + > + ret = freq_qos_update_request(policy->max_freq_req, policy->max); > + if (ret < 0) > + return ret; > + > + return 0; > +} > + > static struct cpufreq_driver cppc_cpufreq_driver = { > .flags = CPUFREQ_CONST_LOOPS, > .verify = cppc_verify_policy, > @@ -417,6 +451,7 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum) > .get = cppc_cpufreq_get_rate, > .init = cppc_cpufreq_cpu_init, > .stop_cpu = cppc_cpufreq_stop_cpu, > + .set_boost = cppc_cpufreq_set_boost, > .name = "cppc_cpufreq", > }; > > -- > 1.7.12.4 -- viresh ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH v3 2/2] CPPC: add support for SW BOOST 2020-05-20 5:00 ` Viresh Kumar @ 2020-05-21 6:25 ` Xiongfeng Wang 0 siblings, 0 replies; 9+ messages in thread From: Xiongfeng Wang @ 2020-05-21 6:25 UTC (permalink / raw) To: Viresh Kumar Cc: rjw, Souvik.Chakravarty, Thanu.Rangarajan, Sudeep.Holla, guohanjun, john.garry, jonathan.cameron, linux-pm, linux-kernel Hi Viresh, On 2020/5/20 13:00, Viresh Kumar wrote: > On 19-05-20, 19:41, Xiongfeng Wang wrote: >> To add SW BOOST support for CPPC, we need to get the max frequency of >> boost mode and non-boost mode. ACPI spec 6.2 section 8.4.7.1 describe >> the following two CPC registers. >> >> "Highest performance is the absolute maximum performance an individual >> processor may reach, assuming ideal conditions. This performance level >> may not be sustainable for long durations, and may only be achievable if >> other platform components are in a specific state; for example, it may >> require other processors be in an idle state. >> >> Nominal Performance is the maximum sustained performance level of the >> processor, assuming ideal operating conditions. In absence of an >> external constraint (power, thermal, etc.) this is the performance level >> the platform is expected to be able to maintain continuously. All >> processors are expected to be able to sustain their nominal performance >> state simultaneously." >> >> To add SW BOOST support for CPPC, we can use Highest Performance as the >> max performance in boost mode and Nominal Performance as the max >> performance in non-boost mode. If the Highest Performance is greater >> than the Nominal Performance, we assume SW BOOST is supported. >> >> The current CPPC driver does not support SW BOOST and use 'Highest >> Performance' as the max performance the CPU can achieve. 'Nominal >> Performance' is used to convert 'performance' to 'frequency'. That >> means, if firmware enable boost and provide a value for Highest >> Performance which is greater than Nominal Performance, boost feature is >> enabled by default. >> >> Because SW BOOST is disabled by default, so, after this patch, boost >> feature is disabled by default even if boost is enabled by firmware. >> >> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> >> --- >> drivers/cpufreq/cppc_cpufreq.c | 39 +++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 37 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c >> index bda0b24..792ed9e 100644 >> --- a/drivers/cpufreq/cppc_cpufreq.c >> +++ b/drivers/cpufreq/cppc_cpufreq.c >> @@ -37,6 +37,7 @@ >> * requested etc. >> */ >> static struct cppc_cpudata **all_cpu_data; >> +static bool boost_supported; >> >> struct cppc_workaround_oem_info { >> char oem_id[ACPI_OEM_ID_SIZE + 1]; >> @@ -310,7 +311,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) >> * Section 8.4.7.1.1.5 of ACPI 6.1 spec) >> */ >> policy->min = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.lowest_nonlinear_perf); >> - policy->max = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.highest_perf); >> + policy->max = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.nominal_perf); >> >> /* >> * Set cpuinfo.min_freq to Lowest to make the full range of performance >> @@ -318,7 +319,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) >> * nonlinear perf >> */ >> policy->cpuinfo.min_freq = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.lowest_perf); >> - policy->cpuinfo.max_freq = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.highest_perf); >> + policy->cpuinfo.max_freq = cppc_cpufreq_perf_to_khz(cpu, cpu->perf_caps.nominal_perf); >> >> policy->transition_delay_us = cppc_cpufreq_get_transition_delay_us(cpu_num); >> policy->shared_type = cpu->shared_type; >> @@ -343,6 +344,13 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) >> >> cpu->cur_policy = policy; >> >> + /* >> + * If 'highest_perf' is greater than 'nominal_perf', we assume CPU Boost >> + * is supported. >> + */ >> + if (cpu->perf_caps.highest_perf > cpu->perf_caps.nominal_perf) >> + boost_supported = true; >> + >> /* Set policy->cur to max now. The governors will adjust later. */ >> policy->cur = cppc_cpufreq_perf_to_khz(cpu, >> cpu->perf_caps.highest_perf); >> @@ -410,6 +418,32 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum) >> return cppc_get_rate_from_fbctrs(cpu, fb_ctrs_t0, fb_ctrs_t1); >> } >> >> +static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state) >> +{ >> + struct cppc_cpudata *cpudata; >> + int ret = 0; > > No need to initialize this. I will change it in the next version. Thanks for your advice. I will add your 'Suggested-by' for these two patches. Thanks, Xiongfeng > >> + >> + if (!boost_supported) { >> + pr_err("BOOST not supported by CPU or firmware\n"); >> + return -EINVAL; >> + } >> + >> + cpudata = all_cpu_data[policy->cpu]; >> + if (state) >> + policy->max = cppc_cpufreq_perf_to_khz(cpudata, >> + cpudata->perf_caps.highest_perf); >> + else >> + policy->max = cppc_cpufreq_perf_to_khz(cpudata, >> + cpudata->perf_caps.nominal_perf); >> + policy->cpuinfo.max_freq = policy->max; >> + >> + ret = freq_qos_update_request(policy->max_freq_req, policy->max); >> + if (ret < 0) >> + return ret; >> + >> + return 0; >> +} >> + >> static struct cpufreq_driver cppc_cpufreq_driver = { >> .flags = CPUFREQ_CONST_LOOPS, >> .verify = cppc_verify_policy, >> @@ -417,6 +451,7 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum) >> .get = cppc_cpufreq_get_rate, >> .init = cppc_cpufreq_cpu_init, >> .stop_cpu = cppc_cpufreq_stop_cpu, >> + .set_boost = cppc_cpufreq_set_boost, >> .name = "cppc_cpufreq", >> }; >> >> -- >> 1.7.12.4 > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-05-21 6:25 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-19 11:41 [RFC PATCH v3 0/2] add SW BOOST support for CPPC Xiongfeng Wang 2020-05-19 11:41 ` [RFC PATCH v3 1/2] cpufreq: change '.set_boost' to act on only one policy Xiongfeng Wang 2020-05-19 11:57 ` Xiongfeng Wang 2020-05-20 4:59 ` Viresh Kumar 2020-05-20 10:36 ` Rafael J. Wysocki 2020-05-20 10:41 ` Viresh Kumar 2020-05-19 11:41 ` [RFC PATCH v3 2/2] CPPC: add support for SW BOOST Xiongfeng Wang 2020-05-20 5:00 ` Viresh Kumar 2020-05-21 6:25 ` Xiongfeng Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).