Matt Delco has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39411 )
Change subject: soc/intel: fix eist enabling ......................................................................
soc/intel: fix eist enabling
There was a bug like this for skylake that seems to have been copied to other SoCs.
Signed-off-by: Matt Delco delco@chromium.org Change-Id: Ib4651eda46a064dfb59797ac8e1cb8c38bb8e38c --- M src/soc/intel/cannonlake/cpu.c M src/soc/intel/icelake/cpu.c M src/soc/intel/skylake/cpu.c M src/soc/intel/tigerlake/cpu.c 4 files changed, 9 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/39411/1
diff --git a/src/soc/intel/cannonlake/cpu.c b/src/soc/intel/cannonlake/cpu.c index f01b499..3ba0562 100644 --- a/src/soc/intel/cannonlake/cpu.c +++ b/src/soc/intel/cannonlake/cpu.c @@ -263,9 +263,10 @@ msr = rdmsr(IA32_MISC_ENABLE); msr.lo |= (1 << 0); /* Fast String enable */ msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ + wrmsr(IA32_MISC_ENABLE, msr); + /* Set EIST status */ cpu_set_eist(conf->eist_enable); - wrmsr(IA32_MISC_ENABLE, msr);
/* Disable Thermal interrupts */ msr.lo = 0; diff --git a/src/soc/intel/icelake/cpu.c b/src/soc/intel/icelake/cpu.c index e058442..91282d8 100644 --- a/src/soc/intel/icelake/cpu.c +++ b/src/soc/intel/icelake/cpu.c @@ -71,9 +71,10 @@ msr = rdmsr(IA32_MISC_ENABLE); msr.lo |= (1 << 0); /* Fast String enable */ msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ + wrmsr(IA32_MISC_ENABLE, msr); + /* Set EIST status */ cpu_set_eist(conf->eist_enable); - wrmsr(IA32_MISC_ENABLE, msr);
/* Disable Thermal interrupts */ msr.lo = 0; diff --git a/src/soc/intel/skylake/cpu.c b/src/soc/intel/skylake/cpu.c index f5273f6..d7da56e 100644 --- a/src/soc/intel/skylake/cpu.c +++ b/src/soc/intel/skylake/cpu.c @@ -291,14 +291,11 @@ msr = rdmsr(IA32_MISC_ENABLE); msr.lo |= (1 << 0); /* Fast String enable */ msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ - - if (conf->eist_enable) - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ - else - msr.lo &= ~(1 << 16); /* Enhanced SpeedStep Disable */ - wrmsr(IA32_MISC_ENABLE, msr);
+ /* Set EIST status */ + cpu_set_eist(conf->eist_enable); + /* Disable Thermal interrupts */ msr.lo = 0; msr.hi = 0; diff --git a/src/soc/intel/tigerlake/cpu.c b/src/soc/intel/tigerlake/cpu.c index 5f4f081..cfbfdb3 100644 --- a/src/soc/intel/tigerlake/cpu.c +++ b/src/soc/intel/tigerlake/cpu.c @@ -77,9 +77,10 @@ msr = rdmsr(IA32_MISC_ENABLE); msr.lo |= (1 << 0); /* Fast String enable */ msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ + wrmsr(IA32_MISC_ENABLE, msr); + /* Set EIST status */ cpu_set_eist(conf->eist_enable); - wrmsr(IA32_MISC_ENABLE, msr);
/* Disable Thermal interrupts */ msr.lo = 0;
Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39411 )
Change subject: soc/intel: fix eist enabling ......................................................................
Patch Set 1:
For reference, the original fix that was used on skylake:
https://review.coreboot.org/c/coreboot/+/25330/
Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39411 )
Change subject: soc/intel: fix eist enabling ......................................................................
Patch Set 1:
This change basically takes the direction that Aaron suggested in the skylake fix. I sympathize with the hesitation of that fix's author to add another read and write of an MSR, but 1) I don't see a way to avoid it without making larger changes to the common abstractions, and 2) I doubt anyone will notice the additional overhead.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39411 )
Change subject: soc/intel: fix eist enabling ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39411 )
Change subject: soc/intel: fix eist enabling ......................................................................
soc/intel: fix eist enabling
There was a bug like this for skylake that seems to have been copied to other SoCs.
Signed-off-by: Matt Delco delco@chromium.org Change-Id: Ib4651eda46a064dfb59797ac8e1cb8c38bb8e38c Reviewed-on: https://review.coreboot.org/c/coreboot/+/39411 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org --- M src/soc/intel/cannonlake/cpu.c M src/soc/intel/icelake/cpu.c M src/soc/intel/skylake/cpu.c M src/soc/intel/tigerlake/cpu.c 4 files changed, 9 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/cpu.c b/src/soc/intel/cannonlake/cpu.c index f01b499..3ba0562 100644 --- a/src/soc/intel/cannonlake/cpu.c +++ b/src/soc/intel/cannonlake/cpu.c @@ -263,9 +263,10 @@ msr = rdmsr(IA32_MISC_ENABLE); msr.lo |= (1 << 0); /* Fast String enable */ msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ + wrmsr(IA32_MISC_ENABLE, msr); + /* Set EIST status */ cpu_set_eist(conf->eist_enable); - wrmsr(IA32_MISC_ENABLE, msr);
/* Disable Thermal interrupts */ msr.lo = 0; diff --git a/src/soc/intel/icelake/cpu.c b/src/soc/intel/icelake/cpu.c index e058442..91282d8 100644 --- a/src/soc/intel/icelake/cpu.c +++ b/src/soc/intel/icelake/cpu.c @@ -71,9 +71,10 @@ msr = rdmsr(IA32_MISC_ENABLE); msr.lo |= (1 << 0); /* Fast String enable */ msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ + wrmsr(IA32_MISC_ENABLE, msr); + /* Set EIST status */ cpu_set_eist(conf->eist_enable); - wrmsr(IA32_MISC_ENABLE, msr);
/* Disable Thermal interrupts */ msr.lo = 0; diff --git a/src/soc/intel/skylake/cpu.c b/src/soc/intel/skylake/cpu.c index f5273f6..d7da56e 100644 --- a/src/soc/intel/skylake/cpu.c +++ b/src/soc/intel/skylake/cpu.c @@ -291,14 +291,11 @@ msr = rdmsr(IA32_MISC_ENABLE); msr.lo |= (1 << 0); /* Fast String enable */ msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ - - if (conf->eist_enable) - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ - else - msr.lo &= ~(1 << 16); /* Enhanced SpeedStep Disable */ - wrmsr(IA32_MISC_ENABLE, msr);
+ /* Set EIST status */ + cpu_set_eist(conf->eist_enable); + /* Disable Thermal interrupts */ msr.lo = 0; msr.hi = 0; diff --git a/src/soc/intel/tigerlake/cpu.c b/src/soc/intel/tigerlake/cpu.c index 5f4f081..cfbfdb3 100644 --- a/src/soc/intel/tigerlake/cpu.c +++ b/src/soc/intel/tigerlake/cpu.c @@ -77,9 +77,10 @@ msr = rdmsr(IA32_MISC_ENABLE); msr.lo |= (1 << 0); /* Fast String enable */ msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ + wrmsr(IA32_MISC_ENABLE, msr); + /* Set EIST status */ cpu_set_eist(conf->eist_enable); - wrmsr(IA32_MISC_ENABLE, msr);
/* Disable Thermal interrupts */ msr.lo = 0;
-
Aaron Durbin (Code Review) -
Matt Delco (Code Review)
-
Patrick Georgi (Code Review)