[PATCH 1/3] cpufreq: pmac64: speed up frequency switch

[PATCH 1/3] cpufreq: pmac64: speed up frequency switch

[Aaro Koskinen]

Some functions on switch path use msleep() which is inaccurate, and
depends on HZ. With HZ=100 msleep(1) takes actually over ten times longer.
Using usleep_range() we get more accurate sleeps.

I measured the "pfunc_slewing_done" polling to take 300us at max (on
2.3GHz dual-processor Xserve G5), so using 500us sleep there should
be fine.

With the patch, g5_switch_freq() duration drops from ~50ms to ~10ms on
Xserve with HZ=100.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/cpufreq/pmac64-cpufreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/pmac64-cpufreq.c b/drivers/cpufreq/pmac64-cpufreq.c
index 7ba4234..674807d 100644
--- a/drivers/cpufreq/pmac64-cpufreq.c
+++ b/drivers/cpufreq/pmac64-cpufreq.c
@@ -141,7 +141,7 @@ static void g5_vdnap_switch_volt(int speed_mode)
 		pmf_call_one(pfunc_vdnap0_complete, &args);
 		if (done)
 			break;
-		msleep(1);
+		usleep_range(1000, 1000);
 	}
 	if (done == 0)
 		printk(KERN_WARNING "cpufreq: Timeout in clock slewing !\n");
@@ -240,7 +240,7 @@ static void g5_pfunc_switch_volt(int speed_mode)
 		if (pfunc_cpu1_volt_low)
 			pmf_call_one(pfunc_cpu1_volt_low, NULL);
 	}
-	msleep(10); /* should be faster , to fix */
+	usleep_range(10000, 10000); /* should be faster , to fix */
 }
 
 /*
@@ -285,7 +285,7 @@ static int g5_pfunc_switch_freq(int speed_mode)
 		pmf_call_one(pfunc_slewing_done, &args);
 		if (done)
 			break;
-		msleep(1);
+		usleep_range(500, 500);
 	}
 	if (done == 0)
 		printk(KERN_WARNING "cpufreq: Timeout in clock slewing !\n");
-- 
1.8.3.2

[PATCH 2/3] cpufreq: pmac64: re-estimate G5 cpufreq transition latency

[Aaro Koskinen]

The latency is in milliseconds scale rather than microseconds based on
measurements on iMac G5 and Xscale G5. The patch also enables to use
ondemand governor on the latter.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/cpufreq/pmac64-cpufreq.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/pmac64-cpufreq.c b/drivers/cpufreq/pmac64-cpufreq.c
index 674807d..f9e399b 100644
--- a/drivers/cpufreq/pmac64-cpufreq.c
+++ b/drivers/cpufreq/pmac64-cpufreq.c
@@ -85,7 +85,8 @@ static int (*g5_query_freq)(void);
 
 static DEFINE_MUTEX(g5_switch_mutex);
 
-static unsigned long transition_latency;
+/* A conservative estimate, based on Xserve G5 and iMac G5 (iSight). */
+static const unsigned long transition_latency = 10 * NSEC_PER_MSEC;
 
 #ifdef CONFIG_PMAC_SMU
 
@@ -499,7 +500,6 @@ static int __init g5_neo2_cpufreq_init(struct device_node *cpus)
 	g5_cpu_freqs[1].frequency = max_freq/2;
 
 	/* Set callbacks */
-	transition_latency = 12000;
 	g5_switch_freq = g5_scom_switch_freq;
 	g5_query_freq = g5_scom_query_freq;
 	freq_method = "SCOM";
@@ -675,7 +675,6 @@ static int __init g5_pm72_cpufreq_init(struct device_node *cpus)
 	g5_cpu_freqs[1].frequency = min_freq;
 
 	/* Set callbacks */
-	transition_latency = CPUFREQ_ETERNAL;
 	g5_switch_volt = g5_pfunc_switch_volt;
 	g5_switch_freq = g5_pfunc_switch_freq;
 	g5_query_freq = g5_pfunc_query_freq;
-- 
1.8.3.2

Re: [PATCH 2/3] cpufreq: pmac64: re-estimate G5 cpufreq transition latency

[Viresh Kumar]

On 24 July 2013 01:54, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> The patch also enables to use ondemand governor on the latter.

How? I can't see anything obvious here. :(

>
> Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> ---
>  drivers/cpufreq/pmac64-cpufreq.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/pmac64-cpufreq.c b/drivers/cpufreq/pmac64-cpufreq.c
> index 674807d..f9e399b 100644
> --- a/drivers/cpufreq/pmac64-cpufreq.c
> +++ b/drivers/cpufreq/pmac64-cpufreq.c
> @@ -85,7 +85,8 @@ static int (*g5_query_freq)(void);
>
>  static DEFINE_MUTEX(g5_switch_mutex);
>
> -static unsigned long transition_latency;
> +/* A conservative estimate, based on Xserve G5 and iMac G5 (iSight). */
> +static const unsigned long transition_latency = 10 * NSEC_PER_MSEC;
>
>  #ifdef CONFIG_PMAC_SMU
>
> @@ -499,7 +500,6 @@ static int __init g5_neo2_cpufreq_init(struct device_node *cpus)
>         g5_cpu_freqs[1].frequency = max_freq/2;
>
>         /* Set callbacks */
> -       transition_latency = 12000;
>         g5_switch_freq = g5_scom_switch_freq;
>         g5_query_freq = g5_scom_query_freq;
>         freq_method = "SCOM";
> @@ -675,7 +675,6 @@ static int __init g5_pm72_cpufreq_init(struct device_node *cpus)
>         g5_cpu_freqs[1].frequency = min_freq;
>
>         /* Set callbacks */
> -       transition_latency = CPUFREQ_ETERNAL;
>         g5_switch_volt = g5_pfunc_switch_volt;
>         g5_switch_freq = g5_pfunc_switch_freq;
>         g5_query_freq = g5_pfunc_query_freq;
> --
> 1.8.3.2
>

Re: [PATCH 2/3] cpufreq: pmac64: re-estimate G5 cpufreq transition latency

[Aaro Koskinen]

Hi,

On Wed, Jul 24, 2013 at 11:04:50AM +0530, Viresh Kumar wrote:
> On 24 July 2013 01:54, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> > The patch also enables to use ondemand governor on the latter.
> 
> How? I can't see anything obvious here. :(

It replaces CPUFREQ_ETERNAL with a proper value on older PowerMacs.
ondemand does not accept CPUFREQ_ETERNAL transition latency.

A.

> 
> >
> > Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> > ---
> >  drivers/cpufreq/pmac64-cpufreq.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/cpufreq/pmac64-cpufreq.c b/drivers/cpufreq/pmac64-cpufreq.c
> > index 674807d..f9e399b 100644
> > --- a/drivers/cpufreq/pmac64-cpufreq.c
> > +++ b/drivers/cpufreq/pmac64-cpufreq.c
> > @@ -85,7 +85,8 @@ static int (*g5_query_freq)(void);
> >
> >  static DEFINE_MUTEX(g5_switch_mutex);
> >
> > -static unsigned long transition_latency;
> > +/* A conservative estimate, based on Xserve G5 and iMac G5 (iSight). */
> > +static const unsigned long transition_latency = 10 * NSEC_PER_MSEC;
> >
> >  #ifdef CONFIG_PMAC_SMU
> >
> > @@ -499,7 +500,6 @@ static int __init g5_neo2_cpufreq_init(struct device_node *cpus)
> >         g5_cpu_freqs[1].frequency = max_freq/2;
> >
> >         /* Set callbacks */
> > -       transition_latency = 12000;
> >         g5_switch_freq = g5_scom_switch_freq;
> >         g5_query_freq = g5_scom_query_freq;
> >         freq_method = "SCOM";
> > @@ -675,7 +675,6 @@ static int __init g5_pm72_cpufreq_init(struct device_node *cpus)
> >         g5_cpu_freqs[1].frequency = min_freq;
> >
> >         /* Set callbacks */
> > -       transition_latency = CPUFREQ_ETERNAL;
> >         g5_switch_volt = g5_pfunc_switch_volt;
> >         g5_switch_freq = g5_pfunc_switch_freq;
> >         g5_query_freq = g5_pfunc_query_freq;
> > --
> > 1.8.3.2
> >

Re: [PATCH 2/3] cpufreq: pmac64: re-estimate G5 cpufreq transition latency

[Viresh Kumar]

On 24 July 2013 15:48, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Hi,
>
> On Wed, Jul 24, 2013 at 11:04:50AM +0530, Viresh Kumar wrote:
>> On 24 July 2013 01:54, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
>> > The patch also enables to use ondemand governor on the latter.
>>
>> How? I can't see anything obvious here. :(
>
> It replaces CPUFREQ_ETERNAL with a proper value on older PowerMacs.
> ondemand does not accept CPUFREQ_ETERNAL transition latency.

Ahh.. In case you are sending it again, just add this info in log as people
might miss it.

[PATCH 3/3] cpufreq: pmac64: enable cpufreq on iMac G5 (iSight) model

[Aaro Koskinen]

Enable cpufreq on iMac G5 (iSight) model. Tested with the 2.1 GHz version.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/cpufreq/pmac64-cpufreq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/pmac64-cpufreq.c b/drivers/cpufreq/pmac64-cpufreq.c
index f9e399b..1f352e5 100644
--- a/drivers/cpufreq/pmac64-cpufreq.c
+++ b/drivers/cpufreq/pmac64-cpufreq.c
@@ -399,7 +399,8 @@ static int __init g5_neo2_cpufreq_init(struct device_node *cpus)
 	/* Check supported platforms */
 	if (of_machine_is_compatible("PowerMac8,1") ||
 	    of_machine_is_compatible("PowerMac8,2") ||
-	    of_machine_is_compatible("PowerMac9,1"))
+	    of_machine_is_compatible("PowerMac9,1") ||
+	    of_machine_is_compatible("PowerMac12,1"))
 		use_volts_smu = 1;
 	else if (of_machine_is_compatible("PowerMac11,2"))
 		use_volts_vdnap = 1;
-- 
1.8.3.2

Re: [PATCH 3/3] cpufreq: pmac64: enable cpufreq on iMac G5 (iSight) model

[Viresh Kumar]

On 24 July 2013 01:54, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Enable cpufreq on iMac G5 (iSight) model. Tested with the 2.1 GHz version.
>
> Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> ---
>  drivers/cpufreq/pmac64-cpufreq.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Re: [PATCH 1/3] cpufreq: pmac64: speed up frequency switch

[Rafael J. Wysocki]

On Tuesday, July 23, 2013 11:24:37 PM Aaro Koskinen wrote:
> Some functions on switch path use msleep() which is inaccurate, and
> depends on HZ. With HZ=100 msleep(1) takes actually over ten times longer.
> Using usleep_range() we get more accurate sleeps.
> 
> I measured the "pfunc_slewing_done" polling to take 300us at max (on
> 2.3GHz dual-processor Xserve G5), so using 500us sleep there should
> be fine.
> 
> With the patch, g5_switch_freq() duration drops from ~50ms to ~10ms on
> Xserve with HZ=100.
> 
> Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>

All looks good in the patchset from 10000 feet (or more), but I need Ben to
speak here.

Thanks,
Rafael


> ---
>  drivers/cpufreq/pmac64-cpufreq.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cpufreq/pmac64-cpufreq.c b/drivers/cpufreq/pmac64-cpufreq.c
> index 7ba4234..674807d 100644
> --- a/drivers/cpufreq/pmac64-cpufreq.c
> +++ b/drivers/cpufreq/pmac64-cpufreq.c
> @@ -141,7 +141,7 @@ static void g5_vdnap_switch_volt(int speed_mode)
>  		pmf_call_one(pfunc_vdnap0_complete, &args);
>  		if (done)
>  			break;
> -		msleep(1);
> +		usleep_range(1000, 1000);
>  	}
>  	if (done == 0)
>  		printk(KERN_WARNING "cpufreq: Timeout in clock slewing !\n");
> @@ -240,7 +240,7 @@ static void g5_pfunc_switch_volt(int speed_mode)
>  		if (pfunc_cpu1_volt_low)
>  			pmf_call_one(pfunc_cpu1_volt_low, NULL);
>  	}
> -	msleep(10); /* should be faster , to fix */
> +	usleep_range(10000, 10000); /* should be faster , to fix */
>  }
>  
>  /*
> @@ -285,7 +285,7 @@ static int g5_pfunc_switch_freq(int speed_mode)
>  		pmf_call_one(pfunc_slewing_done, &args);
>  		if (done)
>  			break;
> -		msleep(1);
> +		usleep_range(500, 500);
>  	}
>  	if (done == 0)
>  		printk(KERN_WARNING "cpufreq: Timeout in clock slewing !\n");
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/3] cpufreq: pmac64: speed up frequency switch

[Benjamin Herrenschmidt]

On Tue, 2013-07-23 at 23:20 +0200, Rafael J. Wysocki wrote:
> All looks good in the patchset from 10000 feet (or more), but I need
> Ben to speak here.

I want to give it a quick spin on the HW here, I'll ack then. But yes,
it looks good.

Cheers,
Ben.

Re: [PATCH 1/3] cpufreq: pmac64: speed up frequency switch

[Benjamin Herrenschmidt]

On Wed, 2013-07-24 at 07:14 +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2013-07-23 at 23:20 +0200, Rafael J. Wysocki wrote:
> > All looks good in the patchset from 10000 feet (or more), but I need
> > Ben to speak here.
> 
> I want to give it a quick spin on the HW here, I'll ack then. But yes,
> it looks good.

Seems to work here on the quad G5.

However, If I use on-demand, there's a huge latency of switch as far as
I can tell (about 10s) after I start/stop a bunch of CPU eaters... I
quite like how the userspace "powernowd" which I used to use switches
more aggressively.

Is that expected ?

Cheers,
Ben.

Re: [PATCH 1/3] cpufreq: pmac64: speed up frequency switch

[Aaro Koskinen]

Hi,

On Mon, Aug 12, 2013 at 11:07:48AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2013-07-24 at 07:14 +1000, Benjamin Herrenschmidt wrote:
> > On Tue, 2013-07-23 at 23:20 +0200, Rafael J. Wysocki wrote:
> > > All looks good in the patchset from 10000 feet (or more), but I need
> > > Ben to speak here.
> > 
> > I want to give it a quick spin on the HW here, I'll ack then. But yes,
> > it looks good.
> 
> Seems to work here on the quad G5.
> 
> However, If I use on-demand, there's a huge latency of switch as far as
> I can tell (about 10s) after I start/stop a bunch of CPU eaters... I
> quite like how the userspace "powernowd" which I used to use switches
> more aggressively.
> 
> Is that expected ?

I guess we should keep the current 12us latency in g5_neo2_cpufreq_init()
(although I doubt it's correct...), and only add the new 10ms latency
value to g5_pm72_cpufreq_init() - that way we can enable the older systems
to use ondemand (despite long latencies), while not risking regressing
any of the current functionality.

I'll resend the series with the changes.

A.

Re: [PATCH 1/3] cpufreq: pmac64: speed up frequency switch

[Benjamin Herrenschmidt]

On Thu, 2013-08-15 at 23:10 +0300, Aaro Koskinen wrote:

> I guess we should keep the current 12us latency in g5_neo2_cpufreq_init()
> (although I doubt it's correct...), and only add the new 10ms latency
> value to g5_pm72_cpufreq_init() - that way we can enable the older systems
> to use ondemand (despite long latencies), while not risking regressing
> any of the current functionality.
> 
> I'll resend the series with the changes.

Well, mine is a 11,2, it's not a neo2.

I suppose we could try to measure the latency :-)

Cheers,
Ben.

Re: [PATCH 1/3] cpufreq: pmac64: speed up frequency switch

[Viresh Kumar]

On 24 July 2013 01:54, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Some functions on switch path use msleep() which is inaccurate, and
> depends on HZ. With HZ=100 msleep(1) takes actually over ten times longer.
> Using usleep_range() we get more accurate sleeps.
>
> I measured the "pfunc_slewing_done" polling to take 300us at max (on
> 2.3GHz dual-processor Xserve G5), so using 500us sleep there should
> be fine.
>
> With the patch, g5_switch_freq() duration drops from ~50ms to ~10ms on
> Xserve with HZ=100.
>
> Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> ---
>  drivers/cpufreq/pmac64-cpufreq.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Looks fine to me as well..

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>