[coreboot-gerrit] [M] Change in coreboot[main]: mb/google/{brya,hades}: use soc index for variant_update_power_limits()

[SH Kim (Code Review)]

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Paul Menzel, Subrata Banik, Sumeet R Pawnikar.

SH Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81436?usp=email )

Change subject: mb/google/{brya,hades}: use soc index for variant_update_power_limits()
......................................................................

Patch Set 6:

(6 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/81436/comment/b9b6a5dc_6c35fb09 :
PS5, Line 13: override the PL4 value
> Please share more details on the reason for overriding the PL4 value.  […]

I was just trying some tests for b/328729536 using this interface by referring to skolas code. (skolas already uses this function)
And found the code doesn't work properly by accidentlly. I can find some brya boards using variant_update_power_limits() to set the default PL setting, I thought this is for override the default PL1/PL2/PL4 settings according to the board design and SKUs.

https://review.coreboot.org/c/coreboot/+/81436/comment/7428546d_5b86bde3 :
PS5, Line 14: ramstage.c :
> Please remove the space before the colon.

Done

https://review.coreboot.org/c/coreboot/+/81436/comment/99e8571e_460b1e01 :
PS5, Line 15: const struct cpu_power_limits limits[] = {
            :     {PCI_DID_INTEL_RPL_P_ID3, 15, 6000, 15000, 55000, 55000, 80000},
            : }
> Please follow Markdown mark-up language and indent with four spaces.

Done

File src/mainboard/google/brya/variants/baseboard/brya/ramstage.c:

https://review.coreboot.org/c/coreboot/+/81436/comment/46324108_44662a5e :
PS5, Line 39:
> Any reason to removal of these? It was aligned based on the previous argument.

Done

https://review.coreboot.org/c/coreboot/+/81436/comment/10176515_0e948001 :
PS5, Line 41:
> same comment as above.

Done

https://review.coreboot.org/c/coreboot/+/81436/comment/688a0ba7_779501c0 :
PS5, Line 68: 			soc_config->tdp_pl4 = DIV_ROUND_UP(limits[i].pl4_power,
> DPTF (user space service) overrides PL1/PL2 for MMIO registers only.

SOC will use the lower PL1/PL2 value of MMIO and MSR. If user may want to override PL1/PL2 to higher value than original one which is in MSR, SOC will never take higher one since PL1/PL2 are limited by the lower values in MSR.
IMO it would be better to set MSR PL1/PL2 values here, how do you think?