Kernel Bug Tracker – Bug 1349
processor.c using byte IO even if DSDT prescribes otherwise
Last modified: 2004-03-04 12:10:38 UTC
Hardware Environment: Asus M2400N Centrino
Software Environment: Bios 0206; see DSDT in Repository
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
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.
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)
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
hmm, 2.6 test7 has included what you said.
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.