Bug 7859
Summary: | Frequency can't switch on a dual Turion 64 | ||
---|---|---|---|
Product: | Power Management | Reporter: | Romain Li (roms) |
Component: | cpufreq | Assignee: | Thomas Renninger (trenn) |
Status: | CLOSED PATCH_ALREADY_AVAILABLE | ||
Severity: | normal | CC: | trenn |
Priority: | P2 | ||
Hardware: | i386 | ||
OS: | Linux | ||
Kernel Version: | 2.6.19.2 | Subsystem: | |
Regression: | --- | Bisected commit-id: | |
Attachments: |
dmesg output
Values reported by sysfs (and can't be changed!) Output reported by cpufreq-info ACPI dump (BIOS v1.0a) That one should fix it: Set cpufreq limit for all affected/managed cpus dmesg output (PPC - ppc: 0, cpuid 0, policy_cpu: 0 line) dmesg log after marc's patch dmesg output with patch #6 and AC insertion/removal ACPI dump (bios v1.1d) |
Description
Romain Li
2007-01-21 07:53:32 UTC
Created attachment 10143 [details]
dmesg output
Created attachment 10144 [details]
Values reported by sysfs (and can't be changed!)
Created attachment 10145 [details]
Output reported by cpufreq-info
Created attachment 10146 [details]
ACPI dump (BIOS v1.0a)
Created attachment 10149 [details]
That one should fix it: Set cpufreq limit for all affected/managed cpus
What I found: your BIOS only uses Notify (\_PR.CPU0, 0x80) calls, but does not inform the second CPU about changes (something like Notify (\_PR.CPU1, 0x80)). In drivers/cpufreq/cpufreq.c:cpufreq_add_dev() a new policy object is allocated, also for a mangaged CPU. Therefore I think the attached patch is needed. The _PPC function is used to tell the kernel which frequencies are allowed. A value of zero means all freqs are allowed, a value of one means the highest freq is not allowed (only 800 in your case). Your BIOS initialses the _PPC value (XPPC variable) with 1 and it could be that _PPC must not be invoked at cpufreq initialisation time. To figure that out you should add a line like that at the end of drivers/cpufreq/processor_perflib.c: acpi_processor_ppc_notifier() (Best before the verify call, if you use my patch also place this line into the for_each_cpu_mask loop): printk ("PPC - ppc: %d, cpuid: %d, policy_cpu: %d\n", ppc, pr->id, policy->cpu); If _PPC must not get invoked at initialisation time (if you have only the ppc call once at module load time and it's always 1), this patch is also needed: http://marc.theaimsgroup.com/?l=linux-acpi&m=116900368102272&w=2 I have added your printk line at the end of acpi_processor_ppc_notifier() but before the verify call. On the other hand, I forgot to add it in the for_each_cpu_mask loop. I have the call once at module load time (see cpufreq2.dmesg). As stated in your mail, I have added the marc's patch and I'm happy to tell you that's working. I have changed governor twice as you may see in the cpufreq3.dmesg log. As a final check, I will try and build a UP kernel. More report later. Created attachment 10153 [details]
dmesg output (PPC - ppc: 0, cpuid 0, policy_cpu: 0 line)
Created attachment 10154 [details]
dmesg log after marc's patch
I build a UniProcessor kernel: frequency can't switch. I need both of your patches. You are telling that my ACPI tables may be broken... Should I fix them? Do you want I merge the two patches into a single one? Should I submit them on lkml? Do you think that your patches can be accepted for merge in the current Linux kernel tree? Romain. dmesg output (PPC - ppc: 0, cpuid 0, policy_cpu: 0 line) ppc: 0 means you have to reach all freqs. Something is going wrong (or is this with the patch that makes all freqs reachable applied?). The patch I pointed to was for a machine that wrongly returns ppc: 2 (in your case it should be 1) on startup. > You are telling that my ACPI tables may be broken... Should I fix them? Not sure, I am a bit confused now... I expected a line like: dmesg output (PPC - ppc: 1, cpuid 0, policy_cpu: 0 line) In this case it is a BIOS problem (depends on how you read the specs, see the patch description from comment #6). The patch from comment #6 is the right fix and should find it's way mainline through Len, maybe it's worth also adding it to 2.6.19.3. I assume that above line comes from the patched kernel, the patch from comment #6 gets mainline soon, everything is fine and I am closing this one now. After digging a bit more... my patch from comment #5 should not be needed as CPU1's policy never gets used. To verify you can add the printk that produced this line dmesg output (PPC - ppc: 0, cpuid 0, policy_cpu: 0 line) and make sure cpufreq is broken, there should be ppc: 1. And/Or what would be really great: you can search BIOS configs for a Powermanagement/PowerNow! config to let the machine run on lower freq when working on battery if your BIOS provides that. Be sure also AC/battery module is loaded and unplug/replug AC adapter and verify that max freq gets lowered on battery and is increased again when working on AC. Reopen if you think something is still broken. I have taken a look at the Chapter 9 (Power and Thermal Management) of the "BIOS and Kernel Developer's Guide for the AMD Athlon 64 and AMD Opteron Processors" and they are telling this: << 9.5.6.3 Dual Core P-state Transition Requirements For dual core processors, it is the processor driver 1) I can confirm that patch from comment #5 is not needed. 2) I can't config CPU from BIOS because my BIOS is very ... light! 3) When I remove AC power, max freq is lowered. But, when I set AC power again, freq is still low :-( I have attached dmesg output with your 'printk' lines. Created attachment 10165 [details]
dmesg output with patch #6 and AC insertion/removal
Add-on: I can manually change governors and frequencies without any problems (ppc = 0). But switching AC power supply raises ppc to 1 and it is never reset to 0. Can you do: cat /proc/acpi/event (you probably need to stop acpid, or whatever process access this file, you can chech with fuser /proc/acpi/event). The cat should block. If you (un)plug the AC adapter you should see ACPI events popping up. Is there also a processor (0x80 should be the last of the values)? If yes, there might still be a bug in the kernel (related to linked Dual Core cpus?) Also adding a printk add at end of: acpi_processor_get_platform_limit() in drivers/acpi/processor_perflib.c showing the ppc value and the CPU that gets read when a CPU event happens, could be helpful. If not, modifing/debugging the DSDT is the next stepp I'd take. The question then is why "Notify (\_PR.CPU0, 0x80)" is not invoked in DSDT on a AC change. This should happen in "Method (_L10, 0, NotSerialized)", the only place where ACON variable is modified at runtime and where "Notifies" are thrown (conditionally, depending on some variables...). To test whether CPU notification and kernel cpufreq works in general you could modify your DSDT like that: in method _PSR of device ACDC add two lines: --- kernel-7859_Turion_PPC_limited.orig/DSDT.modif.dsl +++ kernel-7859_Turion_PPC_limited/DSDT.modif.dsl @@ -4062,6 +4062,8 @@ DefinitionBlock ("DSDT.aml", "DSDT", 1, Acquire (PSMX, 0xFFFF) And (\_SB.PCI0.LPC0.PMRD (0xA0), 0x01, Local0) Store (Local0, ACFG) + Store (Local0, ACON) + Notify (\_PR.CPU0, 0x80) Release (PSMX) Return (ACFG) To be able to do that follow up following steps: - get the newest iasl tools (not sure which ubuntu package this is, you may want to ask Matthew Garrett <mjg59@srcf.ucam.org> (AFAIK he packaged it). - acpidump > acpidump - acpixtract acpidump (on old versions cat acpidump |acpixtract) - iasl -d DSDT.dat (disassemble the DSDT table) - Modify the outcomig DSDT.dsl as shown above - iasl -ta DSDT.dsl (recompile the DSDT table including your changes, IIRC it's the -ta option and you must use the .hex file popping out) - modify your kernel's compile option (make config, make menuconfig or whatever, whatch out for: CONFIG_ACPI_CUSTOM_DSDT=y must be set, ACPI_CUSTOM_DSDT_FILE must point to your newly created DSDT.hex file - recompile and boot the kernel - when booted, dmesg should show something that DSDT got overridden Are all freqs available if you unplug AC? If you don't even get an AC ACPI event, do a cat /proc/acpi/ac_adapter/state, then above added code (update ACON and throw CPU event) should get processed. It's also very useful (you should always compile with ACPI_DEBUG=y) to additional debug statements inside the DSDT. You might want to add lines like that: Store (ACON, debug) (replace ACON with whatever variable's value you like to debug) Clustering the _L10 method (the conditions where ACON and Notify (..CPU0, 0x80) are involved) would make sense and find out whether ACON did not get set correctly or why the Notify (..CPU0, 0x80) did not get invoked. You should only modify the DSDT for debug purposes. You might want to goolge a bit, there should be more info around. About first step, I get some events (BIOS v1.0a): AC off: processor CPU0 00000080 00000001 ac_adapter ACAD 00000080 00000000 processor CPU0 00000081 00000000 ac_adapter ACAD 00000080 00000000 battery BAT0 00000080 00000001 AC on: processor CPU0 00000080 00000001 ac_adapter ACAD 00000080 00000001 processor CPU0 00000081 00000000 ac_adapter ACAD 00000080 00000001 battery BAT0 00000080 00000001 I have been on my vendor site to find a BIOS update. They have just released (a big) new one: v1.1d. This seems to be a major release but the changelog is almost empty. Anyways, I tried it: cpufreq seems to work fine and fan, too. ACPI events are not the same for now: AC off: processor CPU0 00000080 00000001 ac_adapter ACAD 00000080 00000000 processor CPU0 00000081 00000000 AC on: processor CPU0 00000080 00000000 ac_adapter ACAD 00000080 00000001 processor CPU0 00000081 00000000 Maybe I shouldn't have updated BIOS for testing consistency: I have downgraded my BIOS to original version (1.0a) => cpufreq is now fixed to 800MHz; patch does not work any more (ppc=1, always)?! I don't know whether I should do other tests you proposed. Please tell me... Otherwise, I have attached DSDT tables of new BIOS for comparison (if any interest). Created attachment 10191 [details]
ACPI dump (bios v1.1d)
I have noticed a weird thing: ACPI dumps are the same (diff returns no ouput) between 2 BIOS. Thus, tables did not change?! Patch from comment #6 works for me. Frequency gets limited on battery and unlimited on AC as probably intended by BIOS. |