Attention is currently required from: Tarun Tuli, Subrata Banik, Sumeet R Pawnikar, Eric Lai.
Jérémy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74380 )
Change subject: soc/intel/meteoerlake: set power limits dynamically ......................................................................
Patch Set 4: -Code-Review
(2 comments)
Patchset:
PS4: We had a long call earlier today with Sumeet. I agreed to remove my -1 to let this patch move forward considering the project timeline, the fact that Sumeet seems to be having issue with values coming from the FSP and despite the fact I have concern on adding an unnecessary layer of default values duplicating Power Limits default values coming the FSP. Indeed, as Sumeet demonstrated to me this morning, the values he is adding to the `chipset.cb` are generic values from the power map (doc#640982) before tuning for a particular board design (if we exclude Fast VMode) and therefore identical to the values the FSP should be programming. So technically these values being "default" values they can indeed sit under the soc directory.
Obviously, this patch can create a maintenance challenge as these settings can change in the specification and need to be updated in FSP and also in coreboot due to this patch. This is a similar situation to what we have for Alder Lake and Raptor Lake. In addition, this current infrastructure does not take into account if Fast VMode is enabled or not which is a feature impacting PL4 value.
As even Power Limits can change based on board design, there is still a need for board specific customization of fine tuned Power Limits that will need to be addressed. Sumeet and I agreed that on my side, I would only work on providing the customization framework for the voltage Regulator settings when the VR/PnP team provides the settings they want.
The reason, I'm not okay with too much dependency over FSP is that the fact, FSP code and default value can change any time to address the concern from other customers which might create problem for us.
First, once fine tuned, you should rely on Power Limits settings defined in the mainboard directory so changes coming the FSP shouldn't be a problem for your board designs. Second, Intel does not propagate default values changes which do not apply widely and without any very thorough validation.
File src/soc/intel/meteorlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/74380/comment/a97af462_ff61529e PS4, Line 8: .tdp_pl4 = 101, In our current configuration, fast Vmode is disable which according to #640982 revision 1p1 means that PL4 should be 114W.