| Message ID | 1374611079-28091-1-git-send-email-aaro.koskinen@iki.fi (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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.
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"); >
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>
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.
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.
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.
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");
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(-)