Change in coreboot[master]: soc/intel/tigerlake: Add processor power limits control support
Sumeet R Pawnikar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support to configure values.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/tigerlake/include/soc/msr.h M src/soc/intel/tigerlake/systemagent.c 4 files changed, 213 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/1
diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h index e57abe8..02e7ec2 100644 --- a/src/soc/intel/tigerlake/chip.h +++ b/src/soc/intel/tigerlake/chip.h @@ -148,8 +148,22 @@ /* HeciEnabled decides the state of Heci1 at end of boot * Setting to 0 (default) disables Heci1 and hides the device from OS */ uint8_t HeciEnabled; + /* PL1 Override value in Watts */ + uint32_t tdp_pl1_override; /* PL2 Override value in Watts */ uint32_t tdp_pl2_override; + /* SysPL2 Value in Watts */ + uint32_t tdp_psyspl2; + /* SysPL3 Value in Watts */ + uint32_t tdp_psyspl3; + /* SysPL3 window size */ + uint32_t tdp_psyspl3_time; + /* SysPL3 duty cycle */ + uint32_t tdp_psyspl3_dutycycle; + /* PL4 Value in Watts */ + uint32_t tdp_pl4; + /* Estimated maximum platform power in Watts */ + uint16_t psys_pmax; /* Intel Speed Shift Technology */ uint8_t speed_shift_enable;
diff --git a/src/soc/intel/tigerlake/cpu.c b/src/soc/intel/tigerlake/cpu.c index 5f4f081..1be9f99 100644 --- a/src/soc/intel/tigerlake/cpu.c +++ b/src/soc/intel/tigerlake/cpu.c @@ -38,6 +38,194 @@ #include <soc/pm.h> #include <soc/soc_chip.h>
+ +/* Convert time in seconds to POWER_LIMIT_1_TIME MSR value */ +static const u8 power_limit_time_sec_to_msr[] = { + [0] = 0x00, + [1] = 0x0a, + [2] = 0x0b, + [3] = 0x4b, + [4] = 0x0c, + [5] = 0x2c, + [6] = 0x4c, + [7] = 0x6c, + [8] = 0x0d, + [10] = 0x2d, + [12] = 0x4d, + [14] = 0x6d, + [16] = 0x0e, + [20] = 0x2e, + [24] = 0x4e, + [28] = 0x6e, + [32] = 0x0f, + [40] = 0x2f, + [48] = 0x4f, + [56] = 0x6f, + [64] = 0x10, + [80] = 0x30, + [96] = 0x50, + [112] = 0x70, + [128] = 0x11, +}; + +/* Convert POWER_LIMIT_1_TIME MSR value to seconds */ +static const u8 power_limit_time_msr_to_sec[] = { + [0x00] = 0, + [0x0a] = 1, + [0x0b] = 2, + [0x4b] = 3, + [0x0c] = 4, + [0x2c] = 5, + [0x4c] = 6, + [0x6c] = 7, + [0x0d] = 8, + [0x2d] = 10, + [0x4d] = 12, + [0x6d] = 14, + [0x0e] = 16, + [0x2e] = 20, + [0x4e] = 24, + [0x6e] = 28, + [0x0f] = 32, + [0x2f] = 40, + [0x4f] = 48, + [0x6f] = 56, + [0x10] = 64, + [0x30] = 80, + [0x50] = 96, + [0x70] = 112, + [0x11] = 128, +}; + +/* + * Configure processor power limits if possible + * This must be done AFTER set of BIOS_RESET_CPL + */ +void set_power_limits(u8 power_limit_1_time) +{ + msr_t msr = rdmsr(MSR_PLATFORM_INFO); + msr_t limit; + unsigned int power_unit; + unsigned int tdp, min_power, max_power, max_time, tdp_pl2, tdp_pl1; + u8 power_limit_1_val; + struct device *dev = SA_DEV_ROOT; + config_t *conf = dev->chip_info; + + if (power_limit_1_time > ARRAY_SIZE(power_limit_time_sec_to_msr)) + power_limit_1_time = 28; + + if (!(msr.lo & PLATFORM_INFO_SET_TDP)) + return; + + /* Get units */ + msr = rdmsr(MSR_PKG_POWER_SKU_UNIT); + power_unit = 1 << (msr.lo & 0xf); + + /* Get power defaults for this SKU */ + msr = rdmsr(MSR_PKG_POWER_SKU); + tdp = msr.lo & 0x7fff; + min_power = (msr.lo >> 16) & 0x7fff; + max_power = msr.hi & 0x7fff; + max_time = (msr.hi >> 16) & 0x7f; + + printk(BIOS_DEBUG, "CPU TDP: %u Watts\n", tdp / power_unit); + + if (power_limit_time_msr_to_sec[max_time] > power_limit_1_time) + power_limit_1_time = power_limit_time_msr_to_sec[max_time]; + + if (min_power > 0 && tdp < min_power) + tdp = min_power; + + if (max_power > 0 && tdp > max_power) + tdp = max_power; + + power_limit_1_val = power_limit_time_sec_to_msr[power_limit_1_time]; + + /* Set long term power limit to TDP */ + limit.lo = 0; + tdp_pl1 = ((conf->tdp_pl1_override == 0) ? + tdp : (conf->tdp_pl1_override * power_unit)); + limit.lo |= (tdp_pl1 & PKG_POWER_LIMIT_MASK); + + /* Set PL1 Pkg Power clamp bit */ + limit.lo |= PKG_POWER_LIMIT_CLAMP; + + limit.lo |= PKG_POWER_LIMIT_EN; + limit.lo |= (power_limit_1_val & PKG_POWER_LIMIT_TIME_MASK) << + PKG_POWER_LIMIT_TIME_SHIFT; + + /* Set short term power limit to 1.25 * TDP if no config given */ + limit.hi = 0; + tdp_pl2 = (conf->tdp_pl2_override == 0) ? + (tdp * 125) / 100 : (conf->tdp_pl2_override * power_unit); + printk(BIOS_DEBUG, "CPU PL2 = %u Watts\n", tdp_pl2 / power_unit); + limit.hi |= (tdp_pl2) & PKG_POWER_LIMIT_MASK; + limit.hi |= PKG_POWER_LIMIT_CLAMP; + limit.hi |= PKG_POWER_LIMIT_EN; + + /* Power limit 2 time is only programmable on server SKU */ + wrmsr(MSR_PKG_POWER_LIMIT, limit); + + /* Set PL2 power limit values in MCHBAR and disable PL1 */ + MCHBAR32(MCH_PKG_POWER_LIMIT_LO) = limit.lo & (~(PKG_POWER_LIMIT_EN)); + MCHBAR32(MCH_PKG_POWER_LIMIT_HI) = limit.hi; + + /* Set PsysPl2 */ + if (conf->tdp_psyspl2) { + limit = rdmsr(MSR_PLATFORM_POWER_LIMIT); + limit.hi = 0; + printk(BIOS_DEBUG, "CPU PsysPL2=%u Watts\n", conf->tdp_psyspl2); + limit.hi |= (conf->tdp_psyspl2 * power_unit) & + PKG_POWER_LIMIT_MASK; + limit.hi |= PKG_POWER_LIMIT_CLAMP; + limit.hi |= PKG_POWER_LIMIT_EN; + wrmsr(MSR_PLATFORM_POWER_LIMIT, limit); + } + + /* Set PsysPl3 */ + if (conf->tdp_psyspl3) { + limit = rdmsr(MSR_PL3_CONTROL); + limit.lo = 0; + printk(BIOS_DEBUG, "CPU PsysPL3=%u Watts\n", conf->tdp_psyspl3); + limit.lo |= (conf->tdp_psyspl3 * power_unit) & + PKG_POWER_LIMIT_MASK; + /* Enable PsysPl3 */ + limit.lo |= PKG_POWER_LIMIT_EN; + /* set PsysPl3 time window */ + limit.lo |= (conf->tdp_psyspl3_time & + PKG_POWER_LIMIT_TIME_MASK) << + PKG_POWER_LIMIT_TIME_SHIFT; + /* set PsysPl3 duty cycle */ + limit.lo |= (conf->tdp_psyspl3_dutycycle & + PKG_POWER_LIMIT_DUTYCYCLE_MASK) << + PKG_POWER_LIMIT_DUTYCYCLE_SHIFT; + wrmsr(MSR_PL3_CONTROL, limit); + } + + /* Set Pl4 */ + if (conf->tdp_pl4) { + limit = rdmsr(MSR_VR_CURRENT_CONFIG); + limit.lo = 0; + printk(BIOS_DEBUG, "CPU PL4 = %u Watts\n", conf->tdp_pl4); + limit.lo |= (conf->tdp_pl4 * power_unit) & + PKG_POWER_LIMIT_MASK; + wrmsr(MSR_VR_CURRENT_CONFIG, limit); + } + + /* Set DDR RAPL power limit by copying from MMIO to MSR */ + msr.lo = MCHBAR32(MCH_DDR_POWER_LIMIT_LO); + msr.hi = MCHBAR32(MCH_DDR_POWER_LIMIT_HI); + wrmsr(MSR_DDR_RAPL_LIMIT, msr); + + /* Use nominal TDP values for CPUs with configurable TDP */ + if (cpu_config_tdp_levels()) { + msr = rdmsr(MSR_CONFIG_TDP_NOMINAL); + limit.hi = 0; + limit.lo = cpu_get_tdp_nominal_ratio(); + wrmsr(MSR_TURBO_ACTIVATION_RATIO, limit); + } +} + static void soc_fsp_load(void) { fsps_load(romstage_handoff_is_resume()); diff --git a/src/soc/intel/tigerlake/include/soc/msr.h b/src/soc/intel/tigerlake/include/soc/msr.h index 2aa79af..51e30dd 100644 --- a/src/soc/intel/tigerlake/include/soc/msr.h +++ b/src/soc/intel/tigerlake/include/soc/msr.h @@ -19,6 +19,10 @@ #include <intelblocks/msr.h>
#define MSR_PIC_MSG_CONTROL 0x2e -#define MSR_VR_MISC_CONFIG2 0x636 +#define MSR_VR_CURRENT_CONFIG 0x601 +#define MSR_VR_MISC_CONFIG2 0x636 +#define MSR_VR_CURRENT_CONFIG 0x601 +#define MSR_PL3_CONTROL 0x615 +#define MSR_PLATFORM_POWER_LIMIT 0x65c
#endif diff --git a/src/soc/intel/tigerlake/systemagent.c b/src/soc/intel/tigerlake/systemagent.c index 9c8f645..4a572cb 100644 --- a/src/soc/intel/tigerlake/systemagent.c +++ b/src/soc/intel/tigerlake/systemagent.c @@ -20,8 +20,10 @@ */
#include <device/device.h> +#include <delay.h> #include <device/pci.h> #include <intelblocks/systemagent.h> +#include <soc/cpu.h> #include <soc/iomap.h> #include <soc/systemagent.h>
@@ -70,4 +72,8 @@
/* Enable BIOS Reset CPL */ enable_bios_reset_cpl(); + + /* Configure turbo power limits 1ms after reset complete bit */ + mdelay(1); + set_power_limits(28); }
Hello Furquan Shaikh, caveh jalali, Nick Vaccaro, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support to configure values.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CL:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/tigerlake/include/soc/msr.h M src/soc/intel/tigerlake/systemagent.c 4 files changed, 213 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/2
Hello build bot (Jenkins), Furquan Shaikh, caveh jalali, Nick Vaccaro, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#3).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support to configure values.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/tigerlake/include/soc/msr.h M src/soc/intel/tigerlake/systemagent.c 4 files changed, 213 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/3
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 131: BIOS_DEBUG Even log level INFO?
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 161: printk(BIOS_DEBUG, "CPU PL2 = %u Watts\n", tdp_pl2 / power_unit); Ditto.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 4:
(2 comments)
Do all the changes here apply to JSL as well?
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... File src/soc/intel/tigerlake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... PS4, Line 75: /* Configure turbo power limits 1ms after reset complete bit */ : mdelay(1); Why is this? Is this requirement captured anywhere?
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... PS4, Line 77: 28 If this is statically being set to 28, why does it have to be passed in as a parameter? Can't set_power_limits() use it directly?
caveh jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 114: >
=
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 115: 28 so, 28s is the default if out of range? 1) let's add a constant for this 2) should this be the nearest supported value instead of some default? how do you guarantee that power_limit_1_time has an entry in the table? 3) is this the same "max_time" SKU default queried a few lines later?
please reconcile differences with src/soc/intel/cannonlake/cpu.c
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 148: extra indent?
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 198: extra space
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 209: = be consistent about spaces around '=' in these messages and the format of these strings in general.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 4:
(9 comments)
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 114: >
=
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 115: 28
so, 28s is the default if out of range? […]
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 131: BIOS_DEBUG
Even log level INFO?
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 148:
extra indent?
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 161: printk(BIOS_DEBUG, "CPU PL2 = %u Watts\n", tdp_pl2 / power_unit);
Ditto.
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 198:
extra space
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 209: =
be consistent about spaces around '=' in these messages […]
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... File src/soc/intel/tigerlake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... PS4, Line 75: /* Configure turbo power limits 1ms after reset complete bit */ : mdelay(1);
Why is this? Is this requirement captured anywhere?
We will get back on this.
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... PS4, Line 77: 28
If this is statically being set to 28, why does it have to be passed in as a parameter? Can't set_po […]
Need to check this kind of implementation.
Hello build bot (Jenkins), Furquan Shaikh, Caveh Jalali, Tim Wawrzynczak, Subrata Banik, Nick Vaccaro, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#5).
Change subject: oc/intel/tigerlake: Add processor power limits control support ......................................................................
oc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support to configure values.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CL:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- A src/soc/intel/tigerlake/acpi/dptf.asl M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/tigerlake/include/soc/msr.h M src/soc/intel/tigerlake/systemagent.c 5 files changed, 260 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/5
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: oc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/chi... PS5, Line 174: psys_pmax This isn't used anywhere; is that intentional?
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/cpu... PS5, Line 218: MCH_DDR_POWER_LIMIT_LO I don't see coreboot setting these, are these set by FSP-M?
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/sys... File src/soc/intel/tigerlake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/sys... PS5, Line 86: 28 Could we get a symbolic constant for this? Something like MOBILE_SKU_PL1_TIME_SEC ?
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: oc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39346/5//COMMIT_MSG@7 PS5, Line 7: oc/ soc/
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: oc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/msr.h:
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/inc... PS5, Line 23: Remove tap
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/inc... PS5, Line 25: Remove tap
Hello build bot (Jenkins), Furquan Shaikh, Caveh Jalali, Tim Wawrzynczak, Subrata Banik, Nick Vaccaro, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#6).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support to configure values.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CL:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- A src/soc/intel/tigerlake/acpi/dptf.asl M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/tigerlake/fsp_params_tgl.c M src/soc/intel/tigerlake/include/soc/msr.h M src/soc/intel/tigerlake/systemagent.c 6 files changed, 261 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/6
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 6:
Can you add Topic as TGL_UPSTREAM
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 6:
(2 comments)
Patch Set 6:
Can you add Topic as TGL_UPSTREAM
Done.
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/chi... PS5, Line 174: psys_pmax
This isn't used anywhere; is that intentional?
We have plan to use this once closed system available.
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/cpu... PS5, Line 218: MCH_DDR_POWER_LIMIT_LO
I don't see coreboot setting these, are these set by FSP-M?
This is the same as per other platform implementation.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39346/6//COMMIT_MSG@16 PS6, Line 16: CL this goes to the wrong place. try CB:
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Roy Mingi Park, Puthikorn Voravootivat, Duncan Laurie, Tim Wawrzynczak, Subrata Banik, Nick Vaccaro, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#7).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support to configure values.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CB:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- A src/soc/intel/tigerlake/acpi/dptf.asl M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/tigerlake/fsp_params_tgl.c M src/soc/intel/tigerlake/include/soc/msr.h M src/soc/intel/tigerlake/systemagent.c 6 files changed, 261 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/7
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39346/6//COMMIT_MSG@16 PS6, Line 16: CL
this goes to the wrong place. […]
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/7/src/soc/intel/tigerlake/sys... File src/soc/intel/tigerlake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/39346/7/src/soc/intel/tigerlake/sys... PS7, Line 84: /* Configure turbo power limits 1ms after reset complete bit */ Is that documented in some datasheet?
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 7:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/chi... PS5, Line 174: psys_pmax
We have plan to use this once closed system available.
Done
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/cpu... PS5, Line 218: MCH_DDR_POWER_LIMIT_LO
This is the same as per other platform implementation.
Done
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/msr.h:
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/inc... PS5, Line 23:
Remove tap
Done
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/inc... PS5, Line 25:
Remove tap
Done
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/sys... File src/soc/intel/tigerlake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/39346/5/src/soc/intel/tigerlake/sys... PS5, Line 86: 28
Could we get a symbolic constant for this? Something like MOBILE_SKU_PL1_TIME_SEC ?
Ack
https://review.coreboot.org/c/coreboot/+/39346/7/src/soc/intel/tigerlake/sys... File src/soc/intel/tigerlake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/39346/7/src/soc/intel/tigerlake/sys... PS7, Line 84: /* Configure turbo power limits 1ms after reset complete bit */
Is that documented in some datasheet?
this documented in EDS (External design specification) document
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39346/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39346/5//COMMIT_MSG@7 PS5, Line 7: oc/
soc/
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... File src/soc/intel/tigerlake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... PS4, Line 75: /* Configure turbo power limits 1ms after reset complete bit */ : mdelay(1);
We will get back on this.
Done
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/sys... PS4, Line 77: 28
Need to check this kind of implementation.
Done
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Patrick Rudolph, Venkata Krishna Nimmagadda, Caveh Jalali, Roy Mingi Park, Puthikorn Voravootivat, Tim Wawrzynczak, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#8).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support to configure values.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CB:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- A src/soc/intel/tigerlake/acpi/dptf.asl M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/cpu.c M src/soc/intel/tigerlake/fsp_params_tgl.c M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/include/soc/msr.h M src/soc/intel/tigerlake/systemagent.c 7 files changed, 265 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/8
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/8/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/8/src/soc/intel/tigerlake/cpu... PS8, Line 103: void set_power_limits(u8 power_limit_1_time) This whole thing looks awfully similar to skylake and cannonlake. Seems like this might be another good candidate to move to soc/intel/common ? set_power_limits() can then take const pointers to the sec_to_msr and msr_to_sec arrays if they differ between SoCs.
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Patrick Rudolph, Venkata Krishna Nimmagadda, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#9).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support to under soc/intel/common code. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CB:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/common/Makefile.inc A src/soc/intel/common/power_limit.c A src/soc/intel/common/power_limit.h A src/soc/intel/tigerlake/acpi/dptf.asl M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/include/soc/msr.h M src/soc/intel/tigerlake/systemagent.c 9 files changed, 313 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/9
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/8/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/8/src/soc/intel/tigerlake/cpu... PS8, Line 103: void set_power_limits(u8 power_limit_1_time)
This whole thing looks awfully similar to skylake and cannonlake. […]
I have submitted new patch for adding this functionality under soc/intel/common. Also, tested this with tigerlake platform. Next activity would be cleaning up this set_power_limits() functionalities from skylake and cannonlake platforms if required.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/8/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/8/src/soc/intel/tigerlake/cpu... PS8, Line 103: void set_power_limits(u8 power_limit_1_time)
I have submitted new patch for adding this functionality under soc/intel/common. […]
Thanks Sumeet! I can take a look at skylake & cannonlake if you want.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/8/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/8/src/soc/intel/tigerlake/cpu... PS8, Line 103: void set_power_limits(u8 power_limit_1_time)
Thanks Sumeet! I can take a look at skylake & cannonlake if you want.
Actually that's why the build is failing with this patch:
src/soc/intel/cannonlake/cpu.c:100: multiple definition of `set_power_limits'
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Patrick Rudolph, Venkata Krishna Nimmagadda, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#10).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake and skylake. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CB:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/common/Makefile.inc A src/soc/intel/common/power_limit.c A src/soc/intel/common/power_limit.h M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h A src/soc/intel/tigerlake/acpi/dptf.asl M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/include/soc/msr.h M src/soc/intel/tigerlake/systemagent.c 14 files changed, 313 insertions(+), 392 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/10
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/8/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/8/src/soc/intel/tigerlake/cpu... PS8, Line 103: void set_power_limits(u8 power_limit_1_time)
Actually that's why the build is failing with this patch: […]
Thanks Tim for this info. I had tried to address this build failure in patchset 10. Please, take a look for this skylake and cannonlake required changes.
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Patrick Rudolph, Venkata Krishna Nimmagadda, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#11).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake and skylake. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CB:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/common/Makefile.inc A src/soc/intel/common/power_limit.c A src/soc/intel/common/power_limit.h M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h A src/soc/intel/tigerlake/acpi/dptf.asl M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/include/soc/msr.h M src/soc/intel/tigerlake/systemagent.c 14 files changed, 312 insertions(+), 392 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/11
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Patrick Rudolph, Venkata Krishna Nimmagadda, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#12).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake, skylake and broadwell. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CB:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/broadwell/cpu.c M src/soc/intel/broadwell/include/soc/cpu.h M src/soc/intel/broadwell/systemagent.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/cannonlake/systemagent.c M src/soc/intel/common/Makefile.inc A src/soc/intel/common/power_limit.c A src/soc/intel/common/power_limit.h M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h M src/soc/intel/skylake/systemagent.c A src/soc/intel/tigerlake/acpi/dptf.asl M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params_tgl.c M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/include/soc/msr.h M src/soc/intel/tigerlake/systemagent.c 19 files changed, 318 insertions(+), 533 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/12
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 12:
(1 comment)
So now we just need a little refactor in order to get the board-specific power limit overrides out of their SoC configuration structs from the respective chip.h files.
How about creating a new struct for this? I'm thinking something like: struct soc_power_limits { /* PL1 Override value in Watts */ uint32_t tdp_pl1_override; /* PL2 Override value in Watts */ uint32_t tdp_pl2_override; /* SysPL2 Value in Watts */ uint32_t tdp_psyspl2; /* SysPL3 Value in Watts */ uint32_t tdp_psyspl3; /* SysPL3 window size */ uint32_t tdp_psyspl3_time; /* SysPL3 duty cycle */ uint32_t tdp_psyspl3_dutycycle; /* PL4 Value in Watts */ uint32_t tdp_pl4; /* Estimated maximum platform power in Watts */ uint16_t psys_pmax; };
Then soc_systemagent_init() could grab the config, get this new soc_power_limits out of it, and then pass that into set_power_limits().
What do you think?
https://review.coreboot.org/c/coreboot/+/39346/12/src/soc/intel/common/power... File src/soc/intel/common/power_limit.c:
https://review.coreboot.org/c/coreboot/+/39346/12/src/soc/intel/common/power... PS12, Line 206: msr = rdmsr(MSR_CONFIG_TDP_NOMINAL); This statement has no effect.
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Patrick Rudolph, Venkata Krishna Nimmagadda, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#13).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake, skylake and broadwell. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CB:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/octopus/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/coral/devicetree.cb M src/mainboard/google/reef/variants/pyro/devicetree.cb M src/mainboard/google/reef/variants/sand/devicetree.cb M src/mainboard/google/reef/variants/snappy/devicetree.cb M src/mainboard/intel/glkrvp/variants/baseboard/devicetree.cb M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/soc/intel/broadwell/cpu.c M src/soc/intel/broadwell/include/soc/cpu.h M src/soc/intel/broadwell/systemagent.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/cannonlake/systemagent.c A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/jasperlake/include/soc/cpu.h M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h M src/soc/intel/skylake/systemagent.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/systemagent.c 28 files changed, 337 insertions(+), 561 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/13
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 13:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 120: uint32_t tdp_psyspl2; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 122: uint32_t tdp_psyspl3; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 124: uint32_t tdp_psyspl3_time; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 126: uint32_t tdp_psyspl3_dutycycle; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 128: uint32_t tdp_pl4; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 130: uint16_t psys_pmax; please, no spaces at the start of a line
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Andrey Petrov, Patrick Rudolph, Venkata Krishna Nimmagadda, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#14).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake, skylake and broadwell. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CB:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/octopus/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/coral/devicetree.cb M src/mainboard/google/reef/variants/pyro/devicetree.cb M src/mainboard/google/reef/variants/sand/devicetree.cb M src/mainboard/google/reef/variants/snappy/devicetree.cb M src/mainboard/intel/glkrvp/variants/baseboard/devicetree.cb M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/soc/intel/broadwell/cpu.c M src/soc/intel/broadwell/include/soc/cpu.h M src/soc/intel/broadwell/systemagent.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/cannonlake/systemagent.c A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/jasperlake/include/soc/cpu.h M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h M src/soc/intel/skylake/systemagent.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/systemagent.c 28 files changed, 322 insertions(+), 561 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/14
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Andrey Petrov, Patrick Rudolph, Venkata Krishna Nimmagadda, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#15).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake, skylake and broadwell. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CB:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/octopus/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/coral/devicetree.cb M src/mainboard/google/reef/variants/pyro/devicetree.cb M src/mainboard/google/reef/variants/sand/devicetree.cb M src/mainboard/google/reef/variants/snappy/devicetree.cb M src/mainboard/intel/glkrvp/variants/baseboard/devicetree.cb M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/acpi.c M src/soc/intel/broadwell/cpu.c M src/soc/intel/broadwell/include/soc/cpu.h M src/soc/intel/broadwell/include/soc/msr.h M src/soc/intel/broadwell/systemagent.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/cannonlake/systemagent.c A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/jasperlake/include/soc/cpu.h M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h M src/soc/intel/skylake/systemagent.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/systemagent.c 33 files changed, 330 insertions(+), 565 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/15
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 15:
This is great Sumeet! Only thing I see is broadwell doesn't have a soc/soc_chip.h file.
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Andrey Petrov, Patrick Rudolph, Venkata Krishna Nimmagadda, Sumeet Pawnikar, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#16).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake, skylake and broadwell. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CB:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/octopus/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/coral/devicetree.cb M src/mainboard/google/reef/variants/pyro/devicetree.cb M src/mainboard/google/reef/variants/sand/devicetree.cb M src/mainboard/google/reef/variants/snappy/devicetree.cb M src/mainboard/intel/glkrvp/variants/baseboard/devicetree.cb M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/acpi.c M src/soc/intel/broadwell/cpu.c M src/soc/intel/broadwell/include/soc/cpu.h M src/soc/intel/broadwell/include/soc/msr.h A src/soc/intel/broadwell/include/soc/soc_chip.h M src/soc/intel/broadwell/systemagent.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/cannonlake/systemagent.c A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/jasperlake/include/soc/cpu.h M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h M src/soc/intel/skylake/systemagent.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/systemagent.c 34 files changed, 350 insertions(+), 565 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/16
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 16:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 120: uint32_t tdp_psyspl2;
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 122: uint32_t tdp_psyspl3;
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 124: uint32_t tdp_psyspl3_time;
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 126: uint32_t tdp_psyspl3_dutycycle;
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 128: uint32_t tdp_pl4;
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/39346/13/src/soc/intel/apollolake/c... PS13, Line 130: uint16_t psys_pmax;
please, no spaces at the start of a line
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 16:
cpu_get_tdp_nominal_ratio for Broadwell?
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 16:
Patch Set 16:
cpu_get_tdp_nominal_ratio for Broadwell?
Yes. Interstringly this error is coming only for Broadwell it seems as per https://qa.coreboot.org/job/coreboot-gerrit/122078/.
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Andrey Petrov, Patrick Rudolph, Venkata Krishna Nimmagadda, Sumeet Pawnikar, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#17).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake, skylake and broadwell. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
CQ-DEPEND=CB:39345 Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/octopus/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/coral/devicetree.cb M src/mainboard/google/reef/variants/pyro/devicetree.cb M src/mainboard/google/reef/variants/sand/devicetree.cb M src/mainboard/google/reef/variants/snappy/devicetree.cb M src/mainboard/intel/glkrvp/variants/baseboard/devicetree.cb M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/acpi.c M src/soc/intel/broadwell/chip.h M src/soc/intel/broadwell/cpu.c M src/soc/intel/broadwell/include/soc/cpu.h M src/soc/intel/broadwell/include/soc/msr.h A src/soc/intel/broadwell/include/soc/soc_chip.h M src/soc/intel/broadwell/systemagent.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/cannonlake/systemagent.c A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/jasperlake/include/soc/cpu.h M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h M src/soc/intel/skylake/systemagent.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/systemagent.c 35 files changed, 353 insertions(+), 575 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/17
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/12/src/soc/intel/common/power... File src/soc/intel/common/power_limit.c:
https://review.coreboot.org/c/coreboot/+/39346/12/src/soc/intel/common/power... PS12, Line 206: msr = rdmsr(MSR_CONFIG_TDP_NOMINAL);
This statement has no effect.
Done
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 17:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39346/17/src/mainboard/intel/glkrvp... File src/mainboard/intel/glkrvp/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39346/17/src/mainboard/intel/glkrvp... PS17, Line 58: 7.5W setting gives a run-time 6W actual to what extent does this still hold?
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 115: mW just W
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 117: mW W
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 322: / 1000 really?
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 342: %d does this need to be %02d?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/common/block... File src/soc/intel/common/block/power_limit/power_limit.c:
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/common/block... PS17, Line 206: msr 'msr' isnt' used after this. Does it require a read at this point?
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 17:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39346/17/src/mainboard/intel/glkrvp... File src/mainboard/intel/glkrvp/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39346/17/src/mainboard/intel/glkrvp... PS17, Line 58: 7.5W setting gives a run-time 6W actual
to what extent does this still hold?
This was based on glkrvp reference open board during early enablement.
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 115: mW
just W
Done
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 117: mW
W
Done
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 322: / 1000
really?
This is not required any more.
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 342: %d
does this need to be %02d?
Ack
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/common/block... File src/soc/intel/common/block/power_limit/power_limit.c:
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/common/block... PS17, Line 206: msr
'msr' isnt' used after this. […]
This is not required. Thanks for finding this.
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Andrey Petrov, Patrick Rudolph, Venkata Krishna Nimmagadda, Sumeet Pawnikar, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#18).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake, skylake, apollolake and broadwell. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/mainboard/google/octopus/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/baseboard/devicetree.cb M src/mainboard/google/reef/variants/coral/devicetree.cb M src/mainboard/google/reef/variants/pyro/devicetree.cb M src/mainboard/google/reef/variants/sand/devicetree.cb M src/mainboard/google/reef/variants/snappy/devicetree.cb M src/mainboard/intel/glkrvp/variants/baseboard/devicetree.cb M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/acpi.c M src/soc/intel/broadwell/chip.h M src/soc/intel/broadwell/cpu.c M src/soc/intel/broadwell/include/soc/cpu.h M src/soc/intel/broadwell/include/soc/msr.h A src/soc/intel/broadwell/include/soc/soc_chip.h M src/soc/intel/broadwell/systemagent.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/cannonlake/systemagent.c A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/jasperlake/include/soc/cpu.h M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h M src/soc/intel/skylake/systemagent.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/systemagent.c 35 files changed, 355 insertions(+), 578 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/18
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/18/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/c/coreboot/+/39346/18/src/soc/intel/apollolake/c... PS18, Line 344: d %02d here as well.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/18/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/c/coreboot/+/39346/18/src/soc/intel/apollolake/c... PS18, Line 344: d
%02d here as well.
Ack
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Andrey Petrov, Patrick Rudolph, Venkata Krishna Nimmagadda, Sumeet Pawnikar, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#19).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake, skylake, apollolake and broadwell. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/acpi.c M src/soc/intel/broadwell/chip.h M src/soc/intel/broadwell/cpu.c M src/soc/intel/broadwell/include/soc/cpu.h M src/soc/intel/broadwell/include/soc/msr.h A src/soc/intel/broadwell/include/soc/soc_chip.h M src/soc/intel/broadwell/systemagent.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/cannonlake/systemagent.c A src/soc/intel/common/acpi/dptf.asl A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/include/soc/cpu.h M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h M src/soc/intel/skylake/systemagent.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/systemagent.c 33 files changed, 387 insertions(+), 599 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/19
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 19:
Devicetree entries need to be updated to reference "power_limits_config":
register "power_limits_config.tdp_pl1_override" = ..., etc.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 19:
Patch Set 19:
Devicetree entries need to be updated to reference "power_limits_config":
register "power_limits_config.tdp_pl1_override" = ..., etc.
Yes. Updating these in respective files and submitted https://review.coreboot.org/c/coreboot/+/40340 for review. Thanks.
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Andrey Petrov, Patrick Rudolph, Venkata Krishna Nimmagadda, Sumeet Pawnikar, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#20).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake, skylake, apollolake and broadwell. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h A src/soc/intel/apollolake/include/soc/msr.h M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/acpi.c M src/soc/intel/broadwell/chip.h M src/soc/intel/broadwell/cpu.c M src/soc/intel/broadwell/include/soc/cpu.h M src/soc/intel/broadwell/include/soc/msr.h A src/soc/intel/broadwell/include/soc/soc_chip.h M src/soc/intel/broadwell/systemagent.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/fsp_params.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/cannonlake/systemagent.c A src/soc/intel/common/acpi/dptf.asl A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/include/soc/cpu.h M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h M src/soc/intel/skylake/systemagent.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/systemagent.c 37 files changed, 409 insertions(+), 668 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/20
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Andrey Petrov, Patrick Rudolph, Venkata Krishna Nimmagadda, Sumeet Pawnikar, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#22).
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
soc/intel/tigerlake: Add processor power limits control support
Add processor power limits control support under soc/intel/common code. Remove the previous processor specific power limit control support from cannonlake, icelake, skylake, apollolake and broadwell. Also, configure these processor power limit controls for tigerlake platform.
BRANCH=None BUG=b:149722146 TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h A src/soc/intel/apollolake/include/soc/msr.h M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/acpi.c M src/soc/intel/broadwell/chip.h M src/soc/intel/broadwell/cpu.c M src/soc/intel/broadwell/include/soc/cpu.h M src/soc/intel/broadwell/include/soc/msr.h A src/soc/intel/broadwell/include/soc/soc_chip.h M src/soc/intel/broadwell/systemagent.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/cpu.c M src/soc/intel/cannonlake/fsp_params.c M src/soc/intel/cannonlake/include/soc/cpu.h M src/soc/intel/cannonlake/systemagent.c A src/soc/intel/common/acpi/dptf.asl A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c M src/soc/intel/icelake/include/soc/cpu.h M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/include/soc/cpu.h M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/include/soc/cpu.h M src/soc/intel/skylake/systemagent.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/include/soc/cpu.h M src/soc/intel/tigerlake/systemagent.c 37 files changed, 427 insertions(+), 668 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/22
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 22:
(11 comments)
This is looking pretty good! Thanks, Sumeet. I think breaking this up into a few CLs might make it a little more digestible.
My suggestion: 1) A patch to add the set_power_limits functionality into soc/intel/common 2) A patch to add the Kconfigs to enable the one in soc/intel/common (CPU_INTEL_COMMON & SOC_INTEL_COMMON_BLOCK_POWER_LIMIT and friends), and also remove the extra versions of set_power_limits(). This will be the big one as it will also have the struct power_limits_config that needs to be handled everywhere. 3) Continue like you did on top of this patch.
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/cannonlake/f... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/cannonlake/f... PS22, Line 157: struct soc_power_limits_config *soc_confg; const
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/acpi/... File src/soc/intel/common/acpi/dptf.asl:
PS22: What made this file necessary?
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/power_limit.h:
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 28: Used to configure Maybe say 'default' ?
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 33: uint32_t This should be OK as uint8, I don't think you can set PL1 more than 255 W 😊 uint16_t if we want to consider the case of desktop or server SKUs at some point in the future.
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 35: tdp_pl2_override Same thing? will uint8_t or uint16_t work?
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 37: tdp_psyspl2 Same
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 39: tdp_psyspl3 Same
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 41: tdp_psyspl3_time Same
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 43: tdp_psyspl3_dutycycle Same
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 45: tdp_pl4 Same
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 47: psys_pmax Same
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 22:
(10 comments)
Patch Set 22:
(11 comments)
This is looking pretty good! Thanks, Sumeet. I think breaking this up into a few CLs might make it a little more digestible.
My suggestion:
- A patch to add the set_power_limits functionality into soc/intel/common
- A patch to add the Kconfigs to enable the one in soc/intel/common (CPU_INTEL_COMMON & SOC_INTEL_COMMON_BLOCK_POWER_LIMIT and friends), and also remove the extra versions of set_power_limits(). This will be the big one as it will also have the struct power_limits_config that needs to be handled everywhere.
- Continue like you did on top of this patch.
Thanks for these suggestions. I am working on this and will try to incorporate. Still I observe there is dependencies between mb/google/.../devicetree.cb files and src/soc/intel/.../* files.
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/cannonlake/f... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/cannonlake/f... PS22, Line 157: struct soc_power_limits_config *soc_confg;
const
Ack
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/power_limit.h:
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 28: Used to configure
Maybe say 'default' ?
Ok
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 33: uint32_t
This should be OK as uint8, I don't think you can set PL1 more than 255 W 😊 uint16_t if we want to […]
I would consider to use uint16_t considering the need in the future.
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 35: tdp_pl2_override
Same thing? will uint8_t or uint16_t work?
planning to use uint16_t
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 37: tdp_psyspl2
Same
same as above, planning to use uint16_t
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 39: tdp_psyspl3
Same
same as above, planning to use uint16_t
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 41: tdp_psyspl3_time
Same
same as above, planning to use uint16_t
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 43: tdp_psyspl3_dutycycle
Same
same as above, planning to use uint16_t
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 45: tdp_pl4
Same
same as above planning to use uint16_t
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 47: psys_pmax
Same
same as above planning to use uint16_t
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 22:
Patch Set 22:
(10 comments)
Patch Set 22:
(11 comments)
This is looking pretty good! Thanks, Sumeet. I think breaking this up into a few CLs might make it a little more digestible.
My suggestion:
- A patch to add the set_power_limits functionality into soc/intel/common
- A patch to add the Kconfigs to enable the one in soc/intel/common (CPU_INTEL_COMMON & SOC_INTEL_COMMON_BLOCK_POWER_LIMIT and friends), and also remove the extra versions of set_power_limits(). This will be the big one as it will also have the struct power_limits_config that needs to be handled everywhere.
- Continue like you did on top of this patch.
Thanks for these suggestions. I am working on this and will try to incorporate. Still I observe there is dependencies between mb/google/.../devicetree.cb files and src/soc/intel/.../* files.
Yes, there is a dependency. I think it can be broken by first just adding a patch to copy the functionality to common code, then another patch to switch the devicetree's over to the new common code, and then one last one to remove the old set_power_limits() functions from each SoC. I think that will compile and also make each patch easier to review.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 22:
Patch Set 22:
(10 comments)
Patch Set 22:
(11 comments)
This is looking pretty good! Thanks, Sumeet. I think breaking this up into a few CLs might make it a little more digestible.
My suggestion:
- A patch to add the set_power_limits functionality into soc/intel/common
- A patch to add the Kconfigs to enable the one in soc/intel/common (CPU_INTEL_COMMON & SOC_INTEL_COMMON_BLOCK_POWER_LIMIT and friends), and also remove the extra versions of set_power_limits(). This will be the big one as it will also have the struct power_limits_config that needs to be handled everywhere.
- Continue like you did on top of this patch.
Thanks for these suggestions. I am working on this and will try to incorporate. Still I observe there is dependencies between mb/google/.../devicetree.cb files and src/soc/intel/.../* files.
In reply to above suggestion, 1) can be done. For 2) even after moving out Kconfigs in a separate patch, I observed that there is still dependencies for src/soc/intel/…/(chip/cpu/fsp_param/systemagent) files on src/mainboard/google/…/devicetree.cb files for their respective SoC products/systems. Here, these dependencies are on member variables of soc_power_limits_config structure like tdp_pl1_override, tdp_pl2_override, etc. I am getting errors like below, which shows real dependencies between src/mainboard/google/.../devicetree.cb files and src/soc/intel/.../* files. These are the same errors as reported by Jenkin https://qa.coreboot.org/job/coreboot-gerrit/124944/consoleText for this patch.
SCONFIG mainboard/google/drallion/variants/drallion/devicetree.cb CC bootblock/mainboard/google/drallion/static.o build/mainboard/google/drallion/static.c:336:3: error: 'const struct soc_intel_cannonlake_config' has no member named 'tdp_pl1_override'; did you mean 'cpu_ratio_override'? .tdp_pl1_override = 25,
To address these kind of dependencies, I can think of below solution. Create different patch based on SoC types like one single patch for Cannonlake type which has all src/soc/intel/cannonlake/*files and src/mainboard/google/…/devicetree.cb files of Cannonlake based products. Add another patch for Skylake with src/soc/intel/skylake/*files and src/mainboard/google/…/devicetree.cb file of skylake based products. This way we can segregate all different SoC specific changes in different patches which can be easy for track, review, portability and cherry-picking for product specific firmware branches. I feel this approach fixes Jenkin build failures as well. Also, this SoC specific separate patches will be smaller with comparison of current ones.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 22:
Patch Set 22:
Patch Set 22:
(10 comments)
Patch Set 22:
(11 comments)
This is looking pretty good! Thanks, Sumeet. I think breaking this up into a few CLs might make it a little more digestible.
My suggestion:
- A patch to add the set_power_limits functionality into soc/intel/common
- A patch to add the Kconfigs to enable the one in soc/intel/common (CPU_INTEL_COMMON & SOC_INTEL_COMMON_BLOCK_POWER_LIMIT and friends), and also remove the extra versions of set_power_limits(). This will be the big one as it will also have the struct power_limits_config that needs to be handled everywhere.
- Continue like you did on top of this patch.
Thanks for these suggestions. I am working on this and will try to incorporate. Still I observe there is dependencies between mb/google/.../devicetree.cb files and src/soc/intel/.../* files.
In reply to above suggestion, 1) can be done. For 2) even after moving out Kconfigs in a separate patch, I observed that there is still dependencies for src/soc/intel/…/(chip/cpu/fsp_param/systemagent) files on src/mainboard/google/…/devicetree.cb files for their respective SoC products/systems. Here, these dependencies are on member variables of soc_power_limits_config structure like tdp_pl1_override, tdp_pl2_override, etc. I am getting errors like below, which shows real dependencies between src/mainboard/google/.../devicetree.cb files and src/soc/intel/.../* files. These are the same errors as reported by Jenkin https://qa.coreboot.org/job/coreboot-gerrit/124944/consoleText for this patch.
SCONFIG mainboard/google/drallion/variants/drallion/devicetree.cb CC bootblock/mainboard/google/drallion/static.obuild/mainboard/google/drallion/static.c:336:3: error: 'const struct soc_intel_cannonlake_config' has no member named 'tdp_pl1_override'; did you mean 'cpu_ratio_override'? .tdp_pl1_override = 25,
To address these kind of dependencies, I can think of below solution. Create different patch based on SoC types like one single patch for Cannonlake type which has all src/soc/intel/cannonlake/*files and src/mainboard/google/…/devicetree.cb files of Cannonlake based products. Add another patch for Skylake with src/soc/intel/skylake/*files and src/mainboard/google/…/devicetree.cb file of skylake based products. This way we can segregate all different SoC specific changes in different patches which can be easy for track, review, portability and cherry-picking for product specific firmware branches. I feel this approach fixes Jenkin build failures as well. Also, this SoC specific separate patches will be smaller with comparison of current ones.
That would work, and that does work better for later cherry-picks if we do that!
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 22:
Patch Set 22:
Patch Set 22:
(10 comments)
Patch Set 22:
(11 comments)
This is looking pretty good! Thanks, Sumeet. I think breaking this up into a few CLs might make it a little more digestible.
My suggestion:
- A patch to add the set_power_limits functionality into soc/intel/common
- A patch to add the Kconfigs to enable the one in soc/intel/common (CPU_INTEL_COMMON & SOC_INTEL_COMMON_BLOCK_POWER_LIMIT and friends), and also remove the extra versions of set_power_limits(). This will be the big one as it will also have the struct power_limits_config that needs to be handled everywhere.
- Continue like you did on top of this patch.
Thanks for these suggestions. I am working on this and will try to incorporate. Still I observe there is dependencies between mb/google/.../devicetree.cb files and src/soc/intel/.../* files.
Yes, there is a dependency. I think it can be broken by first just adding a patch to copy the functionality to common code, then another patch to switch the devicetree's over to the new common code, and then one last one to remove the old set_power_limits() functions from each SoC. I think that will compile and also make each patch easier to review.
while entering my previous comments, I missed this one. sorry for that.
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Andrey Petrov, Patrick Rudolph, Venkata Krishna Nimmagadda, Sumeet Pawnikar, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#23).
Change subject: soc/intel/common: add processor power limits control support ......................................................................
soc/intel/common: add processor power limits control support
Add processor power limits control support under common code.
BRANCH=None BUG=None TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c 4 files changed, 271 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/23
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Andrey Petrov, Patrick Rudolph, Venkata Krishna Nimmagadda, Sumeet Pawnikar, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#24).
Change subject: soc/intel/common: add processor power limits control support ......................................................................
soc/intel/common: add processor power limits control support
Add processor power limits control support under common code.
BRANCH=None BUG=None TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c 4 files changed, 271 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/24
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/common: add processor power limits control support ......................................................................
Patch Set 24:
(10 comments)
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/acpi/... File src/soc/intel/common/acpi/dptf.asl:
PS22:
What made this file necessary?
This is required to add TCPU zone for thermal control.
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/power_limit.h:
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 28: Used to configure
Ok
Done
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 33: uint32_t
I would consider to use uint16_t considering the need in the future.
Done
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 35: tdp_pl2_override
planning to use uint16_t
Done
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 37: tdp_psyspl2
same as above, planning to use uint16_t
Done
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 39: tdp_psyspl3
same as above, planning to use uint16_t
Done
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 41: tdp_psyspl3_time
same as above, planning to use uint16_t
Done
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 43: tdp_psyspl3_dutycycle
same as above, planning to use uint16_t
Done
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 45: tdp_pl4
same as above planning to use uint16_t
Done
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/block... PS22, Line 47: psys_pmax
same as above planning to use uint16_t
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/common: add processor power limits control support ......................................................................
Patch Set 24:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/power_limit.h:
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... PS24, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright (C) 2020 Intel Corporation : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */ coreboot now uses SPDX headers:
/* This file is part of the coreboot project. */ /* SPDX-License-Identifier: GPL-2.0-or-later */
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... PS24, Line 25: nit: extra tab
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... File src/soc/intel/common/block/power_limit/power_limit.c:
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... PS24, Line 14: */ coreboot now uses SPDX headers:
/* This file is part of the coreboot project. */ /* SPDX-License-Identifier: GPL-2.0-or-later */
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/common: add processor power limits control support ......................................................................
Patch Set 24:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... File src/soc/intel/common/block/power_limit/power_limit.c:
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... PS24, Line 14: */
coreboot now uses SPDX headers: […]
And we even dropped the "file is part of" line
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/common: add processor power limits control support ......................................................................
Patch Set 24:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/power_limit.h:
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... PS24, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright (C) 2020 Intel Corporation : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */
coreboot now uses SPDX headers: […]
keep only : /* SPDX-License-Identifier: GPL-2.0-or-later */
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... File src/soc/intel/common/block/power_limit/power_limit.c:
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... PS24, Line 14: */
And we even dropped the "file is part of" line
please keep only "/* SPDX-License-Identifier: GPL-2.0-or-later */"
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/common: add processor power limits control support ......................................................................
Patch Set 24:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/power_limit.h:
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... PS24, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright (C) 2020 Intel Corporation : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */
keep only : […]
Done
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... PS24, Line 25:
nit: extra tab
Done
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... File src/soc/intel/common/block/power_limit/power_limit.c:
https://review.coreboot.org/c/coreboot/+/39346/24/src/soc/intel/common/block... PS24, Line 14: */
please keep only "/* SPDX-License-Identifier: GPL-2. […]
Done
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Subrata Banik, Andrey Petrov, Patrick Rudolph, Venkata Krishna Nimmagadda, Sumeet Pawnikar, Martin Roth, Caveh Jalali, Roy Mingi Park, Tim Wawrzynczak, Puthikorn Voravootivat, Todd Broch, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39346
to look at the new patch set (#25).
Change subject: soc/intel/common: add processor power limits control support ......................................................................
soc/intel/common: add processor power limits control support
Add processor power limits control support under common code.
BRANCH=None BUG=None TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c 4 files changed, 245 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/39346/25
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/common: add processor power limits control support ......................................................................
Patch Set 25: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/common: add processor power limits control support ......................................................................
Patch Set 25:
Would you mind stacking these all in one single chain? Whenever I go visit a specific SoC-change on top of this, it seems they all have the same parent, which is this one.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/common: add processor power limits control support ......................................................................
Patch Set 25:
(2 comments)
Patch Set 25:
Would you mind stacking these all in one single chain? Whenever I go visit a specific SoC-change on top of this, it seems they all have the same parent, which is this one.
I have created this intentionally because I did not find any dependencies between these SoC-changes with each other, for an example there is no dependencies between tigerlake patch and skylake patch. Also, I though if all these in one single chain, that would create unnecessary dependencies between SoC-chagnes and if one breaks in Jenkin build, all other fails considering dependencies between them. So, keeping independent this approach reduces Jenkin build failures as well. Keeping them as independent would help to track, review, portability and easy to cherry pick for product specific firmware branches. Please, correct if this way of independent implemention is improper or suggest any other better approach. Thanks.
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/cannonlake/f... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/cannonlake/f... PS22, Line 157: struct soc_power_limits_config *soc_confg;
Ack
Done
https://review.coreboot.org/c/coreboot/+/39346/22/src/soc/intel/common/acpi/... File src/soc/intel/common/acpi/dptf.asl:
PS22:
This is required to add TCPU zone for thermal control.
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/common: add processor power limits control support ......................................................................
soc/intel/common: add processor power limits control support
Add processor power limits control support under common code.
BRANCH=None BUG=None TEST=Built and checked this entry on Volteer system, cat /sys/class/powercap/intel-rapl/intel-rapl:0/*
Change-Id: I41fd95949aa2b02828aa2d13d29b962cb579904a Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39346 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- A src/soc/intel/common/block/include/intelblocks/power_limit.h A src/soc/intel/common/block/power_limit/Kconfig A src/soc/intel/common/block/power_limit/Makefile.inc A src/soc/intel/common/block/power_limit/power_limit.c 4 files changed, 245 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/common/block/include/intelblocks/power_limit.h b/src/soc/intel/common/block/include/intelblocks/power_limit.h new file mode 100644 index 0000000..2fa25de --- /dev/null +++ b/src/soc/intel/common/block/include/intelblocks/power_limit.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef _SOC_INTEL_COMMON_BLOCK_POWER_LIMIT_H_ +#define _SOC_INTEL_COMMON_BLOCK_POWER_LIMIT_H_ + +#define MCH_PKG_POWER_LIMIT_LO 0x59a0 +#define MCH_PKG_POWER_LIMIT_HI 0x59a4 +#define MCH_DDR_POWER_LIMIT_LO 0x58e0 +#define MCH_DDR_POWER_LIMIT_HI 0x58e4 + +#define MSR_VR_CURRENT_CONFIG 0x601 +#define MSR_PL3_CONTROL 0x615 +#define MSR_PLATFORM_POWER_LIMIT 0x65c + +/* Default power limit value in secs */ +#define MOBILE_SKU_PL1_TIME_SEC 28 + +struct soc_power_limits_config { + /* PL1 Override value in Watts */ + uint16_t tdp_pl1_override; + /* PL2 Override value in Watts */ + uint16_t tdp_pl2_override; + /* SysPL2 Value in Watts */ + uint16_t tdp_psyspl2; + /* SysPL3 Value in Watts */ + uint16_t tdp_psyspl3; + /* SysPL3 window size */ + uint32_t tdp_psyspl3_time; + /* SysPL3 duty cycle */ + uint32_t tdp_psyspl3_dutycycle; + /* PL4 Value in Watts */ + uint16_t tdp_pl4; + /* Estimated maximum platform power in Watts */ + uint16_t psys_pmax; +}; + +/* Configure power limits for turbo mode */ +void set_power_limits(u8 power_limit_1_time, + struct soc_power_limits_config *config); + +#endif /* _SOC_INTEL_COMMON_BLOCK_POWER_LIMIT_H_ */ diff --git a/src/soc/intel/common/block/power_limit/Kconfig b/src/soc/intel/common/block/power_limit/Kconfig new file mode 100644 index 0000000..5b2b348 --- /dev/null +++ b/src/soc/intel/common/block/power_limit/Kconfig @@ -0,0 +1,5 @@ +config SOC_INTEL_COMMON_BLOCK_POWER_LIMIT + bool + default n + help + This option allows to configure processor power limit values. diff --git a/src/soc/intel/common/block/power_limit/Makefile.inc b/src/soc/intel/common/block/power_limit/Makefile.inc new file mode 100644 index 0000000..83c41a7 --- /dev/null +++ b/src/soc/intel/common/block/power_limit/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_POWER_LIMIT) += power_limit.c diff --git a/src/soc/intel/common/block/power_limit/power_limit.c b/src/soc/intel/common/block/power_limit/power_limit.c new file mode 100644 index 0000000..2ac82b3 --- /dev/null +++ b/src/soc/intel/common/block/power_limit/power_limit.c @@ -0,0 +1,198 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <console/console.h> +#include <cpu/x86/msr.h> +#include <intelblocks/cpulib.h> +#include <intelblocks/power_limit.h> +#include <soc/msr.h> +#include <soc/soc_chip.h> +#include <soc/systemagent.h> + +/* Convert time in seconds to POWER_LIMIT_1_TIME MSR value */ +static const u8 power_limit_time_sec_to_msr[] = { + [0] = 0x00, + [1] = 0x0a, + [2] = 0x0b, + [3] = 0x4b, + [4] = 0x0c, + [5] = 0x2c, + [6] = 0x4c, + [7] = 0x6c, + [8] = 0x0d, + [10] = 0x2d, + [12] = 0x4d, + [14] = 0x6d, + [16] = 0x0e, + [20] = 0x2e, + [24] = 0x4e, + [28] = 0x6e, + [32] = 0x0f, + [40] = 0x2f, + [48] = 0x4f, + [56] = 0x6f, + [64] = 0x10, + [80] = 0x30, + [96] = 0x50, + [112] = 0x70, + [128] = 0x11, +}; + +/* Convert POWER_LIMIT_1_TIME MSR value to seconds */ +static const u8 power_limit_time_msr_to_sec[] = { + [0x00] = 0, + [0x0a] = 1, + [0x0b] = 2, + [0x4b] = 3, + [0x0c] = 4, + [0x2c] = 5, + [0x4c] = 6, + [0x6c] = 7, + [0x0d] = 8, + [0x2d] = 10, + [0x4d] = 12, + [0x6d] = 14, + [0x0e] = 16, + [0x2e] = 20, + [0x4e] = 24, + [0x6e] = 28, + [0x0f] = 32, + [0x2f] = 40, + [0x4f] = 48, + [0x6f] = 56, + [0x10] = 64, + [0x30] = 80, + [0x50] = 96, + [0x70] = 112, + [0x11] = 128, +}; + +/* + * Configure processor power limits if possible + * This must be done AFTER set of BIOS_RESET_CPL + */ +void set_power_limits(u8 power_limit_1_time, + struct soc_power_limits_config *conf) +{ + msr_t msr = rdmsr(MSR_PLATFORM_INFO); + msr_t limit; + unsigned int power_unit; + unsigned int tdp, min_power, max_power, max_time, tdp_pl2, tdp_pl1; + u8 power_limit_1_val; + + if (power_limit_1_time >= ARRAY_SIZE(power_limit_time_sec_to_msr)) + power_limit_1_time = + ARRAY_SIZE(power_limit_time_sec_to_msr) - 1; + + if (!(msr.lo & PLATFORM_INFO_SET_TDP)) + return; + + /* Get units */ + msr = rdmsr(MSR_PKG_POWER_SKU_UNIT); + power_unit = 1 << (msr.lo & 0xf); + + /* Get power defaults for this SKU */ + msr = rdmsr(MSR_PKG_POWER_SKU); + tdp = msr.lo & 0x7fff; + min_power = (msr.lo >> 16) & 0x7fff; + max_power = msr.hi & 0x7fff; + max_time = (msr.hi >> 16) & 0x7f; + + printk(BIOS_INFO, "CPU TDP = %u Watts\n", tdp / power_unit); + + if (power_limit_time_msr_to_sec[max_time] > power_limit_1_time) + power_limit_1_time = power_limit_time_msr_to_sec[max_time]; + + if (min_power > 0 && tdp < min_power) + tdp = min_power; + + if (max_power > 0 && tdp > max_power) + tdp = max_power; + + power_limit_1_val = power_limit_time_sec_to_msr[power_limit_1_time]; + + /* Set long term power limit to TDP */ + limit.lo = 0; + tdp_pl1 = ((conf->tdp_pl1_override == 0) ? + tdp : (conf->tdp_pl1_override * power_unit)); + printk(BIOS_INFO, "CPU PL1 = %u Watts\n", tdp_pl1 / power_unit); + limit.lo |= (tdp_pl1 & PKG_POWER_LIMIT_MASK); + + /* Set PL1 Pkg Power clamp bit */ + limit.lo |= PKG_POWER_LIMIT_CLAMP; + + limit.lo |= PKG_POWER_LIMIT_EN; + limit.lo |= (power_limit_1_val & PKG_POWER_LIMIT_TIME_MASK) << + PKG_POWER_LIMIT_TIME_SHIFT; + + /* Set short term power limit to 1.25 * TDP if no config given */ + limit.hi = 0; + tdp_pl2 = (conf->tdp_pl2_override == 0) ? + (tdp * 125) / 100 : (conf->tdp_pl2_override * power_unit); + printk(BIOS_INFO, "CPU PL2 = %u Watts\n", tdp_pl2 / power_unit); + limit.hi |= (tdp_pl2) & PKG_POWER_LIMIT_MASK; + limit.hi |= PKG_POWER_LIMIT_CLAMP; + limit.hi |= PKG_POWER_LIMIT_EN; + + /* Power limit 2 time is only programmable on server SKU */ + wrmsr(MSR_PKG_POWER_LIMIT, limit); + + /* Set PL2 power limit values in MCHBAR and disable PL1 */ + MCHBAR32(MCH_PKG_POWER_LIMIT_LO) = limit.lo & (~(PKG_POWER_LIMIT_EN)); + MCHBAR32(MCH_PKG_POWER_LIMIT_HI) = limit.hi; + + /* Set PsysPl2 */ + if (conf->tdp_psyspl2) { + limit = rdmsr(MSR_PLATFORM_POWER_LIMIT); + limit.hi = 0; + printk(BIOS_INFO, "CPU PsysPL2 = %u Watts\n", + conf->tdp_psyspl2); + limit.hi |= (conf->tdp_psyspl2 * power_unit) & + PKG_POWER_LIMIT_MASK; + limit.hi |= PKG_POWER_LIMIT_CLAMP; + limit.hi |= PKG_POWER_LIMIT_EN; + wrmsr(MSR_PLATFORM_POWER_LIMIT, limit); + } + + /* Set PsysPl3 */ + if (conf->tdp_psyspl3) { + limit = rdmsr(MSR_PL3_CONTROL); + limit.lo = 0; + printk(BIOS_INFO, "CPU PsysPL3 = %u Watts\n", + conf->tdp_psyspl3); + limit.lo |= (conf->tdp_psyspl3 * power_unit) & + PKG_POWER_LIMIT_MASK; + /* Enable PsysPl3 */ + limit.lo |= PKG_POWER_LIMIT_EN; + /* set PsysPl3 time window */ + limit.lo |= (conf->tdp_psyspl3_time & + PKG_POWER_LIMIT_TIME_MASK) << + PKG_POWER_LIMIT_TIME_SHIFT; + /* set PsysPl3 duty cycle */ + limit.lo |= (conf->tdp_psyspl3_dutycycle & + PKG_POWER_LIMIT_DUTYCYCLE_MASK) << + PKG_POWER_LIMIT_DUTYCYCLE_SHIFT; + wrmsr(MSR_PL3_CONTROL, limit); + } + + /* Set Pl4 */ + if (conf->tdp_pl4) { + limit = rdmsr(MSR_VR_CURRENT_CONFIG); + limit.lo = 0; + printk(BIOS_INFO, "CPU PL4 = %u Watts\n", conf->tdp_pl4); + limit.lo |= (conf->tdp_pl4 * power_unit) & + PKG_POWER_LIMIT_MASK; + wrmsr(MSR_VR_CURRENT_CONFIG, limit); + } + + /* Set DDR RAPL power limit by copying from MMIO to MSR */ + msr.lo = MCHBAR32(MCH_DDR_POWER_LIMIT_LO); + msr.hi = MCHBAR32(MCH_DDR_POWER_LIMIT_HI); + wrmsr(MSR_DDR_RAPL_LIMIT, msr); + + /* Use nominal TDP values for CPUs with configurable TDP */ + if (cpu_config_tdp_levels()) { + limit.hi = 0; + limit.lo = cpu_get_tdp_nominal_ratio(); + wrmsr(MSR_TURBO_ACTIVATION_RATIO, limit); + } +}
-
build bot (Jenkins) (Code Review) -
Caveh Jalali (Code Review)
-
caveh jalali (Code Review)
-
Furquan Shaikh (Code Review)
-
HAOUAS Elyes (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Sumeet R Pawnikar (Code Review)
-
Tim Wawrzynczak (Code Review)
-
Wonkyu Kim (Code Review)