Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... PS5, Line 68: LPM_S0i3_4 = BIT(7), It might be helpful to have an enum which says all S0ix substates 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;
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... PS5, Line 93: enable disable
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... PS5, Line 93: It will be good to mention here that by default all S0ix substates are enabled unless any substate is explicitly disabled by mainboard using this parameter.
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/fsp... PS5, Line 213: ~config->LpmStateDisableMask LPM_S0ix_all & ~config->LpmStateDisableMask;
Ensures that this will set the enable bits correctly even if the default UPD value changes in the future.