Change in coreboot[master]: [WIP] tigerlake: add TCC activation functionality
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) -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 1 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-MessageType: newchange
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. -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 1 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Fri, 29 May 2020 09:19:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
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. -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 1 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Fri, 12 Jun 2020 05:05:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
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 -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 2 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
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? -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 3 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 16 Jun 2020 17:02:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
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.
-- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 3 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 18 Jun 2020 13:12:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-MessageType: comment
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 -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 4 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
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 -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 5 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
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? -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 5 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 18 Jun 2020 18:06:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
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. -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 5 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 18 Jun 2020 18:22:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
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 -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 5 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 18 Jun 2020 18:42:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
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.
-- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 5 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 19 Jun 2020 09:28:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-MessageType: comment
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 -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 6 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
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. -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 7 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 23 Jun 2020 17:10:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
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 -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 7 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 23 Jun 2020 18:55:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
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 -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 8 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
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! -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 8 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 25 Jun 2020 20:05:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
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; -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 9 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: merged
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? -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 9 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 29 Jun 2020 10:42:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
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.
-- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 9 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh@siemens.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 29 Jun 2020 18:52:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-MessageType: comment
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
-- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 9 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh@siemens.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 29 Jun 2020 19:10:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak@chromium.org> Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-MessageType: comment
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.
-- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 9 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh@siemens.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 29 Jun 2020 19:16:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak@chromium.org> Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-MessageType: comment
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.
-- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 9 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh@siemens.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 30 Jun 2020 09:31:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak@chromium.org> Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-MessageType: comment
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. -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 9 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh@siemens.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 30 Jun 2020 09:42:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
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.
-- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 9 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh@siemens.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 30 Jun 2020 12:34:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak@chromium.org> Comment-In-Reply-To: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-MessageType: comment
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. -- To view, visit https://review.coreboot.org/c/coreboot/+/41855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37dd878902b080602d70c5c3c906820613ea14a5 Gerrit-Change-Number: 41855 Gerrit-PatchSet: 9 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan@google.com> Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer@siemens.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Puthikorn Voravootivat <puthik@chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar@intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar@intel.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda <Venkata.krishna.nimmagadda@intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh@siemens.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 01 Jul 2020 11:56:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak@chromium.org> Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer@siemens.com> Comment-In-Reply-To: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-MessageType: comment
participants (6)
-
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)