Date: Tue, 19 Feb 2008 21:25:19 +0200 From: Andriy Gapon <avg@icyb.net.ua> To: Nate Lawson <nate@root.org> Cc: freebsd-acpi@freebsd.org Subject: Re: acpi_throttle: quirk based on pci info Message-ID: <47BB2D1F.8000008@icyb.net.ua> In-Reply-To: <47BAFB27.1050509@root.org> References: <47B96989.6070008@icyb.net.ua> <98F7E48C-1CBF-494D-8411-D80E25247214@FreeBSD.org> <47BAF97A.80405@icyb.net.ua> <47BAFB27.1050509@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
on 19/02/2008 17:52 Nate Lawson said the following: > Andriy Gapon wrote: >> on 19/02/2008 01:00 Rui Paulo said the following: >>> On Feb 18, 2008, at 11:18 AM, Andriy Gapon wrote: >>> >>>> While looking for something else I accidentally noticed that >>>> acpi_throttle has one quirk for some early revisions of PIIX4 chipset >>>> and the quirk is enabled based on PCI info. >>>> I have a newer revision of PIIX4 so the quirk is not applicable in >>>> my case. >>>> >>>> Nevertheless I noticed that acpi_throttle is initialized before PCI >>>> bus >>>> driver, so when it calls pci_find_device() it always returns NULL and >>>> quirk is not applied. At least this is what I see in dmesg on my >>>> machine. >>> I run into a similar problem on my SoC MacBook project and I ended up >>> using the SMI vendor strings because it was too early in boot in order >>> to find the PCI devices. The problem here is very similar. >>> Maybe we should try to use SMI vendor strings instead of >>> pci_find_device()? >> I am not familiar with this approach and I am not sure if that works >> with that type of (quite old) hardware. >> >> When I worked on something else I remember somebody (maybe Nate) >> recommending me to model my code after ichss: >> sys/dev/cpufreq/ichss.c >> >> Maybe the same approach could be used here? >> >> I.e. no identify method for acpi bus. >> Additional device/driver for pci bus. >> pci probe method checks for duplicates and adds the acpi device as a >> child to acpi bus. >> pci probe method sets quirks based on pci info. >> pci probe method runs device_probe_and_attach on the acpi device. >> as a consequence acpi probe and attach (for successful probe) are executed. >> >> The only unclear issue is where to place the code that is currently in >> (acpi) identify method - should it go to pci identify method or should >> it go to acpi probe method. > > Yeah, it's a problem either way. I remember something like: > > 1. Separate PCI attachment that adds ACPI device > Guaranteed that PCI is available. Bad: won't work if loaded after boot > (not probed) > > 2. Directly calling device_probe() > Good: works after boot. Bad: always adds a device even if none present. > > You can see the first one in ichss.c. It's why cpufreq devices are not > loadable. > As I understand, currently acpi_thermal is always a part of "acpi", not a separate module. And acpi should either be built into kernel or pre-loaded at boot time. So, it seems that we don't have to worry about the issue that you are mentioning in this case, at least for now. If I have time I'll try to write a patch some time later. I have a bit of a practical interest in it: I have a system based on PIIX4E (as you already know). Its ACPI FADT specifies duty offset of 1 and duty width of 1. So, according to FADT only 50% throttling is available. On the other hand, chipset specification updates for PIIX4E and PIIX4M say that PCNTRL (P_CNT) register has 3 bits (at offset 1 indeed) for controlling throttling in 12.5% steps. So it seems, that acpi_throttle controls only the lowest bit of the three; moreover, it seems that BIOS assigns some initial value to those bits. So when throttling is active those bits do not have 001 value corresponding to 87.5% duty cycle, but have some XX1 value and that value seems to be 111 (12.5% duty cycle), because the system becomes extremely slow. So, as experiment, I hardcoded duty width to 3 and everything works great - I have 8 throttling steps and system performance seems to really correspond to expected duty levels. So now I am thinking about writing a proper patch for this (positive) quirk. Reference: http://www.intel.com/design/chipsets/specupdt/29773817.pdf -- Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?47BB2D1F.8000008>