Bug 42870
| Summary: | [BISECTED]acpi_processor fails to recognize C4 after NOTIFY_POWER - Thinkpad T43 | ||
|---|---|---|---|
| Product: | ACPI | Reporter: | Andreas Müller (andreas) |
| Component: | Power-Processor | Assignee: | Len Brown (lenb) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | CC: | acpi-bugzilla, alan, florian, lenb, thomas.schlichter |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 3.2 | Subsystem: | |
| Regression: | Yes | Bisected commit-id: | |
| Attachments: |
dmesg with attached /proc/cpuinfo
dmidecode setup state-info for new surfacing states |
||
please attach the output from dmidecode. I should be able to get access to a t43 in a few weeks and when i try to re-produce I'll want to assure we are both running the latest BIOS. Created attachment 72546 [details]
dmidecode
I am running the latest EC-firmware (1YHT29WW-1.06) and latest BIOS (1YET65WW) for the T43 (2669).
Created attachment 75371 [details]
setup state-info for new surfacing states
The problem seems to be with 46bcfad7a819bd17ac4e831b04405152d59784ab, a rather large modification.
acpi_processor_cst_has_changed() calls acpi_processor_setup_cpuidle_states() before setting up "pr->power.states[]", which would happen in acpi_processor_get_power_info(). So, this array only contains the old valid states. Eventhough get_power_info() will be called right after and updates the processors data, the driver's (cpuidle_driver) information about the states will not be updated.
Furthermore, the cpuidle_driver sets the states[i].power_usage only when it is registered. There is no update, so power_usage of the surfaced state would be zero and is therefore not considered.
In this patch, acpi_processor_get_power_info() will be called for processor 0 right before the cpuidle-driver's state info will be updated.
Furthermore, the states[].power_usage variable will be set as cpuidle_driver would do, it might be more consistent to let it do it by itself but for now it doesn't hurt (me :).
There still seems to be a problem with accounting time in C0 after cst_has_changed. But this seems to be that way for some time, unrelated to this patch.
*** Bug 43349 has been marked as a duplicate of this bug. *** Hi Andreas, your patch also fixes my problem, described in bug 43349. Have you considered publishing the patch at the linux kernel mailing list? Maybe this would rise the chance to get this fixed in mainline... Kind regards, Thomas P.S.: Is the last hunk of your patch really neccessary? Hi Thomas, I thought this would be the appropriate place. Although the patch fixes a problem, there might be a problem in future. At first with cpuidle. As far as I can see, one shouldn't mess with state->power_usage outside of cpuidle without setting the state->power_specified bit. So either cpuidle or (acpi) processor_idle should set/update those exclusively. Furthermore the states are only initialized from cpu-0. Is it possible that different cpus may have different states (types, names etc.)? It seems to be intended that this is not the case and to be this way. When new cstates arise only for some cpu (whereas it would be only called if cpu-0 states would change too), that might be a problem. On the other hand that could be nonsense and not the problem of that specific bug. ;) The last hunk in the patch shouldn't really be necessary, but the call of acpi_processor_get_power_info() for processor 0 (the initial pr will always be processor 0) is superfluous. On that different states issue, I just looked that up the acpi-specs 5.0: FADT/P_BLK: "Processor power state support is symmetric when presented via the FADT and P_BLK interfaces; OSPM assumes all processorts in a system support the same power sta tes. If processors have non-symmetric power state support, then the BIOS will ch oose and use the lowest common power states supported by all the processors in t he system through the FADT table." (p. 386) _CST: "With _CST each processor can have its own characteristics independent of other processors. For example processor 0 can support C1, C2 and C3 while processor 1 supports only C1." (p.396) That doesn't seem to be implemented for _CST. However, the initialization is also only done for one cpu, as the update would do with this patch. Each processor has its own correct state-information ((cpuidle_device) but cpuidle (acpi_idle_driver) might not. It doesn't seem to hurt current systems though. Hi Andreas, regarding your concerns about modifying state->power_usage outside of cpuidle, I think I might have cleaner patches attached to bug 43349. Maybe you'd like to have a look at them (or even test them)? And I think the other issue (different C-states on different CPUs) is indeed an other bug. That issue is also not a regression, as I believe the situation was not better in the past. Kind regards, Thomas A patch referencing this bug report has been merged in Linux v3.8-rc4: commit 8aef33a7cf40ca9da188e8578b2abe7267a38c52 Author: Daniel Lezcano <daniel.lezcano@linaro.org> Date: Tue Jan 15 14:18:04 2013 +0100 cpuidle: remove the power_specified field in the driver |
Created attachment 72542 [details] dmesg with attached /proc/cpuinfo I observed that acpi_idle won't utilize the C4-state after unplugging the laptop from power. However, after sending it into standby, it will be able to use it properly. Maybe the state-data is not properly initialized by the processor, as it doesn't seem to fiddle with (only reads) the data after the notification ACPI_PROCESSOR_NOTIFY_POWER has been received. (But I have not expertise here, I have no idea where to look further.) Still, linux recognizes that there is another state, but cannot make use of it. Some information from /sys/devices/system/cpu/cpu0/cpuidle/state4/*: Before unplugging, there is no "state4". after unplugging: desc: <null> latency: 185 name: <null> power: 0 after standby while unplugged (or booted up while unplugged): desc: ACPI IOPORT 0x1016 latency: 185 name: C4 power: 0 I am using a IBM Thinkpad T43 (2669Y3N) with a Pentium-M (Donthan 740) running a vanilla kernel 3.2.9, I attached /proc/cpuinfo and a dmesg. Please let me know what else information I could supply or try out.