Attention is currently required from: Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Paul Menzel, V Sowmya, Vidya Gopalakrishnan, Vinay Kumar.
Subrata Banik has posted comments on this change by Vidya Gopalakrishnan. ( https://review.coreboot.org/c/coreboot/+/85184?usp=email )
Change subject: mb/google/brya/var/trulo: Remove overriding of PL1 value to 20W ......................................................................
Patch Set 5:
(1 comment)
File src/mainboard/google/brya/variants/trulo/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/85184/comment/65e6a3f3_ea1cb6b4?usp... : PS4, Line 184: register "power_limits_config[ADL_N_041_6W_CORE]" = "{ : .tdp_pl1_override = 20, : .tdp_pl2_override = 25, : .tdp_pl4 = 78, : }" : : register "power_limits_config[ADL_N_081_7W_CORE]" = "{ : .tdp_pl1_override = 20, : .tdp_pl2_override = 25, : .tdp_pl4 = 78, : }" : : register "power_limits_config[ADL_N_081_15W_CORE]" = "{ : .tdp_pl1_override = 20, : .tdp_pl2_override = 35, : .tdp_pl4 = 83, : }"
PL1/PL2 override hooks can still be used by ODMs by adding this block of code back. But overriding PL1/PL2 to a higher value as done here should not be a default guidance given from a reference design as using a higher PLx can have thermal impacts to silicon. This code block seems to be copied from some other production device variant that is also using a higher override for PLx values. Such overrides are not part of any other reference design variant.
This code may coexist with the actual limit if there's a check to assert the build if the overridden value is larger than the limit. Ideally, a check like this should be included.
There is no override required if overriden value is invalid or matches to the existing limit.
we have agreed to implement an assert logic if overriden value is higher than the limit.