Attention is currently required from: Tarun Tuli, Subrata Banik, Name of user not set #1004406, Michal Zygowski.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69798 )
Change subject: Add basic support for Rapto Lake S CPUs on Alder Lake PCH (#640555) This was tested on a MSI PRO Z690-A (ms7d25) with Ubuntu 22.10 and LinuxBoot. TODOS are Intel VBT. ......................................................................
Patch Set 3:
(14 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/69798/comment/99dfe230_bc9a7fae PS3, Line 7: 640555 I see this is an EDS. What revision it was?
https://review.coreboot.org/c/coreboot/+/69798/comment/c2822de6_cc35d94f PS3, Line 8: This was tested on a MSI PRO Z690-A (ms7d25) with Ubuntu 22.10 and LinuxBoot. Move this sentence just above Change-ID in the commit message liek this:
TEST=Tested on a MSI PRO Z690-A (ms7d25) with Ubuntu 22.10 and LinuxBoot.
File src/include/cpu/intel/cpu_ids.h:
https://review.coreboot.org/c/coreboot/+/69798/comment/0d3b7962_e601cc1e PS3, Line 67: #define CPUID_RAPTORLAKE_S_S0 0xb0671 CPUID_RAPTORLAKE_S_B0 it is B0 stepping according to the datasheet. And if so, I would also add:
#define CPUID_RAPTORLAKE_S_A0 0xb0670
Because it will automatically be A0 stepping which we can also report (but unlikely to be found on production systems).
File src/include/device/pci_ids.h:
https://review.coreboot.org/c/coreboot/+/69798/comment/f7d8be98_b0291f33 PS3, Line 4009: #define PCI_DID_INTEL_RPL_S_GT2 0xa782 : #define PCI_DID_INTEL_RPL_S_GT3 0xa783 I didn't find them in the public datasheet here https://edc.intel.com/content/www/us/en/design/products/platforms/details/ra... Where do these come from?
https://review.coreboot.org/c/coreboot/+/69798/comment/346dc856_e785442c PS3, Line 4141: #define PCI_DID_INTEL_RPL_S_ID_4 0xa705 : #define PCI_DID_INTEL_RPL_S_ID_5 0x4692 I also can;t find these in public datasheet. Any reference for those?
File src/soc/intel/alderlake/bootblock/report_platform.c:
https://review.coreboot.org/c/coreboot/+/69798/comment/f751fa19_8382d7de PS3, Line 37: { CPUID_RAPTORLAKE_S_S0, "Raptorlake-S S0 Platform" }, { CPUID_RAPTORLAKE_S_B0, "Raptorlake-S B0 Platform" },
https://review.coreboot.org/c/coreboot/+/69798/comment/5ba8cb6d_b76cfdf2 PS3, Line 207: { PCI_DID_INTEL_RPL_S_GT1, "Raptorlake S GT1(32EU)" }, : { PCI_DID_INTEL_RPL_S_GT2, "Raptorlake S GT2(24EU)" }, : { PCI_DID_INTEL_RPL_S_GT3, "Raptorlake S GT3(16EU)" }, Let's stick with previous convention and omit execution units.
File src/soc/intel/alderlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/69798/comment/d2aa7c32_8987f29d PS3, Line 544: return ICC_MAX_ADL_S; Isn't it 36A for RPL-S?
File src/soc/intel/alderlake/include/soc/cpu.h:
https://review.coreboot.org/c/coreboot/+/69798/comment/45f3ba9a_66c04554 PS3, Line 28: RPL_S, Please add it as a last item in the enum
File src/soc/intel/alderlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/69798/comment/8944a762_d888a64c PS3, Line 174: { PCI_DID_INTEL_RPL_S_ID_1, 125, VR_CFG_ALL_DOMAINS_ICC(280, 30) }, : { PCI_DID_INTEL_RPL_S_ID_2, 125, VR_CFG_ALL_DOMAINS_ICC(280, 30) }, RPL-S 8+16 and 8+8 have 307A ICCmax for IA domain.
https://review.coreboot.org/c/coreboot/+/69798/comment/9dedb1ad_ec5c7c12 PS3, Line 176: { PCI_DID_INTEL_RPL_S_ID_3, 125, VR_CFG_ALL_DOMAINS_ICC(280, 30) }, RPL-S 6+8 has 200A ICCmax for IA domain
https://review.coreboot.org/c/coreboot/+/69798/comment/523dc5fa_b888cd9e PS3, Line 177: { PCI_DID_INTEL_RPL_S_ID_4, 125, VR_CFG_ALL_DOMAINS_ICC(280, 30) }, Not sure what is this ID and what ICCmax values it should have.
https://review.coreboot.org/c/coreboot/+/69798/comment/a142ac83_7a7826bc PS3, Line 256: { PCI_DID_INTEL_RPL_S_ID_1, 125, VR_CFG_ALL_DOMAINS_TDC_CURRENT(132, 132) }, : { PCI_DID_INTEL_RPL_S_ID_2, 125, VR_CFG_ALL_DOMAINS_TDC_CURRENT(132, 132) }, : { PCI_DID_INTEL_RPL_S_ID_3, 125, VR_CFG_ALL_DOMAINS_TDC_CURRENT(132, 132) }, : { PCI_DID_INTEL_RPL_S_ID_4, 125, VR_CFG_ALL_DOMAINS_TDC_CURRENT(132, 132) }, No info in public datasheets about those values. Any references? Also nto all RSP-S IDs in this table have to have TPD 125W, so some values would not be used at all.
File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/69798/comment/533357a2_26063880 PS3, Line 84: { X86_VENDOR_INTEL, CPUID_RAPTORLAKE_S_S0 }, { X86_VENDOR_INTEL, CPUID_RAPTORLAKE_S_B0 },