Attention is currently required from: Cliff Huang, Jérémy Compostella, Kapil Porwal, Pranava Y N, Ravishankar Sarawadi, Saurabh Mishra.
Subrata Banik has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83635?usp=email )
Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till romstage ......................................................................
Patch Set 72:
(6 comments)
File src/soc/intel/pantherlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/83635/comment/ec1c3f89_0f7c413e?usp... : PS64, Line 108: 12 this is correct for PTL-UH
RP counts
1. PTL-UH - 12 2. PTL-H - 10
Please use PCH Kconfig to differentiate
File src/soc/intel/pantherlake/chip.h:
https://review.coreboot.org/c/coreboot/+/83635/comment/87c85f1f_b9db0789?usp... : PS72, Line 63: /* Bit values for use in LpmStateEnableMask. */ : enum lpm_state_mask { : LPM_S0i2_0 = BIT(0), : LPM_S0i2_1 = BIT(1), : LPM_S0i2_2 = BIT(2), : LPM_S0i3_0 = BIT(3), : LPM_S0i3_1 = BIT(4), : LPM_S0i3_2 = BIT(5), : LPM_S0i3_3 = BIT(6), : LPM_S0i3_4 = BIT(7), : LPM_S0iX_ALL = LPM_S0i2_0 | LPM_S0i2_1 | LPM_S0i2_2 : | LPM_S0i3_0 | LPM_S0i3_1 | LPM_S0i3_2 | LPM_S0i3_3 | LPM_S0i3_4, : }; : do you see any users for these macros ? if not, then please drop
File src/soc/intel/pantherlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/83635/comment/e93db90f_a5f25d94?usp... : PS72, Line 5: PTL_U_H_POWER_LIMITS as I could see from the doc 823589,
you might need to keep 3 macros
1. for PTL-U (15W) 2. for PTL-H (25W) 3. for PTL-H (45W)
an attempt to combine #1 and #2 won't work.
Additionally, doc doesn't specify the PL2 value, rather it says that the PL2 is same as ARL-H on same segment. I don't know the PL2 value for ARL as well. Can you please help here. Unable to find PL4 as well in the doc.
File src/soc/intel/pantherlake/include/soc/meminit.h:
https://review.coreboot.org/c/coreboot/+/83635/comment/fc7d2b98_e9e24e4b?usp... : PS72, Line 97: Lp4/ I don't believe we support LP4 with PTL
File src/soc/intel/pantherlake/include/soc/romstage.h:
https://review.coreboot.org/c/coreboot/+/83635/comment/7bb22104_0a987706?usp... : PS72, Line 10: mainboard_update_premem_soc_chip_config who is the consumer of this code ?
File src/soc/intel/pantherlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/83635/comment/0a210823_ac2b2166?usp... : PS72, Line 26: /*TODO a space to start with
``` /* TODO: */ ```