Attention is currently required from: Cliff Huang, Kapil Porwal, Pranava Y N, Subrata Banik.
Saurabh Mishra has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83798?usp=email )
Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till ramstage ......................................................................
Patch Set 40:
(11 comments)
File src/soc/intel/pantherlake/pmc.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/5acb5623_c51ed3fa?usp... : PS38, Line 76: pmc_dump_soc_qdf_info();
why again ?
Removed. Already called in bootblock stage. https://review.coreboot.org/c/coreboot/+/83940/6/src/soc/intel/pantherlake/b...
https://review.coreboot.org/c/coreboot/+/83798/comment/4c4b996a_9535ef11?usp... : PS38, Line 106:
please follow https://review.coreboot. […]
Sure, implemented.
https://review.coreboot.org/c/coreboot/+/83798/comment/fa93d336_5a2a3ab8?usp... : PS38, Line 124: lnl_pmc_lpm
should be `ptl_pmc_lpm`?
Corrected.
https://review.coreboot.org/c/coreboot/+/83798/comment/5a780df4_3ebf9fce?usp... : PS38, Line 132: lnl_pmc_lpm
same
Corrected.
https://review.coreboot.org/c/coreboot/+/83798/comment/ce196645_9c839fec?usp... : PS38, Line 188: soc_acpi_mode_init
should be `soc_pmc_init`
Updated the name and usage as suggested.
https://review.coreboot.org/c/coreboot/+/83798/comment/2f5ca5c4_bae1572f?usp... : PS38, Line 189: pmc_init
Why is `pmc_int` for enable function ? ideally you should be using `soc_pmc_enable`
Updated the name and usage as suggested.
File src/soc/intel/pantherlake/pmutil.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/9c93a3e5_0b7c4409?usp... : PS38, Line 15: #include <device/pci_def.h>
why we need this ?
Ack, removed.
File src/soc/intel/pantherlake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/83798/comment/031614cb_f1dc7083?usp... : PS38, Line 99: get_mmcfg_size
sa_get_mmcfg_size
Rebased to use IA common code for range calculations.
https://review.coreboot.org/c/coreboot/+/83798/comment/b7105d06_17c41c99?usp... : PS38, Line 105: get_dsm_size
please look at https://review.coreboot. […]
Rebased to use IA common code for range calculations.
https://review.coreboot.org/c/coreboot/+/83798/comment/aed06a3a_21f7bfde?usp... : PS38, Line 163: /* Enable BIOS Reset CPL */ : enable_bios_reset_cpl();
Acknowledged
https://review.coreboot.org/c/coreboot/+/83798/comment/23adaf3e_b9ce7990?usp... : PS38, Line 206: uint64_t get_mmcfg_size(const struct device *dev) : { : uint32_t pciexbar_reg; : uint64_t mmcfg_length; : : if (!dev) { : printk(BIOS_DEBUG, "%s : device is null\n", __func__); : return 0; : } : : pciexbar_reg = pci_read_config32(dev, PCIEXBAR); : : if (!(pciexbar_reg & (1 << 0))) { : printk(BIOS_DEBUG, "%s : PCIEXBAR disabled\n", __func__); : return 0; : } : : switch ((pciexbar_reg & MASK_PCIEXBAR_LENGTH) >> PCIEXBAR_LENGTH_LSB) { : case PCIEXBAR_LENGTH_4096MB: : mmcfg_length = 4 * ((uint64_t)GiB); : break; : case PCIEXBAR_LENGTH_2048MB: : mmcfg_length = 2 * ((uint64_t)GiB); : break; : case PCIEXBAR_LENGTH_1024MB: : mmcfg_length = 1 * GiB; : break; : case PCIEXBAR_LENGTH_512MB: : mmcfg_length = 512 * MiB; : break; : case PCIEXBAR_LENGTH_256MB: : mmcfg_length = 256 * MiB; : break; : case PCIEXBAR_LENGTH_128MB: : mmcfg_length = 128 * MiB; : break; : case PCIEXBAR_LENGTH_64MB: : mmcfg_length = 64 * MiB; : break; : default: : printk(BIOS_DEBUG, "%s : PCIEXBAR - invalid length (0x%x)\n", __func__, : pciexbar_reg & MASK_PCIEXBAR_LENGTH); : mmcfg_length = 0x0; : break; : } : : return mmcfg_length; : } : : uint64_t get_dsm_size(const struct device *dev) : { : // - size : B0/D0/F0:R 50h [15:8] : uint32_t reg32 = pci_read_config32(dev, GGC); : uint64_t size; : uint32_t size_field = (reg32 & MASK_DSM_LENGTH) >> MASK_DSM_LENGTH_LSB; : if (size_field <= 0x10) { // 0x0 - 0x10 : size = size_field * 32 * MiB; : } else if ((size_field >= 0xF0) && (size_field >= 0xFE)) { : size = ((uint64_t)size_field - 0xEF) * 4 * MiB; : } else { : switch (size_field) { : case 0x20: : size = 1 * GiB; : break; : case 0x30: : size = 1536 * MiB; : break; : case 0x40: : size = 2 * (uint64_t)GiB; : break; : default: : printk(BIOS_DEBUG, "%s : DSM - invalid length (0x%x)\n", : __func__, size_field); : size = 0x0; : break; : } : } : return size; : } : : uint64_t get_gsm_size(const struct device *dev) : { : const u32 gsm_size = pci_read_config32(dev, GGC); : uint64_t size; : uint32_t size_field = (gsm_size & MASK_GSM_LENGTH) >> MASK_GSM_LENGTH_LSB; : switch (size_field) { : case 0x0: : size = 0; : break; : case 0x1: : size = 2 * MiB; : break; : case 0x2: : size = 4 * MiB; : break; : case 0x3: : size = 8 * MiB; : break; : default: : size = 0; : break; : } : return size; : } : uint64_t get_dpr_size(const struct device *dev) : { : uint64_t size; : uint32_t dpr_reg = pci_read_config32(dev, DPR_REG); : uint32_t size_field = (dpr_reg & MASK_DPR_LENGTH) >> MASK_DPR_LENGTH_LSB; : size = (uint64_t)size_field * MiB; : return size; : }
please drop
Acknowledged