[PATCH] cpufreq: exynos5440: Fix to skip when new frequency same as current

[PATCH] cpufreq: exynos5440: Fix to skip when new frequency same as current

[Amit Daniel Kachhap]

This patch fixes the issue of un-necessary setting the clock controller
when the new target frequency is same as the current one. This case usually
occurs with governors like ondemand which passes the target frequency as the
percentage of average frequency. This check is present in most of the cpufreq
driver.

Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/cpufreq/exynos5440-cpufreq.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/cpufreq/exynos5440-cpufreq.c b/drivers/cpufreq/exynos5440-cpufreq.c
index 0c74018..d514c15 100644
--- a/drivers/cpufreq/exynos5440-cpufreq.c
+++ b/drivers/cpufreq/exynos5440-cpufreq.c
@@ -238,6 +238,9 @@ static int exynos_target(struct cpufreq_policy *policy,
 	freqs.old = dvfs_info->cur_frequency;
 	freqs.new = freq_table[index].frequency;
 
+	if (freqs.old == freqs.new)
+		goto out;
+
 	cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
 
 	/* Set the target frequency in all C0_3_PSTATE register */
-- 
1.7.1

[PATCH] cpufreq: exynos5440: Fix to skip when new frequency same as current

[Viresh Kumar]

On 7 August 2013 16:46, Amit Daniel Kachhap <amit.daniel@samsung.com> wrote:
> This patch fixes the issue of un-necessary setting the clock controller
> when the new target frequency is same as the current one. This case usually
> occurs with governors like ondemand which passes the target frequency as the
> percentage of average frequency. This check is present in most of the cpufreq
> driver.
>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/cpufreq/exynos5440-cpufreq.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Applied.

[PATCH] cpufreq: exynos5440: Fix to skip when new frequency same as current

[Sudeep KarkadaNagesha]

On 07/08/13 12:22, Viresh Kumar wrote:
> On 7 August 2013 16:46, Amit Daniel Kachhap <amit.daniel@samsung.com> wrote:
>> This patch fixes the issue of un-necessary setting the clock controller
>> when the new target frequency is same as the current one. This case usually
>> occurs with governors like ondemand which passes the target frequency as the
>> percentage of average frequency. This check is present in most of the cpufreq
>> driver.
>>
>> Cc: Viresh Kumar <viresh.kumar@linaro.org>
>> Cc: Rafael J. Wysocki <rjw@sisk.pl>
>> Cc: Kukjin Kim <kgene.kim@samsung.com>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  drivers/cpufreq/exynos5440-cpufreq.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> Applied.
Hi Viresh,

Any particular reason we need this check in all drivers after your
commit: 5a1c0228 "cpufreq: Avoid calling cpufreq driver's target()
routine if target_freq == policy->cur"

I think it can removed from all drivers, am I missing something ?

Regards,
Sudeep

[PATCH] cpufreq: exynos5440: Fix to skip when new frequency same as current

[Viresh Kumar]

On 7 August 2013 17:00, Sudeep KarkadaNagesha
<Sudeep.KarkadaNagesha@arm.com> wrote:
> Any particular reason we need this check in all drivers after your
> commit: 5a1c0228 "cpufreq: Avoid calling cpufreq driver's target()
> routine if target_freq == policy->cur"
>
> I think it can removed from all drivers, am I missing something ?

Yeah.. Just a bit though :)

So, cpufreq core checks this when we call target for any frequency.
Now, cpufreq driver actually does a cpufreq_frequency_table_target()
and so the frequency may vary than what is requested, in case
requested frequency isn't picked from the table.

In such cases we check it again to be sure that we aren't at this
frequency already..

Earlier I thought of calling cpufreq_frequency_table_target() in the
core before calling target but dropped the idea as I wasn't sure of
the side effects.

@Rafael: Do you see why we shouldn't/can't call
cpufreq_frequency_table_target() from the core itself and so drivers
never need to do it?

[PATCH] cpufreq: exynos5440: Fix to skip when new frequency same as current

[Rafael J. Wysocki]

On Wednesday, August 07, 2013 05:03:59 PM Viresh Kumar wrote:
> On 7 August 2013 17:00, Sudeep KarkadaNagesha
> <Sudeep.KarkadaNagesha@arm.com> wrote:
> > Any particular reason we need this check in all drivers after your
> > commit: 5a1c0228 "cpufreq: Avoid calling cpufreq driver's target()
> > routine if target_freq == policy->cur"
> >
> > I think it can removed from all drivers, am I missing something ?
> 
> Yeah.. Just a bit though :)
> 
> So, cpufreq core checks this when we call target for any frequency.
> Now, cpufreq driver actually does a cpufreq_frequency_table_target()
> and so the frequency may vary than what is requested, in case
> requested frequency isn't picked from the table.
> 
> In such cases we check it again to be sure that we aren't at this
> frequency already..
> 
> Earlier I thought of calling cpufreq_frequency_table_target() in the
> core before calling target but dropped the idea as I wasn't sure of
> the side effects.
> 
> @Rafael: Do you see why we shouldn't/can't call
> cpufreq_frequency_table_target() from the core itself and so drivers
> never need to do it?

It looks like it would require us to redefine .target() to take next_state
instead of target_freq (at least in the acpi-cpufreq case), wouldn't it?

Rafael

[PATCH] cpufreq: exynos5440: Fix to skip when new frequency same as current

[Viresh Kumar]

On 8 August 2013 04:55, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> It looks like it would require us to redefine .target() to take next_state
> instead of target_freq (at least in the acpi-cpufreq case), wouldn't it?

If we don't do it, then atleast for few drivers, like acpi-cpufreq, which
use index more than just for frequency, we may end up calling
cpufreq_frequency_table_target() twice. Once in the core and then
in driver.

I believe this is doable and will post a patch soon.