On Mon, Nov 25, 2019 at 4:45 AM Daniel Drake <drake@xxxxxxxxxxxx> wrote: > > On Fri, Nov 22, 2019 at 7:15 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote: > > > But then when pci_update_current_state() is called, it reads pmcsr as > > > 3 (D3hot). That's not what I would expect. I guess this means that > > > this platform's _PR3/_PS3 do not actually allow us to put the device > > > into D3cold, > > > > That you can't really say. > > > > Anyway, it is not guaranteed to do that. For example, the power > > resource(s) listed by _PR3 for the device may be referenced by > > something else too which prevents them from being turned off. > > > > > and/or the _PR0/_PS0 transition does not actually transition the device to D0. > > > > Yes. > > > > Which may be the case if the power resource(s) in _PR3 have not been > > turned off really. > > > > [To debug this a bit more, you can enable dynamic debug in > > drivers/acpi/device_pm.c.] > > We checked in an earlier thread before I figured out the timing detail > - these power resources are being turned off at this point. > > > > While there is some ACPI strangeness here, the D3hot vs D3cold thing > > > is perhaps not the most relevant point. If I hack the code to avoid > > > D3cold altogether, just trying to do D0->D3hot->D0, it fails in the > > > same way. > > > > OK, but then you don't really flip the power resource(s), so that only > > means that _PS0 does not restore D0, but in general it only is valid > > to execute _PS0 after _PS3 (if both are present which is the case > > here), so this is not conclusive again. > > _PS0 is called after _PS3 in the above case. > > My feeling is that on this platform, we are not actually entering > D3cold at any point. Linux appears to be powering off the specified > ACPI power domains, but after turning them back on and executing _PS0 > to move to D0initialized, the pmcsr still reporting D3 state seems > highly suspicious to me. Well, it very well may just mean that the device didn't have enough time to get to D0 before reading its PMCSR and it reports a (internally) cached value. > Also, I just experimented adding a pmscr register read to the end of > pci_set_power_state() , after pci_platform_power_transition() has been > called. If the power was truly cut and we're in D3cold then I would > expect this to fail. However the register read succeeds and returns > the same value 0x103. Yes, that is more conclusive. [In theory it may still not have enough time to complete the transition before the read, so you can add a reasonable delay in there and retest, but I don't really expect that to make any difference. :-)] > During resume, Linux seems to have accurately detected this failure to > transition to D3cold in pci_update_current_state() by reading pmcsr > and setting dev->current_state to D3hot accordingly. We then deal with > what looks like a D3hot->D0 transition, which suffers the same failure > as seen when I force Linux to avoid D3cold and actually do a "real" > D0->D3hot->D0 cycle. > > Presumably on a platform where D3cold actually works, after the device > has then been moved to D0uninitialized via ACPI _PS0 and _PR0, > pci_update_current_state() would then read pmcsr and update > dev->current_state to have value D0? Yes, that'd be the expected behavior in that case. > So in terms of the review comment questioning if the function name > quirk_d3_delay() and accompanying message "extending delay after > power-on from D3 to %d msec\n" is good (or whether it should say D3hot > or D3cold), maybe it should say D3hot. That would be more accurate in my view. > Plus a comment noting that D3cold doesn't actually seem to be fully cold on this platform, so > we're actually dealing with a D3hot -> D0 transition? Sounds reasonable to me. Thanks!
