Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Matt DeVillier, Nick Vaccaro, Nico Huber, Paul Menzel, Sean Rhodes, Subrata Banik.
Angel Pons has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81638?usp=email )
Change subject: intel/alderlake: Add helper functions for Power Management ......................................................................
Patch Set 15: Code-Review+1
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81638/comment/6d454014_a6e5b4b3?usp... : PS15, Line 33: Surprise- "No surprise", given how messy FSP UPDs tend to be 😜
File src/soc/intel/alderlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/81638/comment/3d520dab_3fd0a25e?usp... : PS15, Line 507: if (rp_cfg->pcie_rp_aspm == 0) : s_cfg->PcieRpAspm[index] = 4; : else : s_cfg->PcieRpAspm[index] = rp_cfg->pcie_rp_aspm - 1; I would make a helper function for this:
``` static unsigned int adl_get_aspm_control(unsigned int dt_aspm) { if (dt_aspm == 0) return 4; else return dt_aspm - 1; } ```
https://review.coreboot.org/c/coreboot/+/81638/comment/07285c71_5b5e3738?usp... : PS15, Line 519: if (CONFIG(SOC_INTEL_COMPLIANCE_TEST_MODE)) : s_cfg->PcieRpL1Substates[index] = 0; : else if (rp_cfg->PcieRpL1Substates == 0) : s_cfg->PcieRpL1Substates[index] = 3; : else : s_cfg->PcieRpL1Substates[index] = rp_cfg->PcieRpL1Substates - 1; I would make a helper function for this:
``` static unsigned int adl_get_l1_substate_control(unsigned int dt_l1ss) { if (CONFIG(SOC_INTEL_COMPLIANCE_TEST_MODE)) return 0; else if (dt_l1ss == 0) return 3; else return dt_l1ss - 1; } ```