Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
On 29 May 2014 01:10, Doug Anderson wrote: > As far as I can tell this notification says "I tried to switch from > "intermediate_freq" to "policy->restore_freq" but I failed, so I'm > still at "intermediate_freq". I think you probably want to pass 0 as > the last argument to cpufreq_freq_transition_end() to fix... Correct !! > Other than that this looks good to me. I'll do a final review when > you spin the next version (since I think you need to fix stuff for > Stephen too). I'll probably wait and re-review the Tegra version when > you and Stephen come to a consensus on it. I am waiting for Stephen's reply on the other thread to close this issue. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
On 29 May 2014 01:10, Doug Anderson diand...@chromium.org wrote: As far as I can tell this notification says I tried to switch from intermediate_freq to policy-restore_freq but I failed, so I'm still at intermediate_freq. I think you probably want to pass 0 as the last argument to cpufreq_freq_transition_end() to fix... Correct !! Other than that this looks good to me. I'll do a final review when you spin the next version (since I think you need to fix stuff for Stephen too). I'll probably wait and re-review the Tegra version when you and Stephen come to a consensus on it. I am waiting for Stephen's reply on the other thread to close this issue. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
Viresh,
On Wed, May 21, 2014 at 1:59 AM, Viresh Kumar wrote:
> @@ -1841,9 +1876,23 @@ static int __target_index(struct cpufreq_policy
> *policy,
> pr_err("%s: Failed to change cpu frequency: %d\n", __func__,
>retval);
>
> - if (notify)
> + if (notify) {
> cpufreq_freq_transition_end(policy, , retval);
>
> + /*
> +* Failed after setting to intermediate freq? Driver should
> have
> +* reverted back to initial frequency and so should we. Check
> +* here for intermediate_freq instead of get_intermediate, in
> +* case we have't switched to intermediate freq at all.
> +*/
> + if (unlikely(retval && intermediate_freq)) {
> + freqs.old = intermediate_freq;
> + freqs.new = policy->restore_freq;
> + cpufreq_freq_transition_begin(policy, );
> + cpufreq_freq_transition_end(policy, , retval);
As far as I can tell this notification says "I tried to switch from
"intermediate_freq" to "policy->restore_freq" but I failed, so I'm
still at "intermediate_freq". I think you probably want to pass 0 as
the last argument to cpufreq_freq_transition_end() to fix...
Other than that this looks good to me. I'll do a final review when
you spin the next version (since I think you need to fix stuff for
Stephen too). I'll probably wait and re-review the Tegra version when
you and Stephen come to a consensus on it.
-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
Viresh,
On Wed, May 21, 2014 at 1:59 AM, Viresh Kumar viresh.ku...@linaro.org wrote:
@@ -1841,9 +1876,23 @@ static int __target_index(struct cpufreq_policy
*policy,
pr_err(%s: Failed to change cpu frequency: %d\n, __func__,
retval);
- if (notify)
+ if (notify) {
cpufreq_freq_transition_end(policy, freqs, retval);
+ /*
+* Failed after setting to intermediate freq? Driver should
have
+* reverted back to initial frequency and so should we. Check
+* here for intermediate_freq instead of get_intermediate, in
+* case we have't switched to intermediate freq at all.
+*/
+ if (unlikely(retval intermediate_freq)) {
+ freqs.old = intermediate_freq;
+ freqs.new = policy-restore_freq;
+ cpufreq_freq_transition_begin(policy, freqs);
+ cpufreq_freq_transition_end(policy, freqs, retval);
As far as I can tell this notification says I tried to switch from
intermediate_freq to policy-restore_freq but I failed, so I'm
still at intermediate_freq. I think you probably want to pass 0 as
the last argument to cpufreq_freq_transition_end() to fix...
Other than that this looks good to me. I'll do a final review when
you spin the next version (since I think you need to fix stuff for
Stephen too). I'll probably wait and re-review the Tegra version when
you and Stephen come to a consensus on it.
-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
On 23 May 2014 21:26, Stephen Warren wrote: > Oh OK, I guess the "notify" value is static then, and driver defined, so > this is fine. Correct!! Can you reply on the tegra patch also? So that we can close this thread ASAP? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
On 23 May 2014 21:26, Stephen Warren swar...@wwwdotorg.org wrote: Oh OK, I guess the notify value is static then, and driver defined, so this is fine. Correct!! Can you reply on the tegra patch also? So that we can close this thread ASAP? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
On 05/22/2014 10:24 PM, Viresh Kumar wrote:
> On 22 May 2014 22:07, Stephen Warren wrote:
>> It seems rather odd to set both freqs->old and freqs->new to the
>> intermediate frequency upon success. It feels like it would make more
>> sense to remove that final else clause above, and do the following where
>> this function is called:
>>> static int __target_index(struct cpufreq_policy *policy,
>>> struct cpufreq_frequency_table *freq_table, int
>>> index)
>>> {
>>> - struct cpufreq_freqs freqs;
>>> + struct cpufreq_freqs freqs = {.old = policy->cur, .flags = 0};
>>> + unsigned int intermediate_freq = 0;
>>> int retval = -EINVAL;
>>> bool notify;
>>>
>>> notify = !(cpufreq_driver->flags & CPUFREQ_ASYNC_NOTIFICATION);
>>> -
>>> if (notify) {
>>> - freqs.old = policy->cur;
>>> - freqs.new = freq_table[index].frequency;
>>> - freqs.flags = 0;
>>> + /* Handle switching to intermediate frequency */
>>> + if (cpufreq_driver->get_intermediate) {
>>> + retval = __target_intermediate(policy, , index);
>>> + if (retval)
>>> + return retval;
>>
>> Shouldn't this all be outside the if (notify) block, so that
>> __target_intermediate is always called, and it's just the notify
>> callbacks that gets skipped if (!notify)?
>
> So, this is logic I had:
>
> Should we support 'intermediate freq' infrastructure when driver wants
> to handle notifications themselves?
>
> Probably not.
>
> The whole point of implementing this 'intermediate freq' infrastructure is to
> get rid of code redundancy while sending notifications. If drivers want to
> handle notifications then they better handle intermediate freqs as well in
> their target_index() callback. They don't need to implement another routine
> for intermediate stuff..
Oh OK, I guess the "notify" value is static then, and driver defined, so
this is fine.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
On 05/22/2014 10:24 PM, Viresh Kumar wrote:
On 22 May 2014 22:07, Stephen Warren swar...@wwwdotorg.org wrote:
It seems rather odd to set both freqs-old and freqs-new to the
intermediate frequency upon success. It feels like it would make more
sense to remove that final else clause above, and do the following where
this function is called:
static int __target_index(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *freq_table, int
index)
{
- struct cpufreq_freqs freqs;
+ struct cpufreq_freqs freqs = {.old = policy-cur, .flags = 0};
+ unsigned int intermediate_freq = 0;
int retval = -EINVAL;
bool notify;
notify = !(cpufreq_driver-flags CPUFREQ_ASYNC_NOTIFICATION);
-
if (notify) {
- freqs.old = policy-cur;
- freqs.new = freq_table[index].frequency;
- freqs.flags = 0;
+ /* Handle switching to intermediate frequency */
+ if (cpufreq_driver-get_intermediate) {
+ retval = __target_intermediate(policy, freqs, index);
+ if (retval)
+ return retval;
Shouldn't this all be outside the if (notify) block, so that
__target_intermediate is always called, and it's just the notify
callbacks that gets skipped if (!notify)?
So, this is logic I had:
Should we support 'intermediate freq' infrastructure when driver wants
to handle notifications themselves?
Probably not.
The whole point of implementing this 'intermediate freq' infrastructure is to
get rid of code redundancy while sending notifications. If drivers want to
handle notifications then they better handle intermediate freqs as well in
their target_index() callback. They don't need to implement another routine
for intermediate stuff..
Oh OK, I guess the notify value is static then, and driver defined, so
this is fine.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
On 22 May 2014 22:07, Stephen Warren wrote:
> It seems rather odd to set both freqs->old and freqs->new to the
> intermediate frequency upon success. It feels like it would make more
> sense to remove that final else clause above, and do the following where
> this function is called:
>
> intermediate_freq = freqs.new;
> if (intermediate_freq)
> freqs.old = intermediate_freq
> freqs.new = freq_table[index].frequency
Looks better.
> (or even return the intermediate frequency from the function)
It can return errors as well and so I didn't tried to return frequency
from it. Though there are routines that return both error and freq
from such calls :)
> ?
>
> But I suppose isolating all the inside __target_intermediate() isn't so bad.
:)
>> static int __target_index(struct cpufreq_policy *policy,
>> struct cpufreq_frequency_table *freq_table, int
>> index)
>> {
>> - struct cpufreq_freqs freqs;
>> + struct cpufreq_freqs freqs = {.old = policy->cur, .flags = 0};
>> + unsigned int intermediate_freq = 0;
>> int retval = -EINVAL;
>> bool notify;
>>
>> notify = !(cpufreq_driver->flags & CPUFREQ_ASYNC_NOTIFICATION);
>> -
>> if (notify) {
>> - freqs.old = policy->cur;
>> - freqs.new = freq_table[index].frequency;
>> - freqs.flags = 0;
>> + /* Handle switching to intermediate frequency */
>> + if (cpufreq_driver->get_intermediate) {
>> + retval = __target_intermediate(policy, , index);
>> + if (retval)
>> + return retval;
>
> Shouldn't this all be outside the if (notify) block, so that
> __target_intermediate is always called, and it's just the notify
> callbacks that gets skipped if (!notify)?
So, this is logic I had:
Should we support 'intermediate freq' infrastructure when driver wants
to handle notifications themselves?
Probably not.
The whole point of implementing this 'intermediate freq' infrastructure is to
get rid of code redundancy while sending notifications. If drivers want to
handle notifications then they better handle intermediate freqs as well in
their target_index() callback. They don't need to implement another routine
for intermediate stuff..
--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
On 05/21/2014 02:59 AM, Viresh Kumar wrote:
> Douglas Anderson, recently pointed out an interesting problem due to which
> udelay() was expiring earlier than it should.
>
> While transitioning between frequencies few platforms may temporarily switch
> to
> a stable frequency, waiting for the main PLL to stabilize.
>
> For example: When we transition between very low frequencies on exynos, like
> between 200MHz and 300MHz, we may temporarily switch to a PLL running at
> 800MHz.
> No CPUFREQ notification is sent for that. That means there's a period of time
> when we're running at 800MHz but loops_per_jiffy is calibrated at between
> 200MHz
> and 300MHz. And so udelay behaves badly.
>
> To get this fixed in a generic way, lets introduce another set of callbacks
> get_intermediate() and target_intermediate(), only for drivers with
> target_index() and CPUFREQ_ASYNC_NOTIFICATION unset.
>
> get_intermediate() should return a stable intermediate frequency platform
> wants
> to switch to, and target_intermediate() should set CPU to to that frequency,
> before jumping to the frequency corresponding to 'index'. Core will take care
> of
> sending notifications and driver doesn't have to handle them in
> target_intermediate() or target_index().
>
> NOTE: ->target_index() should restore to policy->restore_freq in case of
> failures as core would send notifications for that.
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> +/* Must set freqs->new to intermediate frequency */
> +static int __target_intermediate(struct cpufreq_policy *policy,
> + struct cpufreq_freqs *freqs, int index)
> +{
> + int ret;
> +
> + freqs->new = cpufreq_driver->get_intermediate(policy, index);
> +
> + /* We don't need to switch to intermediate freq */
> + if (!freqs->new)
> + return 0;
> +
> + pr_debug("%s: cpu: %d, switching to intermediate freq: oldfreq: %u,
> intermediate freq: %u\n",
> + __func__, policy->cpu, freqs->old, freqs->new);
> +
> + cpufreq_freq_transition_begin(policy, freqs);
> + ret = cpufreq_driver->target_intermediate(policy, index);
> + cpufreq_freq_transition_end(policy, freqs, ret);
> +
> + if (ret)
> + pr_err("%s: Failed to change to intermediate frequency: %d\n",
> +__func__, ret);
> + else
> + /* Set old freq to intermediate */
> + freqs->old = freqs->new;
> +
> + return ret;
> +}
It seems rather odd to set both freqs->old and freqs->new to the
intermediate frequency upon success. It feels like it would make more
sense to remove that final else clause above, and do the following where
this function is called:
intermediate_freq = freqs.new;
if (intermediate_freq)
freqs.old = intermediate_freq
freqs.new = freq_table[index].frequency
(or even return the intermediate frequency from the function)
?
But I suppose isolating all the inside __target_intermediate() isn't so bad.
> static int __target_index(struct cpufreq_policy *policy,
> struct cpufreq_frequency_table *freq_table, int index)
> {
> - struct cpufreq_freqs freqs;
> + struct cpufreq_freqs freqs = {.old = policy->cur, .flags = 0};
> + unsigned int intermediate_freq = 0;
> int retval = -EINVAL;
> bool notify;
>
> notify = !(cpufreq_driver->flags & CPUFREQ_ASYNC_NOTIFICATION);
> -
> if (notify) {
> - freqs.old = policy->cur;
> - freqs.new = freq_table[index].frequency;
> - freqs.flags = 0;
> + /* Handle switching to intermediate frequency */
> + if (cpufreq_driver->get_intermediate) {
> + retval = __target_intermediate(policy, , index);
> + if (retval)
> + return retval;
Shouldn't this all be outside the if (notify) block, so that
__target_intermediate is always called, and it's just the notify
callbacks that gets skipped if (!notify)?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
On 05/21/2014 02:59 AM, Viresh Kumar wrote:
Douglas Anderson, recently pointed out an interesting problem due to which
udelay() was expiring earlier than it should.
While transitioning between frequencies few platforms may temporarily switch
to
a stable frequency, waiting for the main PLL to stabilize.
For example: When we transition between very low frequencies on exynos, like
between 200MHz and 300MHz, we may temporarily switch to a PLL running at
800MHz.
No CPUFREQ notification is sent for that. That means there's a period of time
when we're running at 800MHz but loops_per_jiffy is calibrated at between
200MHz
and 300MHz. And so udelay behaves badly.
To get this fixed in a generic way, lets introduce another set of callbacks
get_intermediate() and target_intermediate(), only for drivers with
target_index() and CPUFREQ_ASYNC_NOTIFICATION unset.
get_intermediate() should return a stable intermediate frequency platform
wants
to switch to, and target_intermediate() should set CPU to to that frequency,
before jumping to the frequency corresponding to 'index'. Core will take care
of
sending notifications and driver doesn't have to handle them in
target_intermediate() or target_index().
NOTE: -target_index() should restore to policy-restore_freq in case of
failures as core would send notifications for that.
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
+/* Must set freqs-new to intermediate frequency */
+static int __target_intermediate(struct cpufreq_policy *policy,
+ struct cpufreq_freqs *freqs, int index)
+{
+ int ret;
+
+ freqs-new = cpufreq_driver-get_intermediate(policy, index);
+
+ /* We don't need to switch to intermediate freq */
+ if (!freqs-new)
+ return 0;
+
+ pr_debug(%s: cpu: %d, switching to intermediate freq: oldfreq: %u,
intermediate freq: %u\n,
+ __func__, policy-cpu, freqs-old, freqs-new);
+
+ cpufreq_freq_transition_begin(policy, freqs);
+ ret = cpufreq_driver-target_intermediate(policy, index);
+ cpufreq_freq_transition_end(policy, freqs, ret);
+
+ if (ret)
+ pr_err(%s: Failed to change to intermediate frequency: %d\n,
+__func__, ret);
+ else
+ /* Set old freq to intermediate */
+ freqs-old = freqs-new;
+
+ return ret;
+}
It seems rather odd to set both freqs-old and freqs-new to the
intermediate frequency upon success. It feels like it would make more
sense to remove that final else clause above, and do the following where
this function is called:
intermediate_freq = freqs.new;
if (intermediate_freq)
freqs.old = intermediate_freq
freqs.new = freq_table[index].frequency
(or even return the intermediate frequency from the function)
?
But I suppose isolating all the inside __target_intermediate() isn't so bad.
static int __target_index(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *freq_table, int index)
{
- struct cpufreq_freqs freqs;
+ struct cpufreq_freqs freqs = {.old = policy-cur, .flags = 0};
+ unsigned int intermediate_freq = 0;
int retval = -EINVAL;
bool notify;
notify = !(cpufreq_driver-flags CPUFREQ_ASYNC_NOTIFICATION);
-
if (notify) {
- freqs.old = policy-cur;
- freqs.new = freq_table[index].frequency;
- freqs.flags = 0;
+ /* Handle switching to intermediate frequency */
+ if (cpufreq_driver-get_intermediate) {
+ retval = __target_intermediate(policy, freqs, index);
+ if (retval)
+ return retval;
Shouldn't this all be outside the if (notify) block, so that
__target_intermediate is always called, and it's just the notify
callbacks that gets skipped if (!notify)?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
On 22 May 2014 22:07, Stephen Warren swar...@wwwdotorg.org wrote:
It seems rather odd to set both freqs-old and freqs-new to the
intermediate frequency upon success. It feels like it would make more
sense to remove that final else clause above, and do the following where
this function is called:
intermediate_freq = freqs.new;
if (intermediate_freq)
freqs.old = intermediate_freq
freqs.new = freq_table[index].frequency
Looks better.
(or even return the intermediate frequency from the function)
It can return errors as well and so I didn't tried to return frequency
from it. Though there are routines that return both error and freq
from such calls :)
?
But I suppose isolating all the inside __target_intermediate() isn't so bad.
:)
static int __target_index(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *freq_table, int
index)
{
- struct cpufreq_freqs freqs;
+ struct cpufreq_freqs freqs = {.old = policy-cur, .flags = 0};
+ unsigned int intermediate_freq = 0;
int retval = -EINVAL;
bool notify;
notify = !(cpufreq_driver-flags CPUFREQ_ASYNC_NOTIFICATION);
-
if (notify) {
- freqs.old = policy-cur;
- freqs.new = freq_table[index].frequency;
- freqs.flags = 0;
+ /* Handle switching to intermediate frequency */
+ if (cpufreq_driver-get_intermediate) {
+ retval = __target_intermediate(policy, freqs, index);
+ if (retval)
+ return retval;
Shouldn't this all be outside the if (notify) block, so that
__target_intermediate is always called, and it's just the notify
callbacks that gets skipped if (!notify)?
So, this is logic I had:
Should we support 'intermediate freq' infrastructure when driver wants
to handle notifications themselves?
Probably not.
The whole point of implementing this 'intermediate freq' infrastructure is to
get rid of code redundancy while sending notifications. If drivers want to
handle notifications then they better handle intermediate freqs as well in
their target_index() callback. They don't need to implement another routine
for intermediate stuff..
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
Douglas Anderson, recently pointed out an interesting problem due to which udelay() was expiring earlier than it should. While transitioning between frequencies few platforms may temporarily switch to a stable frequency, waiting for the main PLL to stabilize. For example: When we transition between very low frequencies on exynos, like between 200MHz and 300MHz, we may temporarily switch to a PLL running at 800MHz. No CPUFREQ notification is sent for that. That means there's a period of time when we're running at 800MHz but loops_per_jiffy is calibrated at between 200MHz and 300MHz. And so udelay behaves badly. To get this fixed in a generic way, lets introduce another set of callbacks get_intermediate() and target_intermediate(), only for drivers with target_index() and CPUFREQ_ASYNC_NOTIFICATION unset. get_intermediate() should return a stable intermediate frequency platform wants to switch to, and target_intermediate() should set CPU to to that frequency, before jumping to the frequency corresponding to 'index'. Core will take care of sending notifications and driver doesn't have to handle them in target_intermediate() or target_index(). NOTE: ->target_index() should restore to policy->restore_freq in case of failures as core would send notifications for that. Signed-off-by: Viresh Kumar --- Documentation/cpu-freq/cpu-drivers.txt | 29 ++- drivers/cpufreq/cpufreq.c | 67 ++ include/linux/cpufreq.h| 25 + 3 files changed, 112 insertions(+), 9 deletions(-) diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt index b045fe5..14f4e63 100644 --- a/Documentation/cpu-freq/cpu-drivers.txt +++ b/Documentation/cpu-freq/cpu-drivers.txt @@ -26,6 +26,7 @@ Contents: 1.4 target/target_index or setpolicy? 1.5 target/target_index 1.6 setpolicy +1.7 get_intermediate and target_intermediate 2. Frequency Table Helpers @@ -79,6 +80,10 @@ cpufreq_driver.attr -A pointer to a NULL-terminated list of "struct freq_attr" which allow to export values to sysfs. +cpufreq_driver.get_intermediate +and target_intermediateUsed to switch to stable frequency while + changing CPU frequency. + 1.2 Per-CPU Initialization -- @@ -151,7 +156,7 @@ Some cpufreq-capable processors switch the frequency between certain limits on their own. These shall use the ->setpolicy call -1.4. target/target_index +1.5. target/target_index - The target_index call has two arguments: struct cpufreq_policy *policy, @@ -160,6 +165,9 @@ and unsigned int index (into the exposed frequency table). The CPUfreq driver must set the new frequency when called here. The actual frequency must be determined by freq_table[index].frequency. +It should always restore to earlier frequency (i.e. policy->restore_freq) in +case of errors, even if we switched to intermediate frequency earlier. + Deprecated: -- The target call has three arguments: struct cpufreq_policy *policy, @@ -179,7 +187,7 @@ Here again the frequency table helper might assist you - see section 2 for details. -1.5 setpolicy +1.6 setpolicy --- The setpolicy call only takes a struct cpufreq_policy *policy as @@ -190,6 +198,23 @@ setting when policy->policy is CPUFREQ_POLICY_PERFORMANCE, and a powersaving-oriented setting when CPUFREQ_POLICY_POWERSAVE. Also check the reference implementation in drivers/cpufreq/longrun.c +1.7 get_intermediate and target_intermediate + + +Only for drivers with target_index() and CPUFREQ_ASYNC_NOTIFICATION unset. + +get_intermediate should return a stable intermediate frequency platform wants to +switch to, and target_intermediate() should set CPU to to that frequency, before +jumping to the frequency corresponding to 'index'. Core will take care of +sending notifications and driver doesn't have to handle them in +target_intermediate() or target_index(). + +Drivers can return '0' from get_intermediate() in case they don't wish to switch +to intermediate frequency for some target frequency. In that case core will +directly call ->target_index(). + +NOTE: ->target_index() should restore to policy->restore_freq in case of +failures as core would send notifications for that. 2. Frequency Table Helpers diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index ae11dd5..a72c4d5 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1816,20 +1816,55 @@ EXPORT_SYMBOL(cpufreq_unregister_notifier); * GOVERNORS* */ +/* Must set freqs->new to intermediate frequency */ +static int __target_intermediate(struct cpufreq_policy
[PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies
Douglas Anderson, recently pointed out an interesting problem due to which udelay() was expiring earlier than it should. While transitioning between frequencies few platforms may temporarily switch to a stable frequency, waiting for the main PLL to stabilize. For example: When we transition between very low frequencies on exynos, like between 200MHz and 300MHz, we may temporarily switch to a PLL running at 800MHz. No CPUFREQ notification is sent for that. That means there's a period of time when we're running at 800MHz but loops_per_jiffy is calibrated at between 200MHz and 300MHz. And so udelay behaves badly. To get this fixed in a generic way, lets introduce another set of callbacks get_intermediate() and target_intermediate(), only for drivers with target_index() and CPUFREQ_ASYNC_NOTIFICATION unset. get_intermediate() should return a stable intermediate frequency platform wants to switch to, and target_intermediate() should set CPU to to that frequency, before jumping to the frequency corresponding to 'index'. Core will take care of sending notifications and driver doesn't have to handle them in target_intermediate() or target_index(). NOTE: -target_index() should restore to policy-restore_freq in case of failures as core would send notifications for that. Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- Documentation/cpu-freq/cpu-drivers.txt | 29 ++- drivers/cpufreq/cpufreq.c | 67 ++ include/linux/cpufreq.h| 25 + 3 files changed, 112 insertions(+), 9 deletions(-) diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt index b045fe5..14f4e63 100644 --- a/Documentation/cpu-freq/cpu-drivers.txt +++ b/Documentation/cpu-freq/cpu-drivers.txt @@ -26,6 +26,7 @@ Contents: 1.4 target/target_index or setpolicy? 1.5 target/target_index 1.6 setpolicy +1.7 get_intermediate and target_intermediate 2. Frequency Table Helpers @@ -79,6 +80,10 @@ cpufreq_driver.attr -A pointer to a NULL-terminated list of struct freq_attr which allow to export values to sysfs. +cpufreq_driver.get_intermediate +and target_intermediateUsed to switch to stable frequency while + changing CPU frequency. + 1.2 Per-CPU Initialization -- @@ -151,7 +156,7 @@ Some cpufreq-capable processors switch the frequency between certain limits on their own. These shall use the -setpolicy call -1.4. target/target_index +1.5. target/target_index - The target_index call has two arguments: struct cpufreq_policy *policy, @@ -160,6 +165,9 @@ and unsigned int index (into the exposed frequency table). The CPUfreq driver must set the new frequency when called here. The actual frequency must be determined by freq_table[index].frequency. +It should always restore to earlier frequency (i.e. policy-restore_freq) in +case of errors, even if we switched to intermediate frequency earlier. + Deprecated: -- The target call has three arguments: struct cpufreq_policy *policy, @@ -179,7 +187,7 @@ Here again the frequency table helper might assist you - see section 2 for details. -1.5 setpolicy +1.6 setpolicy --- The setpolicy call only takes a struct cpufreq_policy *policy as @@ -190,6 +198,23 @@ setting when policy-policy is CPUFREQ_POLICY_PERFORMANCE, and a powersaving-oriented setting when CPUFREQ_POLICY_POWERSAVE. Also check the reference implementation in drivers/cpufreq/longrun.c +1.7 get_intermediate and target_intermediate + + +Only for drivers with target_index() and CPUFREQ_ASYNC_NOTIFICATION unset. + +get_intermediate should return a stable intermediate frequency platform wants to +switch to, and target_intermediate() should set CPU to to that frequency, before +jumping to the frequency corresponding to 'index'. Core will take care of +sending notifications and driver doesn't have to handle them in +target_intermediate() or target_index(). + +Drivers can return '0' from get_intermediate() in case they don't wish to switch +to intermediate frequency for some target frequency. In that case core will +directly call -target_index(). + +NOTE: -target_index() should restore to policy-restore_freq in case of +failures as core would send notifications for that. 2. Frequency Table Helpers diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index ae11dd5..a72c4d5 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1816,20 +1816,55 @@ EXPORT_SYMBOL(cpufreq_unregister_notifier); * GOVERNORS* */ +/* Must set freqs-new to intermediate frequency */ +static int __target_intermediate(struct
