Attention is currently required from: Michael Niewöhner. Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49140 )
Change subject: soc/intel/skylake/acpi: Add PEP table ......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/49140/comment/52e29674_1fd92bce PS3, Line 11:
> Awesome, then at least S0ix states work just fine. > > That error message comes from the kernel SlpS0 self-test. So, let's test something. I assume `cat /sys/power/mem_sleep` has `s2idle` selected? (With s0ix_enable=1 it should. If not, we have another problem.)
Yes. (I think that's based on the FADT)
Right, there is some bit in FADT for that.
> Does your system stay in sleep after executing `echo freeze >/sys/power/state`?
Is the alternative that it wakes immediately? Then yes, it stays in sleep. However, when it does wake, it has always been unlocked. I don't know if that's relevant.
Unlocked vs. locked is user-space and depends on the desktop environment. XFCE has an option for locking before going to sleep but that only works when sleep was initiated from the DE.
That makes sense; I thought it might have been a possibility.
If it wakes immediately with that error, the self-test failed. I'm surprised to see the self-test fail *without* immediate wake, though.... Maybe we have to exclude the SlpS0 state from the table LPIT, when s0ix is not supported on a board :S Could you test this, please? Insert a "return" right before the comment "System (Slp_S0) residency counter" in `src/soc/intel/common/block/acpi/lpit.c` and see if the error disappears.
Based on https://01.org/blogs/qwang59/2020/linux-s0ix-troubleshooting, PC10 without S0ix residency is a possible case. If I understand correctly, that situation would be what I see with this board: No S0ix/failed self-test, but PC10 residency/no immediate wake. I will attempt to troubleshoot that sometime by disabling devices, I suppose, following that link.
I'm also not certain what you mean. If S0ix is disabled, would LPIT factor? Besides, if S0ix does relate to hardware state, but not board-design, shouldn't LPIT always advertise both PC10 residency and system residency to warn about broken S0ix support for a board?
Well, yes PC10 without S0ix is possible. s0ix_enable means means "PC10 and/or SlpS0 possible". Note that SlpS0 is a substate of s0ix but s0ix != SlpS0.
The LPIT table contains two entries, one for PC10 (the first one) and SlpS0 (the second one. Based on these entries, the OS decides what *actually* is possible. When PC10 and SlpS0 is advertised but SlpS0 doesn't work, that error triggers. So, I want to test what happens, when SlpS0 is not advertised, because it doesn't apply to your board.
I see the same error.
Strange....
I can double-check.
Additionally, I noticed while looking over the vendor firmware's LPIT table that it defines the PC10 subtable twice, if I'm reading it correctly (two copies of address 0x632 as "FunctionalFixedHW" - the PKG_C10_RESIDENCY MSR?). Even so, the kernel warns that the system was unable to enter PC10.
Oh yeah, I've seen that too somewhere but I doubt that's correct.
So, while I was initially surprised to hear that SlpS0 might not apply to my board, perhaps that is the case. I can always try to troubleshoot some other time.
But regardless: `return` or `return current`? The PC10 residency counter portion does increment "current"
Yes, you're right, it should be `return current` ofc :-)
Was this only for testing, or do you think that the S0ix subtable should only be advertised if S0ix is known to work?
Very good question. S0ix isn't documented very good but I wouldn't advertise something that isn't usable. However, that will require us to introduce a dt option `slps0_supported`. That's already planned but I currently have higher-prio stuff. I'll come back to that asap. If you're curious, have a look on my WIP patches on s0ix.
Since it doesn't seem to make a difference irt that error/warning, let's keep it as-is for now and resolve that as soon as I get back to these patches. Wdyt?
The warning is informative and not harmful, so that's fine by me.
If S0ix can be temperamental, I think that the OS should always be able to warn about errors.
Well, when the board does not support SlpS0 (= SlpS0 is unconnected) I wouldn't want the OS to warn about it. That warning/error is triggered by the S0ix self-test, which may also be buggy btw.
Ah. If the pad being disconnected means that lower S0ix states aren't possible, then there might not be anything to debug.
Sorry it took me so long to get back to you.
No problem :-)
So... last question - I hope I didn't ask that already, but I haven't seen it above. Is this patch the cause for that error message or does the error message appear even without it?
The error is produced by the intel_pmc_core driver, which is only loaded if the PEP table is present?
-
Benjamin Doron (Code Review)