Messages in this thread | | | From | Jesse Barnes <> | Subject | Re: PCI PM: Restore standard config registers of all devices early | Date | Tue, 3 Feb 2009 08:33:07 -0800 |
| |
On Monday, February 2, 2009 10:07 pm Benjamin Herrenschmidt wrote: > On Mon, 2009-02-02 at 16:28 -0800, Linus Torvalds wrote: > > On Tue, 3 Feb 2009, Rafael J. Wysocki wrote: > > > BTW, on the PMAC the problematic driver appears to be the radeon > > > driver, according to Ben, and the breakage is not related to USB. > > > > Hmm. atyfb_base.c has the same kind of things with magic PMAC code, but > > it doesn't follow the USB pattern - it just replaces > > "pci_set_power_state()" _entirely_. > > > > It seems a very interesting suspend routine, btw. It doesn't seem to do > > any of the pci_save_state() at all, just re-initializes from scratch. > > Maybe it is unhappy with the PM layer deciding to try to restore state > > since it clearly didn't.. > > In fact, atyfb is also busted in -rc3 for different reasons though > probably by the same patch. I just dug an old powerbook with a mach64 > and it crashes on resume with a machine check in there. (Among 2 or 3 > other problems introduced by recent kernels, such as pci_get_* now > kmallocs with GFP_KERNEL internally which makes it WARN when I use it in > my low level suspend/resume code to whack the memory controller, etc... > this one is going to bite others I reckon, or IDE having some interrupt > problems on resume). > > Adding a pci_save_state() to atyfb_pci_suspend() and a > pci_set_power_state() + pci_restore_state() at the beginning of > atyfb_pci_resume() fixes the machine check here. > > Now where it gets funny is that I've added code to read the BAR and > command register content before, between, and after those calls and > print it and .. they are sane... Until i discovered that what happens is > that the new generic code seems to actually blast 0 all over my config > space if I don't call pci_save_state() in suspend(). I suppose I was > missing a "mandatory" call here... but the core should be more robust, > ie it shouldn't erase the config space of something because a driver > "forgot" to call pci_save_state() !
Whoa, I don't think we actually zero the contents in the suspend/resume core, but if the device goes into D3 the config space contents may be lost, maybe that's what happening here?
-- Jesse Barnes, Intel Open Source Technology Center
| |