| Message ID | 1588929064-30270-3-git-send-email-wangxiongfeng2@huawei.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | add SW BOOST support for CPPC | expand |
On Friday, May 8, 2020 11:11:03 AM CEST Xiongfeng Wang wrote: > Software-managed BOOST get the boost frequency by check the flag > CPUFREQ_BOOST_FREQ at driver's frequency table. But some cpufreq driver > don't have frequency table and use other methods to get the frequency > range, such CPPC cpufreq driver. > > To add SW BOOST support for drivers without frequency table, we add > members in 'cpufreq_policy.cpufreq_cpuinfo' to record the max frequency > of boost mode and non-boost mode. The cpufreq driver initialize these two > members when probing. > > Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> > --- > drivers/cpufreq/cpufreq.c | 23 +++++++++++++++-------- > include/linux/cpufreq.h | 2 ++ > 2 files changed, 17 insertions(+), 8 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 475fb1b..a299426 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -2508,15 +2508,22 @@ static int cpufreq_boost_set_sw(int state) > int ret = -EINVAL; > > for_each_active_policy(policy) { > - if (!policy->freq_table) > - continue; > - > - ret = cpufreq_frequency_table_cpuinfo(policy, > + if (policy->freq_table) { > + ret = cpufreq_frequency_table_cpuinfo(policy, > policy->freq_table); > - if (ret) { > - pr_err("%s: Policy frequency update failed\n", > - __func__); > - break; > + if (ret) { > + pr_err("%s: Policy frequency update failed\n", > + __func__); > + break; > + } > + } else if (policy->cpuinfo.boost_max_freq) { > + if (state) > + policy->max = policy->cpuinfo.boost_max_freq; > + else > + policy->max = policy->cpuinfo.nonboost_max_freq; > + policy->cpuinfo.max_freq = policy->max; > + } else { > + continue; > } Why do you need to update this function? The driver should be able to provide its own ->set_boost callback just fine, shouldn't it? > > ret = freq_qos_update_request(policy->max_freq_req, policy->max); > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > index 018dce8..c3449e6 100644 > --- a/include/linux/cpufreq.h > +++ b/include/linux/cpufreq.h > @@ -43,6 +43,8 @@ enum cpufreq_table_sorting { > struct cpufreq_cpuinfo { > unsigned int max_freq; > unsigned int min_freq; > + unsigned int boost_max_freq; > + unsigned int nonboost_max_freq; > > /* in 10^(-9) s = nanoseconds */ > unsigned int transition_latency; >
On 2020/5/14 22:16, Rafael J. Wysocki wrote: > On Friday, May 8, 2020 11:11:03 AM CEST Xiongfeng Wang wrote: >> Software-managed BOOST get the boost frequency by check the flag >> CPUFREQ_BOOST_FREQ at driver's frequency table. But some cpufreq driver >> don't have frequency table and use other methods to get the frequency >> range, such CPPC cpufreq driver. >> >> To add SW BOOST support for drivers without frequency table, we add >> members in 'cpufreq_policy.cpufreq_cpuinfo' to record the max frequency >> of boost mode and non-boost mode. The cpufreq driver initialize these two >> members when probing. >> >> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> >> --- >> drivers/cpufreq/cpufreq.c | 23 +++++++++++++++-------- >> include/linux/cpufreq.h | 2 ++ >> 2 files changed, 17 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c >> index 475fb1b..a299426 100644 >> --- a/drivers/cpufreq/cpufreq.c >> +++ b/drivers/cpufreq/cpufreq.c >> @@ -2508,15 +2508,22 @@ static int cpufreq_boost_set_sw(int state) >> int ret = -EINVAL; >> >> for_each_active_policy(policy) { >> - if (!policy->freq_table) >> - continue; >> - >> - ret = cpufreq_frequency_table_cpuinfo(policy, >> + if (policy->freq_table) { >> + ret = cpufreq_frequency_table_cpuinfo(policy, >> policy->freq_table); >> - if (ret) { >> - pr_err("%s: Policy frequency update failed\n", >> - __func__); >> - break; >> + if (ret) { >> + pr_err("%s: Policy frequency update failed\n", >> + __func__); >> + break; >> + } >> + } else if (policy->cpuinfo.boost_max_freq) { >> + if (state) >> + policy->max = policy->cpuinfo.boost_max_freq; >> + else >> + policy->max = policy->cpuinfo.nonboost_max_freq; >> + policy->cpuinfo.max_freq = policy->max; >> + } else { >> + continue; >> } > > Why do you need to update this function? My original thought is to reuse the current SW BOOST code as possible, but this seems to change the cpufreq core too much. > > The driver should be able to provide its own ->set_boost callback just fine, > shouldn't it? Thanks for your advice. This is better. I will provide a '->set_boost' callback for CPPC driver. But I will need to export 'cpufreq_policy_list' and make the macro 'for_each_active_policy' public. Thanks, Xiongfeng > >> >> ret = freq_qos_update_request(policy->max_freq_req, policy->max); >> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h >> index 018dce8..c3449e6 100644 >> --- a/include/linux/cpufreq.h >> +++ b/include/linux/cpufreq.h >> @@ -43,6 +43,8 @@ enum cpufreq_table_sorting { >> struct cpufreq_cpuinfo { >> unsigned int max_freq; >> unsigned int min_freq; >> + unsigned int boost_max_freq; >> + unsigned int nonboost_max_freq; >> >> /* in 10^(-9) s = nanoseconds */ >> unsigned int transition_latency; >> > > > > > > . >
Sorry for the delay from my side in replying to this thread. On 15-05-20, 09:49, Xiongfeng Wang wrote: > On 2020/5/14 22:16, Rafael J. Wysocki wrote: > > On Friday, May 8, 2020 11:11:03 AM CEST Xiongfeng Wang wrote: > >> Software-managed BOOST get the boost frequency by check the flag > >> CPUFREQ_BOOST_FREQ at driver's frequency table. But some cpufreq driver > >> don't have frequency table and use other methods to get the frequency > >> range, such CPPC cpufreq driver. > >> > >> To add SW BOOST support for drivers without frequency table, we add > >> members in 'cpufreq_policy.cpufreq_cpuinfo' to record the max frequency > >> of boost mode and non-boost mode. The cpufreq driver initialize these two > >> members when probing. > >> > >> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> > >> --- > >> drivers/cpufreq/cpufreq.c | 23 +++++++++++++++-------- > >> include/linux/cpufreq.h | 2 ++ > >> 2 files changed, 17 insertions(+), 8 deletions(-) > >> > >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > >> index 475fb1b..a299426 100644 > >> --- a/drivers/cpufreq/cpufreq.c > >> +++ b/drivers/cpufreq/cpufreq.c > >> @@ -2508,15 +2508,22 @@ static int cpufreq_boost_set_sw(int state) > >> int ret = -EINVAL; > >> > >> for_each_active_policy(policy) { > >> - if (!policy->freq_table) > >> - continue; > >> - > >> - ret = cpufreq_frequency_table_cpuinfo(policy, > >> + if (policy->freq_table) { > >> + ret = cpufreq_frequency_table_cpuinfo(policy, > >> policy->freq_table); > >> - if (ret) { > >> - pr_err("%s: Policy frequency update failed\n", > >> - __func__); > >> - break; > >> + if (ret) { > >> + pr_err("%s: Policy frequency update failed\n", > >> + __func__); > >> + break; > >> + } > >> + } else if (policy->cpuinfo.boost_max_freq) { > >> + if (state) > >> + policy->max = policy->cpuinfo.boost_max_freq; > >> + else > >> + policy->max = policy->cpuinfo.nonboost_max_freq; > >> + policy->cpuinfo.max_freq = policy->max; > >> + } else { > >> + continue; > >> } > > > > Why do you need to update this function? > > My original thought is to reuse the current SW BOOST code as possible, but this > seems to change the cpufreq core too much. > > Thanks for your advice. This is better. I will provide a '->set_boost' callback > for CPPC driver. But I will need to export 'cpufreq_policy_list' and make the > macro 'for_each_active_policy' public. This can and should be avoided, I will rather move the for-each-policy loop in cpufreq_boost_trigger_state() and call ->set_boost() for each policy and pass policy as argument as well. You would be required to update existing users of sw boost.
Hi Viresh, Thanks for your reply ! On 2020/5/18 15:53, Viresh Kumar wrote: > Sorry for the delay from my side in replying to this thread. > > On 15-05-20, 09:49, Xiongfeng Wang wrote: >> On 2020/5/14 22:16, Rafael J. Wysocki wrote: >>> On Friday, May 8, 2020 11:11:03 AM CEST Xiongfeng Wang wrote: >>>> Software-managed BOOST get the boost frequency by check the flag >>>> CPUFREQ_BOOST_FREQ at driver's frequency table. But some cpufreq driver >>>> don't have frequency table and use other methods to get the frequency >>>> range, such CPPC cpufreq driver. >>>> >>>> To add SW BOOST support for drivers without frequency table, we add >>>> members in 'cpufreq_policy.cpufreq_cpuinfo' to record the max frequency >>>> of boost mode and non-boost mode. The cpufreq driver initialize these two >>>> members when probing. >>>> >>>> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> >>>> --- >>>> drivers/cpufreq/cpufreq.c | 23 +++++++++++++++-------- >>>> include/linux/cpufreq.h | 2 ++ >>>> 2 files changed, 17 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c >>>> index 475fb1b..a299426 100644 >>>> --- a/drivers/cpufreq/cpufreq.c >>>> +++ b/drivers/cpufreq/cpufreq.c >>>> @@ -2508,15 +2508,22 @@ static int cpufreq_boost_set_sw(int state) >>>> int ret = -EINVAL; >>>> >>>> for_each_active_policy(policy) { >>>> - if (!policy->freq_table) >>>> - continue; >>>> - >>>> - ret = cpufreq_frequency_table_cpuinfo(policy, >>>> + if (policy->freq_table) { >>>> + ret = cpufreq_frequency_table_cpuinfo(policy, >>>> policy->freq_table); >>>> - if (ret) { >>>> - pr_err("%s: Policy frequency update failed\n", >>>> - __func__); >>>> - break; >>>> + if (ret) { >>>> + pr_err("%s: Policy frequency update failed\n", >>>> + __func__); >>>> + break; >>>> + } >>>> + } else if (policy->cpuinfo.boost_max_freq) { >>>> + if (state) >>>> + policy->max = policy->cpuinfo.boost_max_freq; >>>> + else >>>> + policy->max = policy->cpuinfo.nonboost_max_freq; >>>> + policy->cpuinfo.max_freq = policy->max; >>>> + } else { >>>> + continue; >>>> } >>> >>> Why do you need to update this function? >> >> My original thought is to reuse the current SW BOOST code as possible, but this >> seems to change the cpufreq core too much. >> >> Thanks for your advice. This is better. I will provide a '->set_boost' callback >> for CPPC driver. But I will need to export 'cpufreq_policy_list' and make the >> macro 'for_each_active_policy' public. > > This can and should be avoided, I will rather move the for-each-policy > loop in cpufreq_boost_trigger_state() and call ->set_boost() for each > policy and pass policy as argument as well. You would be required to > update existing users of sw boost. Thanks for your advice. It's a good idea. I will change it in the next version. Thanks, Xiongfeng >
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 475fb1b..a299426 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2508,15 +2508,22 @@ static int cpufreq_boost_set_sw(int state) int ret = -EINVAL; for_each_active_policy(policy) { - if (!policy->freq_table) - continue; - - ret = cpufreq_frequency_table_cpuinfo(policy, + if (policy->freq_table) { + ret = cpufreq_frequency_table_cpuinfo(policy, policy->freq_table); - if (ret) { - pr_err("%s: Policy frequency update failed\n", - __func__); - break; + if (ret) { + pr_err("%s: Policy frequency update failed\n", + __func__); + break; + } + } else if (policy->cpuinfo.boost_max_freq) { + if (state) + policy->max = policy->cpuinfo.boost_max_freq; + else + policy->max = policy->cpuinfo.nonboost_max_freq; + policy->cpuinfo.max_freq = policy->max; + } else { + continue; } ret = freq_qos_update_request(policy->max_freq_req, policy->max); diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 018dce8..c3449e6 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -43,6 +43,8 @@ enum cpufreq_table_sorting { struct cpufreq_cpuinfo { unsigned int max_freq; unsigned int min_freq; + unsigned int boost_max_freq; + unsigned int nonboost_max_freq; /* in 10^(-9) s = nanoseconds */ unsigned int transition_latency;
Software-managed BOOST get the boost frequency by check the flag CPUFREQ_BOOST_FREQ at driver's frequency table. But some cpufreq driver don't have frequency table and use other methods to get the frequency range, such CPPC cpufreq driver. To add SW BOOST support for drivers without frequency table, we add members in 'cpufreq_policy.cpufreq_cpuinfo' to record the max frequency of boost mode and non-boost mode. The cpufreq driver initialize these two members when probing. Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com> --- drivers/cpufreq/cpufreq.c | 23 +++++++++++++++-------- include/linux/cpufreq.h | 2 ++ 2 files changed, 17 insertions(+), 8 deletions(-)