Hello Aamir Bohra,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/31609
to review the following change.
Change subject: soc/intel/icelake: Enable PM timer emulation support ......................................................................
soc/intel/icelake: Enable PM timer emulation support
TEST=Able to build and boot with alt/tianocore payload.
Change-Id: I7fd11e728b7a14f41f08bc39bcd92a42a8aa6cff Signed-off-by: Aamir Bohra aamir.bohra@intel.com Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/icelake/cpu.c M src/soc/intel/icelake/include/soc/cpu.h 2 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/31609/1
diff --git a/src/soc/intel/icelake/cpu.c b/src/soc/intel/icelake/cpu.c index 0585450..f4ebacf 100644 --- a/src/soc/intel/icelake/cpu.c +++ b/src/soc/intel/icelake/cpu.c @@ -24,6 +24,7 @@ #include <fsp/api.h> #include <intelblocks/cpulib.h> #include <intelblocks/mp_init.h> +#include <intelblocks/msr.h> #include <intelblocks/smm.h> #include <romstage_handoff.h> #include <soc/cpu.h> @@ -118,6 +119,23 @@ } }
+static void enable_pm_timer_emulation(void) +{ + /* ACPI PM timer emulation */ + msr_t msr; + /* + * The derived frequency is calculated as follows: + * (CTC_FREQ * msr[63:32]) >> 32 = target frequency. + * Back solve the multiplier so the 3.579545MHz ACPI timer + * frequency is used. + */ + msr.hi = (3579545ULL << 32) / CTC_FREQ; + /* Set PM1 timer IO port and enable*/ + msr.lo = (EMULATE_DELAY_VALUE << EMULATE_DELAY_OFFSET_VALUE) | + EMULATE_PM_TMR_EN | (ACPI_BASE_ADDRESS + PM1_TMR); + wrmsr(MSR_EMULATE_PM_TIMER, msr); +} + static void set_energy_perf_bias(u8 policy) { msr_t msr; @@ -190,6 +208,9 @@ /* Configure Intel Speed Shift */ configure_isst();
+ /* Enable PM timer emulation */ + enable_pm_timer_emulation(); + /* Enable Direct Cache Access */ configure_dca_cap();
diff --git a/src/soc/intel/icelake/include/soc/cpu.h b/src/soc/intel/icelake/include/soc/cpu.h index 1e8d9e8..fae8500 100644 --- a/src/soc/intel/icelake/include/soc/cpu.h +++ b/src/soc/intel/icelake/include/soc/cpu.h @@ -35,6 +35,9 @@ #define C9_POWER 0xc8 #define C10_POWER 0xc8
+/* Common Timer Copy (CTC) frequency - 38.4MHz. */ +#define CTC_FREQ 38400000 + #define C_STATE_LATENCY_MICRO_SECONDS(limit, base) \ (((1 << ((base)*5)) * (limit)) / 1000) #define C_STATE_LATENCY_FROM_LAT_REG(reg) \
Hello Patrick Rudolph, Aamir Bohra, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31609
to look at the new patch set (#2).
Change subject: soc/intel/icelake: Enable PM timer emulation support ......................................................................
soc/intel/icelake: Enable PM timer emulation support
TEST=Able to build and boot with alt/tianocore payload.
Change-Id: I7fd11e728b7a14f41f08bc39bcd92a42a8aa6cff Signed-off-by: Aamir Bohra aamir.bohra@intel.com Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/icelake/cpu.c M src/soc/intel/icelake/include/soc/cpu.h 2 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/31609/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31609 )
Change subject: soc/intel/icelake: Enable PM timer emulation support ......................................................................
Patch Set 2:
(2 comments)
This seems to be present since Skylake. Why wasn’t it included in the original commit adding support for Ice Lake?
https://review.coreboot.org/#/c/31609/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31609/2//COMMIT_MSG@8 PS2, Line 8: Could you please give more details? Why is the PM timer needed?
How much time does this add to the boot process?
https://review.coreboot.org/#/c/31609/2//COMMIT_MSG@9 PS2, Line 9: alt/tianocore What does *alt* stand for?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31609 )
Change subject: soc/intel/icelake: Enable PM timer emulation support ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/#/c/31609/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31609/2//COMMIT_MSG@8 PS2, Line 8:
Could you please give more details? Why is the PM timer needed? […]
Done
https://review.coreboot.org/#/c/31609/2//COMMIT_MSG@9 PS2, Line 9: alt/tianocore
What does *alt* stand for?
Done
Hello Aaron Durbin, Patrick Rudolph, Aamir Bohra, Duncan Laurie, Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31609
to look at the new patch set (#3).
Change subject: soc/intel/icelake: Add PM timer emulation support in ICL ......................................................................
soc/intel/icelake: Add PM timer emulation support in ICL
CPU PM TIMER EMULATION logic will help UEFI payload to execute rather wait for time tick in absence of TCO and ACPI PM timer after FSP-S.
BUG=N/A TEST=Able to build and boot with tianocore payload.
Change-Id: I7fd11e728b7a14f41f08bc39bcd92a42a8aa6cff Signed-off-by: Aamir Bohra aamir.bohra@intel.com Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/icelake/cpu.c M src/soc/intel/icelake/include/soc/cpu.h 2 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/31609/3
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31609 )
Change subject: soc/intel/icelake: Add PM timer emulation support in ICL ......................................................................
Patch Set 3:
@Reviewers: can you please help to review this
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31609 )
Change subject: soc/intel/icelake: Add PM timer emulation support in ICL ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31609 )
Change subject: soc/intel/icelake: Add PM timer emulation support in ICL ......................................................................
soc/intel/icelake: Add PM timer emulation support in ICL
CPU PM TIMER EMULATION logic will help UEFI payload to execute rather wait for time tick in absence of TCO and ACPI PM timer after FSP-S.
BUG=N/A TEST=Able to build and boot with tianocore payload.
Change-Id: I7fd11e728b7a14f41f08bc39bcd92a42a8aa6cff Signed-off-by: Aamir Bohra aamir.bohra@intel.com Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/31609 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/icelake/cpu.c M src/soc/intel/icelake/include/soc/cpu.h 2 files changed, 24 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/icelake/cpu.c b/src/soc/intel/icelake/cpu.c index 0585450..f4ebacf 100644 --- a/src/soc/intel/icelake/cpu.c +++ b/src/soc/intel/icelake/cpu.c @@ -24,6 +24,7 @@ #include <fsp/api.h> #include <intelblocks/cpulib.h> #include <intelblocks/mp_init.h> +#include <intelblocks/msr.h> #include <intelblocks/smm.h> #include <romstage_handoff.h> #include <soc/cpu.h> @@ -118,6 +119,23 @@ } }
+static void enable_pm_timer_emulation(void) +{ + /* ACPI PM timer emulation */ + msr_t msr; + /* + * The derived frequency is calculated as follows: + * (CTC_FREQ * msr[63:32]) >> 32 = target frequency. + * Back solve the multiplier so the 3.579545MHz ACPI timer + * frequency is used. + */ + msr.hi = (3579545ULL << 32) / CTC_FREQ; + /* Set PM1 timer IO port and enable*/ + msr.lo = (EMULATE_DELAY_VALUE << EMULATE_DELAY_OFFSET_VALUE) | + EMULATE_PM_TMR_EN | (ACPI_BASE_ADDRESS + PM1_TMR); + wrmsr(MSR_EMULATE_PM_TIMER, msr); +} + static void set_energy_perf_bias(u8 policy) { msr_t msr; @@ -190,6 +208,9 @@ /* Configure Intel Speed Shift */ configure_isst();
+ /* Enable PM timer emulation */ + enable_pm_timer_emulation(); + /* Enable Direct Cache Access */ configure_dca_cap();
diff --git a/src/soc/intel/icelake/include/soc/cpu.h b/src/soc/intel/icelake/include/soc/cpu.h index 1e8d9e8..b5722da 100644 --- a/src/soc/intel/icelake/include/soc/cpu.h +++ b/src/soc/intel/icelake/include/soc/cpu.h @@ -35,6 +35,9 @@ #define C9_POWER 0xc8 #define C10_POWER 0xc8
+/* Common Timer Copy (CTC) frequency - 38.4MHz. */ +#define CTC_FREQ 38400000 + #define C_STATE_LATENCY_MICRO_SECONDS(limit, base) \ (((1 << ((base)*5)) * (limit)) / 1000) #define C_STATE_LATENCY_FROM_LAT_REG(reg) \
-
Furquan Shaikh (Code Review) -
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Subrata Banik (Code Review)