Krzysztof M Sywula has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32026 )
Change subject: soc/intel/cannonlake: FSP UPD update ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32026/1/src/soc/intel/cannonlake/fsp_params.... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/#/c/32026/1/src/soc/intel/cannonlake/fsp_params.... PS1, Line 169: PchPmSlpS0VmRuntimeControl
What is the downside of enabling these options when config->s0ix_enable is set?
As far as I know voltage margining is only used for debug/validation purposes. Not sure if that should ever be auto configured. I see in coreboot sources some of these options are left as is (so they are fully controlled from appropriate devicetree.cb). Some other options are auto-configured wihtin if/else logic. I honestly don't know what's better in this case.
Now, addressing your exact question, I think s0ix_enable should not control these params because from end user point of view it's perfectly feasible to have either case: s0ix_enable=1 ; PchPmSlpS0VmRuntimeControl=0 or s0ix_enable=1 ; PchPmSlpS0VmRuntimeControl=1