[XL] Change in coreboot[main]: soc/intel/ptl: Do initial Panther Lake SoC commit till ramstage

[Saurabh Mishra (Code Review)]

Attention is currently required from: Jérémy Compostella, 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 68:

(11 comments)

File src/soc/intel/pantherlake/acpi.c:

https://review.coreboot.org/c/coreboot/+/83798/comment/fc743ab7_94ac445e?usp... :
PS50, Line 138: map
> > If the configuration is static, we can use below code. […]

OK, sure, i have implemeted the flag "initialized", and added the above suggested snippet.

File src/soc/intel/pantherlake/acpi.c:

https://review.coreboot.org/c/coreboot/+/83798/comment/7cd83e02_86301a08?usp... :
PS55, Line 238: PCI_DEVFN_IGD
> > In the case when igpu is disabled using fatcat variant device tree, this will gaurd the inclusion  […]

Sure, this is correct, as chipset.cb has default GFX enabled. I have removed the is_devfn_enabled(PCI_DEVFN_IGD) check.

https://review.coreboot.org/c/coreboot/+/83798/comment/c094d855_80bf0608?usp... :
PS55, Line 247: (is_devfn_enabled(PCI_DEVFN_DPTF) ||
              : 		is_devfn_enabled(PCI_DEVFN_IPU) ||
              : 		is_devfn_enabled(PCI_DEVFN_NPU) ||
              : 		is_devfn_enabled(PCI_DEVFN_IAA))
> > Hi Subrata, this check is introduced to check if none of the IPs (i.e. […]

Looks good, i have made the changes accordingly.

File src/soc/intel/pantherlake/chip.h:

https://review.coreboot.org/c/coreboot/+/83798/comment/ef8e9da8_fc6abf72?usp... :
PS65, Line 339: 	bool enable_c6dram;
> I don't see any usage. […]

Sure, removed.

https://review.coreboot.org/c/coreboot/+/83798/comment/0bbac52b_696a01ab?usp... :
PS65, Line 340: 	bool pm_timer_disabled;
> same

Sure, removed.

File src/soc/intel/pantherlake/chip.h:

https://review.coreboot.org/c/coreboot/+/83798/comment/e0db34e9_6a584501?usp... :
PS55, Line 47: PTL_H_484_12_25W_CORE
> > Looks good to me. […]

Updated accordingly.

File src/soc/intel/pantherlake/cpu.c:

https://review.coreboot.org/c/coreboot/+/83798/comment/ccefc79d_851c1d21?usp... :
PS65, Line 59: BIT(0)
> FAST_STRINGS_ENABLE_BIT

Done.

https://review.coreboot.org/c/coreboot/+/83798/comment/3bae0ddf_08916f70?usp... :
PS65, Line 60: BIT(3)
> can u please implement a macro for the required bit-field under the MSR definition ? […]

Sure, done.

https://review.coreboot.org/c/coreboot/+/83798/comment/1bee8f09_688dd93f?usp... :
PS65, Line 72: = BIT(4);
> same applies here

Done

https://review.coreboot.org/c/coreboot/+/83798/comment/761037f1_1d2a65ef?usp... :
PS65, Line 78: 	msr.lo |= BIT(0);	/* Enable Bi-directional PROCHOT as an input*/
             : 	msr.lo |= BIT(23);	/* Lock it */
             : 	msr.lo |= BIT(18);	/* Power Performance Platform Override */
> same

Added.

File src/soc/intel/pantherlake/include/soc/systemagent.h:

https://review.coreboot.org/c/coreboot/+/83798/comment/9ebb009f_1a5df961?usp... :
PS65, Line 32: #define GFXVT_ENABLED		BIT(0)
             : #define NONGFXVT_ENABLED	BIT(1)
             : #define IOCVT_ENABLED		BIT(2)
> if there is no consumer outside ACPI.c file then no harm to define it inside local . […]

Sure, moved to acpi.c.