aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDominik Brodowski <linux@dominikbrodowski.net>2009-08-07 16:58:51 -0400
committerDave Jones <davej@redhat.com>2009-09-01 12:45:08 -0400
commitce6c3997c2fce74d12e6d8887a1d8cdf024fa850 (patch)
tree047d8577b12e9c5dbcf147f4a76bca537d775b4a
parent37d0892c5a94e208cf863e3b7bac014edee4346d (diff)
[CPUFREQ] Re-enable cpufreq suspend and resume code
Commit 4bc5d3413503 is broken and causes regressions: (1) cpufreq_driver->resume() and ->suspend() were only called on __powerpc__, but you could set them on all architectures. In fact, ->resume() was defined and used before the PPC-related commit 42d4dc3f4e1e complained about in 4bc5d3413503. (2) Therfore, the resume functions in acpi_cpufreq and speedstep-smi would never be called. (3) This means speedstep-smi would be unusuable after suspend or resume. The _real_ problem was calling cpufreq_driver->get() with interrupts off, but it re-enabling interrupts on some platforms. Why is ->get() necessary? Some systems like to change the CPU frequency behind our back, especially during BIOS-intensive operations like suspend or resume. If such systems also use a CPU frequency-dependant timing loop, delays might be off by large factors. Therefore, we need to ascertain as soon as possible that the CPU frequency is indeed at the speed we think it is. You can do this two ways: either setting it anew, or trying to get it. The latter is what was done, the former also has the same IRQ issue. So, let's try something different: defer the checking to after interrupts are re-enabled, by calling cpufreq_update_policy() (via schedule_work()). Timings may be off until this later stage, so let's watch out for resume regressions caused by the deferred handling of frequency changes behind the kernel's back. Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net> Signed-off-by: Dave Jones <davej@redhat.com>
-rw-r--r--drivers/cpufreq/cpufreq.c95
1 files changed, 7 insertions, 88 deletions
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index fd69086d08d5..2968ed6a9c49 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1250,20 +1250,11 @@ static int cpufreq_suspend(struct sys_device *sysdev, pm_message_t pmsg)
1250{ 1250{
1251 int ret = 0; 1251 int ret = 0;
1252 1252
1253#ifdef __powerpc__
1254 int cpu = sysdev->id; 1253 int cpu = sysdev->id;
1255 unsigned int cur_freq = 0;
1256 struct cpufreq_policy *cpu_policy; 1254 struct cpufreq_policy *cpu_policy;
1257 1255
1258 dprintk("suspending cpu %u\n", cpu); 1256 dprintk("suspending cpu %u\n", cpu);
1259 1257
1260 /*
1261 * This whole bogosity is here because Powerbooks are made of fail.
1262 * No sane platform should need any of the code below to be run.
1263 * (it's entirely the wrong thing to do, as driver->get may
1264 * reenable interrupts on some architectures).
1265 */
1266
1267 if (!cpu_online(cpu)) 1258 if (!cpu_online(cpu))
1268 return 0; 1259 return 0;
1269 1260
@@ -1282,47 +1273,13 @@ static int cpufreq_suspend(struct sys_device *sysdev, pm_message_t pmsg)
1282 1273
1283 if (cpufreq_driver->suspend) { 1274 if (cpufreq_driver->suspend) {
1284 ret = cpufreq_driver->suspend(cpu_policy, pmsg); 1275 ret = cpufreq_driver->suspend(cpu_policy, pmsg);
1285 if (ret) { 1276 if (ret)
1286 printk(KERN_ERR "cpufreq: suspend failed in ->suspend " 1277 printk(KERN_ERR "cpufreq: suspend failed in ->suspend "
1287 "step on CPU %u\n", cpu_policy->cpu); 1278 "step on CPU %u\n", cpu_policy->cpu);
1288 goto out;
1289 }
1290 }
1291
1292 if (cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)
1293 goto out;
1294
1295 if (cpufreq_driver->get)
1296 cur_freq = cpufreq_driver->get(cpu_policy->cpu);
1297
1298 if (!cur_freq || !cpu_policy->cur) {
1299 printk(KERN_ERR "cpufreq: suspend failed to assert current "
1300 "frequency is what timing core thinks it is.\n");
1301 goto out;
1302 }
1303
1304 if (unlikely(cur_freq != cpu_policy->cur)) {
1305 struct cpufreq_freqs freqs;
1306
1307 if (!(cpufreq_driver->flags & CPUFREQ_PM_NO_WARN))
1308 dprintk("Warning: CPU frequency is %u, "
1309 "cpufreq assumed %u kHz.\n",
1310 cur_freq, cpu_policy->cur);
1311
1312 freqs.cpu = cpu;
1313 freqs.old = cpu_policy->cur;
1314 freqs.new = cur_freq;
1315
1316 srcu_notifier_call_chain(&cpufreq_transition_notifier_list,
1317 CPUFREQ_SUSPENDCHANGE, &freqs);
1318 adjust_jiffies(CPUFREQ_SUSPENDCHANGE, &freqs);
1319
1320 cpu_policy->cur = cur_freq;
1321 } 1279 }
1322 1280
1323out: 1281out:
1324 cpufreq_cpu_put(cpu_policy); 1282 cpufreq_cpu_put(cpu_policy);
1325#endif /* __powerpc__ */
1326 return ret; 1283 return ret;
1327} 1284}
1328 1285
@@ -1330,24 +1287,21 @@ out:
1330 * cpufreq_resume - restore proper CPU frequency handling after resume 1287 * cpufreq_resume - restore proper CPU frequency handling after resume
1331 * 1288 *
1332 * 1.) resume CPUfreq hardware support (cpufreq_driver->resume()) 1289 * 1.) resume CPUfreq hardware support (cpufreq_driver->resume())
1333 * 2.) if ->target and !CPUFREQ_CONST_LOOPS: verify we're in sync 1290 * 2.) schedule call cpufreq_update_policy() ASAP as interrupts are
1334 * 3.) schedule call cpufreq_update_policy() ASAP as interrupts are 1291 * restored. It will verify that the current freq is in sync with
1335 * restored. 1292 * what we believe it to be. This is a bit later than when it
1293 * should be, but nonethteless it's better than calling
1294 * cpufreq_driver->get() here which might re-enable interrupts...
1336 */ 1295 */
1337static int cpufreq_resume(struct sys_device *sysdev) 1296static int cpufreq_resume(struct sys_device *sysdev)
1338{ 1297{
1339 int ret = 0; 1298 int ret = 0;
1340 1299
1341#ifdef __powerpc__
1342 int cpu = sysdev->id; 1300 int cpu = sysdev->id;
1343 struct cpufreq_policy *cpu_policy; 1301 struct cpufreq_policy *cpu_policy;
1344 1302
1345 dprintk("resuming cpu %u\n", cpu); 1303 dprintk("resuming cpu %u\n", cpu);
1346 1304
1347 /* As with the ->suspend method, all the code below is
1348 * only necessary because Powerbooks suck.
1349 * See commit 42d4dc3f4e1e for jokes. */
1350
1351 if (!cpu_online(cpu)) 1305 if (!cpu_online(cpu))
1352 return 0; 1306 return 0;
1353 1307
@@ -1373,45 +1327,10 @@ static int cpufreq_resume(struct sys_device *sysdev)
1373 } 1327 }
1374 } 1328 }
1375 1329
1376 if (!(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) {
1377 unsigned int cur_freq = 0;
1378
1379 if (cpufreq_driver->get)
1380 cur_freq = cpufreq_driver->get(cpu_policy->cpu);
1381
1382 if (!cur_freq || !cpu_policy->cur) {
1383 printk(KERN_ERR "cpufreq: resume failed to assert "
1384 "current frequency is what timing core "
1385 "thinks it is.\n");
1386 goto out;
1387 }
1388
1389 if (unlikely(cur_freq != cpu_policy->cur)) {
1390 struct cpufreq_freqs freqs;
1391
1392 if (!(cpufreq_driver->flags & CPUFREQ_PM_NO_WARN))
1393 dprintk("Warning: CPU frequency "
1394 "is %u, cpufreq assumed %u kHz.\n",
1395 cur_freq, cpu_policy->cur);
1396
1397 freqs.cpu = cpu;
1398 freqs.old = cpu_policy->cur;
1399 freqs.new = cur_freq;
1400
1401 srcu_notifier_call_chain(
1402 &cpufreq_transition_notifier_list,
1403 CPUFREQ_RESUMECHANGE, &freqs);
1404 adjust_jiffies(CPUFREQ_RESUMECHANGE, &freqs);
1405
1406 cpu_policy->cur = cur_freq;
1407 }
1408 }
1409
1410out:
1411 schedule_work(&cpu_policy->update); 1330 schedule_work(&cpu_policy->update);
1331
1412fail: 1332fail:
1413 cpufreq_cpu_put(cpu_policy); 1333 cpufreq_cpu_put(cpu_policy);
1414#endif /* __powerpc__ */
1415 return ret; 1334 return ret;
1416} 1335}
1417 1336