Distribution: Debian Hardware Environment: Asus M2400N Centrino Software Environment: Bios 0206; see DSDT in Repository Problem Description: ACPI Processor Performance level setting does not work on this Notebook. The reason for this is that processor.c uses outb() byte writes to write to the power control register, even tho the DSDT prescribes otherwise (see PPCT Ressource template in the DSDT, the register has a length of 0x10, and the value is a 16bit value too) Changing the u8 typecasts to u16 and putting in an outw() - et voila: ACPI performance level setting starts to work. The propper fix would probably be to do away with outb() completely and use proper ACPI generic adresses. This would also accomodate the alternate PCT/PSS Methods from the DSDT which use the Centrino MSR directly, and no IO at all. That in turn does depend uppon support of FFixedHW Address Types. Is this planned ? I furthermore do wonder if this is somehow related to #1269, after all, its about the same (0x82) register. What I also noticed: switching the performance level does not update /proc/cpuinfo (using speedstep-centrino patches, it does), and therefore internal clock/timers could possibly be wrong after a performance level change ? Steps to reproduce: load processor.o on a M2400N. Observe processsor performance switching does not work. Patch processor.c replacing the outb() with outw() and expanding the value cast from u8 to u16. recompile insert module observe ACPI processor performance switching does work now.
did you mean outb in cpufreq/acpi.c(I use 2.6.0-test5). Yes, it's a bug. would like make a patch and attach here?
*** Bug 1348 has been marked as a duplicate of this bug. ***
No. I am talking about drivers/acpi/processor.c (in 2.4 kernels; I do not play with 2.6 yet) in acpi_processor_set_performance() where "value" is first declared u8, then fetched from the ACPI information, and finally outb() to the performance control register. And the DSDT for the M2400N says that this register is 0x10 bits in length (ResourceTemplate in the PCT)
yep, we are talking about the same thing. so would you like make a patch for it?
hmm, 2.6 test7 has included what you said.
*OUCH* a) I specifically refer to 2.4, I told you I am not running 2.6 (yet) b) Just to verify your claim I just downloaded 2.6.0-test8 to find that indeed, instead of outb(), now outw() is used. *THIS IS WRONG*. As wrong as outb() was. It needs to decide according to the DSDT wheter to use outb() or outw(). I have seen both DSDTs with 8 as well as 16 bit registers. Please note that it is not mandated that the performance control register is in systemIO address space at all, so any out* is wrong. c) I tried writing a proper patch, using acpi_hw_low_level_write but ran into module symbol export problems. Now I am not that much a kernel developer that I know this stuff. It probably would take me a couple hours to figure out, but I do not currently have that time.
sorry to bother you. we have noticed the same problem. I just told venki (Venkatesh.Pallipadi@intel.com) about it. He will take a look at it. I guess general register will be supported.
Created attachment 1164 [details] patch for the error venki said we don't need the complete GAS details in acpi processor performance struct, because the complete GAS structure will not be useful in MSR case anyway. so I just proposed a patch to support byte, word, or dword IO.
Yes, while not yet having tested this code, it definitly looks better. However, please take another look at the DSDT for the M2400N, 0206 (out of the repository on acpi.sourceforge.net) into the definition of _PCT. As you can see, it is (albeit here its optional) fully legal, and (obviously, as this is living proof) it happens in the wild, that the Performance Control Register does not even live in the SystemIO space (FFixedHW here). In which case the code will break again. I thus assume General ACPI register access is the way to go here. (Yes, I know there is no FFixedHW support there yet, anyway) Could you thus reconsider, or consider evaluating _PCT from the ACPI Tables at runtime ? (I honestly do not know how much overhead that incurs) The tradeof of that vs. a few bytes in the processor performance struct should be considered.
FFixedHW means the register is dependent on CPU. In pentium-M, there are MSRs for it. So, we don't think it's necessary to use GAS. The separate patch for this feature is on it's way.
Created attachment 1273 [details] patch against 2.6
shipped in 2.6.2-rc3 -- closing.