Sumeet R Pawnikar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: [WIP] tigerlake: add TCC activation functionality ......................................................................
[WIP] tigerlake: add TCC activation functionality
This enables to configure the Thermal Control Circuit (TCC) activation value to new value as tcc_offset in degree Celcius. It prevents any abrupt thermal shutdown while running heavy workload. This helps to take early thermal throttling action before CPU temperature reaches maximum operating temperature TjMax value.
BUG=None BRANCH=None TEST=Built for volteer platform and verified the MSR value.
Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/tigerlake/cpu.c M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 26 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/41855/1
diff --git a/src/soc/intel/tigerlake/cpu.c b/src/soc/intel/tigerlake/cpu.c index f5870ec..a61e431 100644 --- a/src/soc/intel/tigerlake/cpu.c +++ b/src/soc/intel/tigerlake/cpu.c @@ -30,6 +30,26 @@ fsps_load(romstage_handoff_is_resume()); }
+static void configure_thermal_target(void) +{ + config_t *conf = config_of_soc(); + msr_t msr; + + /* Set TCC activation offset */ + msr = rdmsr(MSR_PLATFORM_INFO); + if ((msr.lo & (1 << 30)) && conf->tcc_offset) { + msr = rdmsr(MSR_TEMPERATURE_TARGET); + msr.lo &= ~(0xf << 24); + msr.lo |= (conf->tcc_offset & 0xf) << 24; + wrmsr(MSR_TEMPERATURE_TARGET, msr); + } + msr = rdmsr(MSR_TEMPERATURE_TARGET); + /* Time Window Tau Bits [6:0] */ + msr.lo &= ~0x7f; + msr.lo |= 0xe6; /* setting 100ms thermal time window */ + wrmsr(MSR_TEMPERATURE_TARGET, msr); +} + static void configure_isst(void) { config_t *conf = config_of_soc(); @@ -216,4 +236,7 @@ { if (mp_init_with_smm(cpu_bus, &mp_ops)) printk(BIOS_ERR, "MP initialization failure.\n"); + + /* Thermal throttle activation offset */ + configure_thermal_target(); } diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c index bdcd357..1aeddd6 100644 --- a/src/soc/intel/tigerlake/fsp_params.c +++ b/src/soc/intel/tigerlake/fsp_params.c @@ -205,6 +205,9 @@ /* Enable TCPU for processor thermal control */ params->Device4Enable = config->Device4Enable;
+ /* Set TccActivationOffset */ + params->TccActivationOffset = config->tcc_offset; + /* LAN */ dev = pcidev_path_on_root(PCH_DEVFN_GBE); if (!dev)
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: [WIP] tigerlake: add TCC activation functionality ......................................................................
Patch Set 1: Code-Review-1
We are working internally to get required FSP UPD for fixing this build issue. Till that time, making this as -1.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: [WIP] tigerlake: add TCC activation functionality ......................................................................
Patch Set 1: -Code-Review
This has dpendency on https://review.coreboot.org/c/coreboot/+/42243 waiting for this to get merge.
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Tim Wawrzynczak, Puthikorn Voravootivat, Subrata Banik, Raj Astekar, Patrick Rudolph, Venkata Krishna Nimmagadda,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41855
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: add TCC activation functionality ......................................................................
soc/intel/tigerlake: add TCC activation functionality
This enables to configure the Thermal Control Circuit (TCC) activation value to new value as tcc_offset in degree Celcius. It prevents any abrupt thermal shutdown while running heavy workload. This helps to take early thermal throttling action before CPU temperature reaches maximum operating temperature TjMax value.
BUG=None BRANCH=None TEST=Built for volteer platform and verified the MSR value.
Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/tigerlake/cpu.c M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 26 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/41855/2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/tigerlake: add TCC activation functionality ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41855/3/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/41855/3/src/soc/intel/tigerlake/cpu... PS3, Line 41: MSR_TEMPERATURE_TARGET Sumeet, this seems to be a common IA MSR. Would it make sense to move this functionality into soc/intel/common instead of tiger lake specific?
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/tigerlake: add TCC activation functionality ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41855/3/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/41855/3/src/soc/intel/tigerlake/cpu... PS3, Line 41: MSR_TEMPERATURE_TARGET
Sumeet, this seems to be a common IA MSR. […]
sure, let me check on this.
Hello Sumeet Pawnikar, build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Tim Wawrzynczak, Puthikorn Voravootivat, Subrata Banik, Raj Astekar, Patrick Rudolph, Venkata Krishna Nimmagadda,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41855
to look at the new patch set (#4).
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
soc/intel/common: add TCC activation functionality
This enables to configure the Thermal Control Circuit (TCC) activation value to new value as tcc_offset in degree Celcius. It prevents any abrupt thermal shutdown while running heavy workload. This helps to take early thermal throttling action before CPU temperature reaches maximum operating temperature TjMax value.
BUG=None BRANCH=None TEST=Built for volteer platform and verified the MSR value.
Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.corp-partner.google.com --- M src/soc/intel/common/block/cpu/cpulib.c M src/soc/intel/common/block/include/intelblocks/cpulib.h 2 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/41855/4
Hello Sumeet Pawnikar, build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Tim Wawrzynczak, Puthikorn Voravootivat, Subrata Banik, Raj Astekar, Patrick Rudolph, Venkata Krishna Nimmagadda,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41855
to look at the new patch set (#5).
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
soc/intel/common: add TCC activation functionality
This enables to configure the Thermal Control Circuit (TCC) activation value to new value as tcc_offset in degree Celcius. It prevents any abrupt thermal shutdown while running heavy workload. This helps to take early thermal throttling action before CPU temperature reaches maximum operating temperature TjMax value. Also, cleanup local functions from previous intel soc specific code base like for apollolake, broadwell, skylake, haswell and cannonlake.
BUG=None BRANCH=None TEST=Built for volteer platform and verified the MSR value.
Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.corp-partner.google.com --- M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/soc/intel/apollolake/romstage.c M src/soc/intel/broadwell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/cpulib.c M src/soc/intel/common/block/include/intelblocks/cpulib.h M src/soc/intel/skylake/cpu.c 9 files changed, 24 insertions(+), 139 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/41855/5
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41855/5/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/cpulib.c:
https://review.coreboot.org/c/coreboot/+/41855/5/src/soc/intel/common/block/... PS5, Line 265: (1 << 30) nit: BIT(30) (#include <types.h> if necessary). could this be a symbolic constant?
https://review.coreboot.org/c/coreboot/+/41855/5/src/soc/intel/common/block/... PS5, Line 274: 0xe6 another symbolic constant?
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 5:
As per this below Jenkin failure : Build Unstable https://qa.coreboot.org/job/coreboot-gerrit/131283/ : UNSTABLE I checked these build failures and looks like these errors for majorly 3 types of systems, haswell, model_2065x and model_206ax. It looks like code is scattered across many places for these 3 platforms and difficult to address it. So, I am thinking to use new modified function name configure_tcc_thermal_target() instead of configure_thermal_target() in common code base and use it for only for apollolake, broadwell, skylake and cannonlake possible SOCs. Kindly, suggest if you have any other inputs on this. Thanks.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 5:
Patch Set 5:
As per this below Jenkin failure : Build Unstable https://qa.coreboot.org/job/coreboot-gerrit/131283/ : UNSTABLE I checked these build failures and looks like these errors for majorly 3 types of systems, haswell, model_2065x and model_206ax. It looks like code is scattered across many places for these 3 platforms and difficult to address it. So, I am thinking to use new modified function name configure_tcc_thermal_target() instead of configure_thermal_target() in common code base and use it for only for apollolake, broadwell, skylake and cannonlake possible SOCs. Kindly, suggest if you have any other inputs on this. Thanks.
SGTM
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41855/5/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/cpulib.c:
https://review.coreboot.org/c/coreboot/+/41855/5/src/soc/intel/common/block/... PS5, Line 265: (1 << 30)
nit: BIT(30) (#include <types.h> if necessary). […]
Ack
https://review.coreboot.org/c/coreboot/+/41855/5/src/soc/intel/common/block/... PS5, Line 274: 0xe6
another symbolic constant?
I didn't find any particular for this one.
Hello Sumeet Pawnikar, build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Tim Wawrzynczak, Puthikorn Voravootivat, Subrata Banik, Andrey Petrov, Raj Astekar, Patrick Rudolph, Venkata Krishna Nimmagadda,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41855
to look at the new patch set (#6).
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
soc/intel/common: add TCC activation functionality
This enables to configure the Thermal Control Circuit (TCC) activation value to new value as tcc_offset in degree Celcius. It prevents any abrupt thermal shutdown while running heavy workload. This helps to take early thermal throttling action before CPU temperature reaches maximum operating temperature TjMax value. Also, cleanup local functions from previous intel soc specific code base like for apollolake, broadwell, skylake and cannonlake.
BUG=None BRANCH=None TEST=Built for volteer platform and verified the MSR value.
Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/apollolake/romstage.c M src/soc/intel/broadwell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/cpulib.c M src/soc/intel/common/block/include/intelblocks/cpulib.h M src/soc/intel/skylake/cpu.c 6 files changed, 28 insertions(+), 76 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/41855/6
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 7:
I am not clear on above jenkin build errors. How can we address these ? Thanks.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 7:
Patch Set 7:
I am not clear on above jenkin build errors. How can we address these ? Thanks.
There is no soc_chip.h for the following SoCs:
SOC_INTEL_DENVERTON_NS SOC_INTEL_COOPERLAKE_SP SOC_INTEL_SKYLAKE_SP
Hello Sumeet Pawnikar, build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Tim Wawrzynczak, Puthikorn Voravootivat, Subrata Banik, Andrey Petrov, Raj Astekar, Patrick Rudolph, Venkata Krishna Nimmagadda,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41855
to look at the new patch set (#8).
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
soc/intel/common: add TCC activation functionality
This enables to configure the Thermal Control Circuit (TCC) activation value to new value as tcc_offset in degree Celcius. It prevents any abrupt thermal shutdown while running heavy workload. This helps to take early thermal throttling action before CPU temperature reaches maximum operating temperature TjMax value. Also, cleanup local functions from previous intel soc specific code base like for apollolake, broadwell, skylake and cannonlake.
BUG=None BRANCH=None TEST=Built for volteer platform and verified the MSR value.
Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com --- M src/soc/intel/apollolake/romstage.c M src/soc/intel/broadwell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/cpulib.c M src/soc/intel/common/block/include/intelblocks/cpulib.h M src/soc/intel/denverton_ns/chip.h A src/soc/intel/denverton_ns/include/soc/soc_chip.h M src/soc/intel/skylake/cpu.c M src/soc/intel/xeon_sp/cpx/chip.h A src/soc/intel/xeon_sp/include/soc/soc_chip.h M src/soc/intel/xeon_sp/skx/chip.h 11 files changed, 53 insertions(+), 76 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/41855/8
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 8: Code-Review+2
Thanks Sumeet for the refactor!
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
soc/intel/common: add TCC activation functionality
This enables to configure the Thermal Control Circuit (TCC) activation value to new value as tcc_offset in degree Celcius. It prevents any abrupt thermal shutdown while running heavy workload. This helps to take early thermal throttling action before CPU temperature reaches maximum operating temperature TjMax value. Also, cleanup local functions from previous intel soc specific code base like for apollolake, broadwell, skylake and cannonlake.
BUG=None BRANCH=None TEST=Built for volteer platform and verified the MSR value.
Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Signed-off-by: Sumeet R Pawnikar sumeet.r.pawnikar@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41855 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/apollolake/romstage.c M src/soc/intel/broadwell/cpu.c M src/soc/intel/cannonlake/cpu.c M src/soc/intel/common/block/cpu/cpulib.c M src/soc/intel/common/block/include/intelblocks/cpulib.h M src/soc/intel/denverton_ns/chip.h A src/soc/intel/denverton_ns/include/soc/soc_chip.h M src/soc/intel/skylake/cpu.c M src/soc/intel/xeon_sp/cpx/chip.h A src/soc/intel/xeon_sp/include/soc/soc_chip.h M src/soc/intel/xeon_sp/skx/chip.h 11 files changed, 53 insertions(+), 76 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/apollolake/romstage.c b/src/soc/intel/apollolake/romstage.c index bdd6e8c..d14dd8d 100644 --- a/src/soc/intel/apollolake/romstage.c +++ b/src/soc/intel/apollolake/romstage.c @@ -73,23 +73,6 @@ P2SB_HPTC_ADDRESS_ENABLE); }
-/* Thermal throttle activation offset */ -static void configure_thermal_target(void) -{ - msr_t msr; - const config_t *conf = config_of_soc(); - - if (!conf->tcc_offset) - return; - - msr = rdmsr(MSR_TEMPERATURE_TARGET); - /* Bits 27:24 */ - msr.lo &= ~(TEMPERATURE_TCC_MASK << TEMPERATURE_TCC_SHIFT); - msr.lo |= (conf->tcc_offset & TEMPERATURE_TCC_MASK) - << TEMPERATURE_TCC_SHIFT; - wrmsr(MSR_TEMPERATURE_TARGET, msr); -} - /* * Punit Initialization code. This all isn't documented, but * this is the recipe. @@ -101,7 +84,7 @@ struct stopwatch sw;
/* Thermal throttle activation offset */ - configure_thermal_target(); + configure_tcc_thermal_target();
/* * Software Core Disable Mask (P_CR_CORE_DISABLE_MASK_0_0_0_MCHBAR). diff --git a/src/soc/intel/broadwell/cpu.c b/src/soc/intel/broadwell/cpu.c index 4bfa15d..1312525 100644 --- a/src/soc/intel/broadwell/cpu.c +++ b/src/soc/intel/broadwell/cpu.c @@ -290,22 +290,6 @@ wrmsr(MSR_C_STATE_LATENCY_CONTROL_5, msr); }
-static void configure_thermal_target(void) -{ - config_t *conf = config_of_soc(); - msr_t msr; - - - /* Set TCC activation offset if supported */ - msr = rdmsr(MSR_PLATFORM_INFO); - if ((msr.lo & (1 << 30)) && conf->tcc_offset) { - msr = rdmsr(MSR_TEMPERATURE_TARGET); - msr.lo &= ~(0xf << 24); /* Bits 27:24 */ - msr.lo |= (conf->tcc_offset & 0xf) << 24; - wrmsr(MSR_TEMPERATURE_TARGET, msr); - } -} - static void configure_misc(void) { msr_t msr; @@ -430,7 +414,7 @@ configure_misc();
/* Thermal throttle activation offset */ - configure_thermal_target(); + configure_tcc_thermal_target();
/* Enable Direct Cache Access */ configure_dca_cap(); diff --git a/src/soc/intel/cannonlake/cpu.c b/src/soc/intel/cannonlake/cpu.c index 89d3493..36d252e 100644 --- a/src/soc/intel/cannonlake/cpu.c +++ b/src/soc/intel/cannonlake/cpu.c @@ -155,25 +155,6 @@ wrmsr(MSR_C_STATE_LATENCY_CONTROL_5, msr); }
-static void configure_thermal_target(void) -{ - config_t *conf = config_of_soc(); - msr_t msr; - - /* Set TCC activation offset if supported */ - msr = rdmsr(MSR_PLATFORM_INFO); - if ((msr.lo & (1 << 30)) && conf->tcc_offset) { - msr = rdmsr(MSR_TEMPERATURE_TARGET); - msr.lo &= ~(0xf << 24); /* Bits 27:24 */ - msr.lo |= (conf->tcc_offset & 0xf) << 24; - wrmsr(MSR_TEMPERATURE_TARGET, msr); - } - msr = rdmsr(MSR_TEMPERATURE_TARGET); - msr.lo &= ~0x7f; /* Bits 6:0 */ - msr.lo |= 0xe6; /* setting 100ms thermal time window */ - wrmsr(MSR_TEMPERATURE_TARGET, msr); -} - /* * The emulated ACPI timer allows replacing of the ACPI timer * (PM1_TMR) to have no impart on the system. @@ -282,7 +263,7 @@ printk(BIOS_ERR, "MP initialization failure.\n");
/* Thermal throttle activation offset */ - configure_thermal_target(); + configure_tcc_thermal_target(); }
int soc_skip_ucode_update(u32 current_patch_id, u32 new_patch_id) diff --git a/src/soc/intel/common/block/cpu/cpulib.c b/src/soc/intel/common/block/cpu/cpulib.c index dac654f..0ac8dda 100644 --- a/src/soc/intel/common/block/cpu/cpulib.c +++ b/src/soc/intel/common/block/cpu/cpulib.c @@ -8,6 +8,7 @@ #include <intelblocks/cpulib.h> #include <intelblocks/fast_spi.h> #include <intelblocks/msr.h> +#include <soc/soc_chip.h> #include <stdint.h>
/* @@ -254,6 +255,26 @@ return ratio_max; }
+void configure_tcc_thermal_target(void) +{ + const config_t *conf = config_of_soc(); + msr_t msr; + + /* Set TCC activation offset */ + msr = rdmsr(MSR_PLATFORM_INFO); + if ((msr.lo & BIT(30)) && conf->tcc_offset) { + msr = rdmsr(MSR_TEMPERATURE_TARGET); + msr.lo &= ~(0xf << 24); + msr.lo |= (conf->tcc_offset & 0xf) << 24; + wrmsr(MSR_TEMPERATURE_TARGET, msr); + } + msr = rdmsr(MSR_TEMPERATURE_TARGET); + /* Time Window Tau Bits [6:0] */ + msr.lo &= ~0x7f; + msr.lo |= 0xe6; /* setting 100ms thermal time window */ + wrmsr(MSR_TEMPERATURE_TARGET, msr); +} + uint32_t cpu_get_bus_clock(void) { /* CPU bus clock is set by default here to 100MHz. diff --git a/src/soc/intel/common/block/include/intelblocks/cpulib.h b/src/soc/intel/common/block/include/intelblocks/cpulib.h index 73f4e38..09f5e45 100644 --- a/src/soc/intel/common/block/include/intelblocks/cpulib.h +++ b/src/soc/intel/common/block/include/intelblocks/cpulib.h @@ -133,6 +133,9 @@ */ uint32_t cpu_get_max_ratio(void);
+/* Thermal throttle activation offset */ +void configure_tcc_thermal_target(void); + /* * cpu_get_power_max calculates CPU TDP in mW */ diff --git a/src/soc/intel/denverton_ns/chip.h b/src/soc/intel/denverton_ns/chip.h index 8401eb1..2f16bf5 100644 --- a/src/soc/intel/denverton_ns/chip.h +++ b/src/soc/intel/denverton_ns/chip.h @@ -56,6 +56,9 @@ uint32_t ipc1; uint32_t ipc2; uint32_t ipc3; + + /* TCC activation offset */ + uint32_t tcc_offset; };
typedef struct soc_intel_denverton_ns_config config_t; diff --git a/src/soc/intel/denverton_ns/include/soc/soc_chip.h b/src/soc/intel/denverton_ns/include/soc/soc_chip.h new file mode 100644 index 0000000..800e78e --- /dev/null +++ b/src/soc/intel/denverton_ns/include/soc/soc_chip.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _SOC_DENVERTON_NS_SOC_CHIP_H_ +#define _SOC_DENVERTON_NS_SOC_CHIP_H_ + +#include "../../chip.h" + +#endif /* _SOC_DENVERTON_NS_SOC_CHIP_H_ */ diff --git a/src/soc/intel/skylake/cpu.c b/src/soc/intel/skylake/cpu.c index a545435..d941df7 100644 --- a/src/soc/intel/skylake/cpu.c +++ b/src/soc/intel/skylake/cpu.c @@ -27,26 +27,6 @@
#include "chip.h"
-static void configure_thermal_target(void) -{ - config_t *conf = config_of_soc(); - msr_t msr; - - - /* Set TCC activation offset if supported */ - msr = rdmsr(MSR_PLATFORM_INFO); - if ((msr.lo & (1 << 30)) && conf->tcc_offset) { - msr = rdmsr(MSR_TEMPERATURE_TARGET); - msr.lo &= ~(0xf << 24); /* Bits 27:24 */ - msr.lo |= (conf->tcc_offset & 0xf) << 24; - wrmsr(MSR_TEMPERATURE_TARGET, msr); - } - msr = rdmsr(MSR_TEMPERATURE_TARGET); - msr.lo &= ~0x7f; /* Bits 6:0 */ - msr.lo |= 0xe6; /* setting 100ms thermal time window */ - wrmsr(MSR_TEMPERATURE_TARGET, msr); -} - static void configure_isst(void) { config_t *conf = config_of_soc(); @@ -333,7 +313,7 @@ printk(BIOS_ERR, "MP initialization failure.\n");
/* Thermal throttle activation offset */ - configure_thermal_target(); + configure_tcc_thermal_target(); }
int soc_skip_ucode_update(u32 current_patch_id, u32 new_patch_id) diff --git a/src/soc/intel/xeon_sp/cpx/chip.h b/src/soc/intel/xeon_sp/cpx/chip.h index 6bf7272..61e806e 100644 --- a/src/soc/intel/xeon_sp/cpx/chip.h +++ b/src/soc/intel/xeon_sp/cpx/chip.h @@ -67,6 +67,9 @@ uint32_t gen2_dec; uint32_t gen3_dec; uint32_t gen4_dec; + + /* TCC activation offset */ + uint32_t tcc_offset; };
typedef struct soc_intel_xeon_sp_cpx_config config_t; diff --git a/src/soc/intel/xeon_sp/include/soc/soc_chip.h b/src/soc/intel/xeon_sp/include/soc/soc_chip.h new file mode 100644 index 0000000..3113ead --- /dev/null +++ b/src/soc/intel/xeon_sp/include/soc/soc_chip.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _SOC_XEON_SP_SOC_CHIP_H_ +#define _SOC_XEON_SP_SOC_CHIP_H_ + +#include "../chip.h" + +#endif /* _SOC_XEON_SP_SOC_CHIP_H_ */ diff --git a/src/soc/intel/xeon_sp/skx/chip.h b/src/soc/intel/xeon_sp/skx/chip.h index c7f19ec..440fb40 100644 --- a/src/soc/intel/xeon_sp/skx/chip.h +++ b/src/soc/intel/xeon_sp/skx/chip.h @@ -69,6 +69,9 @@ uint32_t gen2_dec; uint32_t gen3_dec; uint32_t gen4_dec; + + /* TCC activation offset */ + uint32_t tcc_offset; };
typedef struct soc_intel_xeon_sp_skx_config config_t;
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/cpulib.c:
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... PS9, Line 271: msr = rdmsr(MSR_TEMPERATURE_TARGET); : /* Time Window Tau Bits [6:0] */ : msr.lo &= ~0x7f; : msr.lo |= 0xe6; /* setting 100ms thermal time window */ : wrmsr(MSR_TEMPERATURE_TARGET, msr); This patch breaks all Siemens APL mainboards. This part was not executed in the old function. The Default value after reboot for MSR_TEMPERATURE_TARGET is 0x006e0000. Everything works fine with this setting. But now also Bits 15:8 will be changed (Fan Temperature Target Offset). Our boards do not have a fan. Is it possible for APL to query whether a fan is present and only then execute the code?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/cpulib.c:
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... PS9, Line 271: msr = rdmsr(MSR_TEMPERATURE_TARGET); : /* Time Window Tau Bits [6:0] */ : msr.lo &= ~0x7f; : msr.lo |= 0xe6; /* setting 100ms thermal time window */ : wrmsr(MSR_TEMPERATURE_TARGET, msr);
This patch breaks all Siemens APL mainboards. This part was not executed in the old function. […]
Sorry about that Mario, we're going through an effort to try and move as much Intel SoC-specific code to src/soc/intel/common as possible. I am not aware of any general mechanism to detect fan presence in coreboot, but this can definitely be solved with a quick Kconfig change. I'll try to post that change ASAP. I'll see you and Werner as reviewers.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/cpulib.c:
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... PS9, Line 271: msr = rdmsr(MSR_TEMPERATURE_TARGET); : /* Time Window Tau Bits [6:0] */ : msr.lo &= ~0x7f; : msr.lo |= 0xe6; /* setting 100ms thermal time window */ : wrmsr(MSR_TEMPERATURE_TARGET, msr);
Sorry about that Mario, we're going through an effort to try and move as much Intel SoC-specific cod […]
Mario, see CB:42879
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/cpulib.c:
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... PS9, Line 271: msr = rdmsr(MSR_TEMPERATURE_TARGET); : /* Time Window Tau Bits [6:0] */ : msr.lo &= ~0x7f; : msr.lo |= 0xe6; /* setting 100ms thermal time window */ : wrmsr(MSR_TEMPERATURE_TARGET, msr);
Mario, see CB:42879
I'll look at it tomorrow. Thanks for the quick patch before.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/cpulib.c:
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... PS9, Line 271: msr = rdmsr(MSR_TEMPERATURE_TARGET); : /* Time Window Tau Bits [6:0] */ : msr.lo &= ~0x7f; : msr.lo |= 0xe6; /* setting 100ms thermal time window */ : wrmsr(MSR_TEMPERATURE_TARGET, msr);
I'll look at it tomorrow. Thanks for the quick patch before.
Sorry for breaking Siemens APL mainboards due to this change. I am not sure how to prevent breaking any specific boards/platforms, even here particularly Siemens APL boards for this reason. If you have any info please do share. One thing we can follow that I will try to add you and Werner in patch to notify you in advance going forward for any APL Intel SoC-specific code changes to address this kind of situation.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41855/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41855/9//COMMIT_MSG@19 PS9, Line 19: TEST=Built for volteer platform and verified the MSR value. not sure how come validating only on TGL platform helps to qualify a common code CL. This is not done in past. Refer to CL https://review.coreboot.org/c/coreboot/+/26132, specifically TEST section the amount of test is required to ensure no brokenness.
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/cpulib.c:
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... PS9, Line 271: msr = rdmsr(MSR_TEMPERATURE_TARGET); : /* Time Window Tau Bits [6:0] */ : msr.lo &= ~0x7f; : msr.lo |= 0xe6; /* setting 100ms thermal time window */ : wrmsr(MSR_TEMPERATURE_TARGET, msr);
Sorry for breaking Siemens APL mainboards due to this change. […]
I think it's a good idea to add us to the patches when making changes on APL specific code.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41855 )
Change subject: soc/intel/common: add TCC activation functionality ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/cpulib.c:
https://review.coreboot.org/c/coreboot/+/41855/9/src/soc/intel/common/block/... PS9, Line 271: msr = rdmsr(MSR_TEMPERATURE_TARGET); : /* Time Window Tau Bits [6:0] */ : msr.lo &= ~0x7f; : msr.lo |= 0xe6; /* setting 100ms thermal time window */ : wrmsr(MSR_TEMPERATURE_TARGET, msr);
I think it's a good idea to add us to the patches when making changes on APL specific code.
The issue here was not the FAN window but the fact that on Apollo Lake the Bits 0..7 in MSR 0x1A2 (MSR_TEMPERATURE_TARGET) are marked as reserved. They do exist on Tigerlake though. So once a write access to this bits has happened, the CPU starts hanging...which is a valid behaviour when reserved bits are written.
@Sumeet: Yes, it would be nice if you can add us to the patches which are touching APL code.
-
Mario Scheithauer (Code Review) -
Patrick Georgi (Code Review)
-
Subrata Banik (Code Review)
-
Sumeet R Pawnikar (Code Review)
-
Tim Wawrzynczak (Code Review)
-
Werner Zeh (Code Review)