Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33766
Change subject: soc/amd/picasso: Update monotonic timer ......................................................................
soc/amd/picasso: Update monotonic timer
The performance TSC isn't supported in Family 17h. Remove it from the calculation.
Change-Id: I799e8bbf522d02eeaedfd0464165ddc466ffe5dc Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/monotonic_timer.c 1 file changed, 38 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/33766/1
diff --git a/src/soc/amd/picasso/monotonic_timer.c b/src/soc/amd/picasso/monotonic_timer.c index 7ea571f..553804f 100644 --- a/src/soc/amd/picasso/monotonic_timer.c +++ b/src/soc/amd/picasso/monotonic_timer.c @@ -16,23 +16,50 @@ #include <cpu/x86/msr.h> #include <timer.h> #include <timestamp.h> +#include <cpu/x86/tsc.h>
-#define CU_PTSC_MSR 0xc0010280 -#define PTSC_FREQ_MHZ 100 +static struct monotonic_counter { + int initialized; + uint32_t core_frequency; + struct mono_time time; + uint64_t last_value; +} mono_counter; + +static inline uint64_t read_counter_msr(void) +{ + msr_t counter_msr; + + counter_msr = rdmsr(TSC_MSR); + + return ((uint64_t)counter_msr.hi << 32) | (uint64_t)counter_msr.lo; +} + +static void init_timer(void) +{ + mono_counter.core_frequency = tsc_freq_mhz(); + mono_counter.last_value = read_counter_msr(); + mono_counter.initialized = 1; +}
void timer_monotonic_get(struct mono_time *mt) { - mono_time_set_usecs(mt, timestamp_get()); -} + uint64_t current_tick; + uint32_t usecs_elapsed = 0;
-uint64_t timestamp_get(void) -{ - unsigned long long val; - msr_t msr; + if (!mono_counter.initialized) + init_timer();
- msr = rdmsr(CU_PTSC_MSR); + current_tick = read_counter_msr(); + if (mono_counter.core_frequency != 0) + usecs_elapsed = (current_tick - mono_counter.last_value) + / mono_counter.core_frequency;
- val = ((unsigned long long)msr.hi << 32) | msr.lo; + /* Update current time and tick values only if a full tick occurred. */ + if (usecs_elapsed) { + mono_time_add_usecs(&mono_counter.time, usecs_elapsed); + mono_counter.last_value = current_tick; + }
- return val / PTSC_FREQ_MHZ; + /* Save result. */ + *mt = mono_counter.time; }
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33766 )
Change subject: soc/amd/picasso: Update monotonic timer ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/33766/4/src/soc/amd/picasso/monoton... File src/soc/amd/picasso/monotonic_timer.c:
https://review.coreboot.org/c/coreboot/+/33766/4/src/soc/amd/picasso/monoton... PS4, Line 25: Did this come from stoneyridge? Implementation of monotonic timer should not depend on timestamp function declarations in any way, so maybe do a bit of renaming there.
https://review.coreboot.org/c/coreboot/+/33766/4/src/soc/amd/picasso/monoton... PS4, Line 18: #include <timestamp.h> Not needed anymore, although I don't see replacement timestamp_get() anywhere yet.
https://review.coreboot.org/c/coreboot/+/33766/4/src/soc/amd/picasso/monoton... PS4, Line 26: } mono_counter; I kind of feel we could have one common definition that is arch-agnostic for this struct, but I did not dig deeper into this.
https://review.coreboot.org/c/coreboot/+/33766/4/src/soc/amd/picasso/monoton... PS4, Line 32: counter_msr = rdmsr(TSC_MSR); Is this different from rdtscll()? From what I recall, on Intel that is a shortcut instruction to access TSC MSR requiring less privileges.
Do you need MFENCE/LFENCE before accessing the counter MSR?
https://review.coreboot.org/c/coreboot/+/33766/4/src/soc/amd/picasso/monoton... PS4, Line 44: void timer_monotonic_get(struct mono_time *mt) How is this different from the generic TSC_MONOTONIC_TIMER ?
Marshall Dawson has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/33766 )
Change subject: soc/amd/picasso: Update monotonic timer ......................................................................
Abandoned
No need for an soc-specific monotonic timer, and can use TSC.
-
Kyösti Mälkki (Code Review) -
Marshall Dawson (Code Review)