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/b7981edf_8abced98 PS3, Line 11:
Not needed. SlpS0 is not asserted externally but by the SoC.
And the pad doesn't need to be `_NF`?
So, the problem seems to be that Linux doesn't differentiate PC10-only and PC10+SlpS0 systems. Iow. there's a check missing here ... https://github.com/torvalds/linux/blob/master/drivers/platform/x86/intel_pmc...
Well, ideally the system would make it to SlpS0. I think it's correct to warn that this didn't happen. However, if the table doesn't advertise support, it can be argued that the kernel doesn't need to check the default address...
To me, this appears deliberate, but I couldn't be sure.
Hmm, somewhere above you wrote Linux prints an error for PC10 on vendor firmware. Did you really mean PC10 or SlpS0?
I meant PC10.
Are ASPM and L1 substates involved? The vendor firmware has the "ASPM not supported" bit set in FADT and disables L1 substates.
In coreboot, I had disabled L1 substates and set ASPM to "L1" on the WLAN's root port to mitigate AER errors. I noticed now that the LAN's root port has disabled ASPM but I'm not sure why (the bit is read-write-once, I'll look into that).
-
Benjamin Doron (Code Review)