Attention is currently required from: Jamie Ryu, Wonkyu Kim, Ethan Tsao, Ravishankar Sarawadi, Rizwan Qureshi, Tim Wawrzynczak, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62581 )
Change subject: soc/intel/common: Include Meteorlake device IDs ......................................................................
Patch Set 1:
(17 comments)
File src/include/device/pci_ids.h:
https://review.coreboot.org/c/coreboot/+/62581/comment/ea87e1c5_b3574058 PS1, Line 3078: PCI_DEVICE_ID_INTEL_MTL_ESPI_0 This is what EDS is saying, do we need to add all of these
eSPI Controller 0 / 31 / 0 7E00h - 7E1Fh
https://review.coreboot.org/c/coreboot/+/62581/comment/ae4657fb_22849005 PS1, Line 3431: #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP9 0x7e4d : #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP10 0x7eca : #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP11 0x7ecb : #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP12 0x7ecc I have some confusion here 1. EDS only mentioned about RPs between (0 / 28 / 0) - (0 / 28 / 7) 2. Unlike, ADL (as below), I'm unable to find D29/Fx RPs in the EDS.
D28/F0 = Port1 D28/F1 = Port2 D28/F2 = Port3 D28/F3 = Port4 D28/F4 = Port5 D28/F5 = Port6 D28/F6 = Port7 D28/F7 = Port8 D29/F0 = Port9 D29/F1 = Port10 D29/F2 = Port11 D29/F3 = Port12
3. Finally, 0x7e4d is not PCH RP, rather it's CPU RP (PEG 1 and 0/6/x), Typically, we try to use CPU name (example: PCI_DEVICE_ID_INTEL_ADL_P_PCIE_RPx for CPU RPs) and PCH name (example: PCI_DEVICE_ID_INTEL_ADP_P_PCIE_RPx for PCH RPs). I can understand for MTL, we don't have PCH die hence, you can use SoC die name which is replacement of PCH in similar manner.
I'm seeing this PCIe RP DIDs as
/* CPU RPs */ #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP1 0x7e4d #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP2 0x7eca #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP3 0x7ecb #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP4 0x7ecc
/* SoC RPs */ #define PCI_DEVICE_ID_INTEL_MTP_PCIE_RP1 0x7e38 #define PCI_DEVICE_ID_INTEL_MTP_PCIE_RP2 0x7e39 #define PCI_DEVICE_ID_INTEL_MTP_PCIE_RP3 0x7e3a #define PCI_DEVICE_ID_INTEL_MTP_PCIE_RP4 0x7e3b #define PCI_DEVICE_ID_INTEL_MTP_PCIE_RP5 0x7e3c #define PCI_DEVICE_ID_INTEL_MTP_PCIE_RP6 0x7e3d #define PCI_DEVICE_ID_INTEL_MTP_PCIE_RP7 0x7e3e #define PCI_DEVICE_ID_INTEL_MTP_PCIE_RP8 0x7e3f
<Missing RPs are RP9-RP12>
or with MTL, we are reducing the RPs from 16 (12 PCH and 4 CPU) to 12 (alone). I would still prefer to have the clean split between RPs at north and south (although this line doesn't exist today)
https://review.coreboot.org/c/coreboot/+/62581/comment/5330d4aa_3779b29b PS1, Line 3508: #define PCI_DEVICE_ID_INTEL_MTL_SATA_2 0x7e67 : #define PCI_DEVICE_ID_INTEL_MTL_SATA_3 0x282a you don't need these DID as for RAID mode. AHCI is enough
https://review.coreboot.org/c/coreboot/+/62581/comment/4550edbd_a63d6f6a PS1, Line 3531: PCI_DEVICE_ID_INTEL_MTL_PMC meaningful would be have different name macro
#define PCI_DEVICE_ID_INTEL_MTL_SOC_PMC 0x7e21
https://review.coreboot.org/c/coreboot/+/62581/comment/05737427_d3b2bd10 PS1, Line 3532: PCI_DEVICE_ID_INTEL_MTL_M_PMC nit: PCI_DEVICE_ID_INTEL_MTL_M_IOE_PMC
https://review.coreboot.org/c/coreboot/+/62581/comment/42526c7b_4c6d02e5 PS1, Line 3533: PCI_DEVICE_ID_INTEL_MTL_P_PMC nit: PCI_DEVICE_ID_INTEL_MTL_P_IOE_PMC
https://review.coreboot.org/c/coreboot/+/62581/comment/fc7142ab_fca256ac PS1, Line 3807: PCI_DEVICE_ID_INTEL_MTL_SPI0 suggestion: maintain the similar naming convention with previous generation
#define PCI_DEVICE_ID_INTEL_MTL_HWSEQ_SPI 0x7e23
https://review.coreboot.org/c/coreboot/+/62581/comment/510f3620_0b5cfd96 PS1, Line 3962: #define PCI_DEVICE_ID_INTEL_SIMICS_GT0 0xffff u don't need this. no probing is done for 0xFFFF DID device.
https://review.coreboot.org/c/coreboot/+/62581/comment/460f4c6b_84513c67 PS1, Line 4140: PCI_DEVICE_ID_INTEL_MTL_M_XHCI PCI_DEVICE_ID_INTEL_MTL_M_TCSS_XHCI
https://review.coreboot.org/c/coreboot/+/62581/comment/84a90f7d_498fd0ba PS1, Line 4141: PCI_DEVICE_ID_INTEL_MTL_P_XHCI same
https://review.coreboot.org/c/coreboot/+/62581/comment/f315af2f_fa4eb426 PS1, Line 4163: PCI_DEVICE_ID_INTEL_MTL_P2SB please follow the same suggestion as PMC DID and split between SOC and IOE
https://review.coreboot.org/c/coreboot/+/62581/comment/4da13c52_88c2b977 PS1, Line 4178: PCI_DEVICE_ID_INTEL_MTL_SRAM same as above
https://review.coreboot.org/c/coreboot/+/62581/comment/43460e60_0dc61630 PS1, Line 4269: #define PCI_DEVICE_ID_INTEL_MTL_CSE1 0x7e71 : #define PCI_DEVICE_ID_INTEL_MTL_CSE2 0x7e74 : #define PCI_DEVICE_ID_INTEL_MTL_CSE3 0x7e75 wondering, if we really need these HECI devices, we are keeping HECI1 alone enable.
PCI: 00:16.0 10 <- [0x00807db000 - 0x00807dbfff] size 0x00001000 gran 0x0c mem64
https://review.coreboot.org/c/coreboot/+/62581/comment/54147bdb_25dffb1d PS1, Line 4292: PCI_DEVICE_ID_INTEL_MTL_M_XDCI Please add _TCSS_ in between
https://review.coreboot.org/c/coreboot/+/62581/comment/9d50888b_1b782879 PS1, Line 4416: can you please add CNVI Ids as well?
https://review.coreboot.org/c/coreboot/+/62581/comment/5a987e39_f96cf35f PS1, Line 4424: can you please add Crashlog ID as well?
File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/62581/comment/febbeb6b_84a07d18 PS1, Line 31: CPUID_METEORLAKE_1_A0 suggestion: CPUID_METEORLAKE_A0_1
-
Subrata Banik (Code Review)