Change in coreboot[master]: soc/amd/picasso: add monotonic_timer

[Kangheui Won (Code Review)]

Kangheui Won has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46058 )

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

soc/amd/picasso: add monotonic_timer

To accommodate better with PSP timestamp, add a layer around tsc to have
microsecond granularity for timestamp table.

BUG=b:159220781
BRANCH=zork
TEST=build, flash and boot, check timestamps are correct

Signed-off-by: Kangheui Won 
Change-Id: Ifced4a84071be8da547e252167ec21cd42f20ccc
---
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/Makefile.inc
A src/soc/amd/picasso/monotonic_timer.c
3 files changed, 22 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/46058/1

diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig
index 6a5b932..360d59d 100644
--- a/src/soc/amd/picasso/Kconfig
+++ b/src/soc/amd/picasso/Kconfig
@@ -24,7 +24,7 @@
    select IOAPIC
    select HAVE_EM100_SUPPORT
    select HAVE_USBDEBUG_OPTIONS
-	select TSC_MONOTONIC_TIMER
+	select COLLECT_TIMESTAMPS_NO_TSC
    select SOC_AMD_COMMON_BLOCK_SPI
    select TSC_SYNC_LFENCE
    select UDELAY_TSC
diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc
index 1e9ba4a..76fd100 100644
--- a/src/soc/amd/picasso/Makefile.inc
+++ b/src/soc/amd/picasso/Makefile.inc
@@ -18,6 +18,7 @@
 bootblock-y += i2c.c
 bootblock-y += uart.c
 bootblock-$(CONFIG_PICASSO_CONSOLE_UART) += uart_console.c
+bootblock-y += monotonic_timer.c
 bootblock-y += tsc_freq.c
 bootblock-y += gpio.c
 bootblock-y += smi_util.c
@@ -32,6 +33,7 @@
 romstage-y += memmap.c
 romstage-y += uart.c
 romstage-$(CONFIG_PICASSO_CONSOLE_UART) += uart_console.c
+romstage-y += monotonic_timer.c
 romstage-y += tsc_freq.c
 romstage-y += aoac.c
 romstage-y += southbridge.c
@@ -47,6 +49,7 @@
 verstage_x86-y += gpio.c
 verstage_x86-y += uart.c
 verstage_x86-$(CONFIG_PICASSO_CONSOLE_UART) += uart_console.c
+verstage_x86-y += monotonic_timer.c
 verstage_x86-y += tsc_freq.c
 verstage_x86-y += reset.c

@@ -70,6 +73,7 @@
 ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smi_util.c
 ramstage-y += uart.c
 ramstage-$(CONFIG_PICASSO_CONSOLE_UART) += uart_console.c
+ramstage-y += monotonic_timer.c
 ramstage-y += tsc_freq.c
 ramstage-y += finalize.c
 ramstage-y += soc_util.c
@@ -84,6 +88,7 @@

smm-y += smihandler.c
 smm-y += smi_util.c
+smm-y += monotonic_timer.c
 smm-y += tsc_freq.c
 ifeq ($(CONFIG_DEBUG_SMI),y)
 smm-y += uart.c
diff --git a/src/soc/amd/picasso/monotonic_timer.c b/src/soc/amd/picasso/monotonic_timer.c
new file mode 100644
index 0000000..941532c
--- /dev/null
+++ b/src/soc/amd/picasso/monotonic_timer.c
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <cpu/x86/msr.h>
+#include <cpu/x86/tsc.h>
+#include <timer.h>
+#include <timestamp.h>
+
+void timer_monotonic_get(struct mono_time *mt)
+{
+	mono_time_set_usecs(mt, timestamp_get());
+}
+
+uint64_t timestamp_get(void)
+{
+	return rdtscll() / tsc_freq_mhz();
+}

[Eric Peers (Code Review)]

Eric Peers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46058 )

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/46058/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/46058/1//COMMIT_MSG@9 
PS1, Line 9: To accommodate better with PSP timestamp, add a layer around tsc to have
This may need a bit more description for people that cannot access the bug.

[Rob Barnes (Code Review)]

Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46058 )

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/46058/1/src/soc/amd/picasso/monoton... 
File src/soc/amd/picasso/monotonic_timer.c:

https://review.coreboot.org/c/coreboot/+/46058/1/src/soc/amd/picasso/monoton... 
PS1, Line 8: void timer_monotonic_get(struct mono_time *mt)
Some comments around these functions would be helpful.

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46058 )

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46058/1/src/soc/amd/picasso/monoton... 
File src/soc/amd/picasso/monotonic_timer.c:

https://review.coreboot.org/c/coreboot/+/46058/1/src/soc/amd/picasso/monoton... 
PS1, Line 10: 	mono_time_set_usecs(mt, timestamp_get());
This is assuming the tsc is aligned in time w/ psp (i.e. all values are *after* what psp reported). Is that true? Or did we need to apply an offset adjustment?

[Kangheui Won (Code Review)]

Kangheui Won has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46058 )

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46058/1/src/soc/amd/picasso/monoton... 
File src/soc/amd/picasso/monotonic_timer.c:

https://review.coreboot.org/c/coreboot/+/46058/1/src/soc/amd/picasso/monoton... 
PS1, Line 10: 	mono_time_set_usecs(mt, timestamp_get());
> This is assuming the tsc is aligned in time w/ psp (i.e. all values are *after* what psp reported). […]

PSP timestamps will be adjusted to relative to TSC in CB:45059.

[Martin Roth (Code Review)]

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46058 )

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

Patch Set 1:

Could you address Eric's comment in the commit message and mark all comments as resolved?

[Kangheui Won (Code Review)]

Hello build bot (Jenkins), Martin Roth, Furquan Shaikh, Patrick Georgi, Rob Barnes, Eric Peers,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46058

to look at the new patch set (#2).

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

soc/amd/picasso: add monotonic_timer

On Zork(picasso) platform we run verstage on the PSP. It has its own
timer, but the frequency is not matched with TSC.

To ease the work to merge timestamps from the PSP and TSC, add a layer
around tsc to have microsecond granularity for timestamp table. PSP
already records timestamp in microseconds.

BUG=b:159220781
BRANCH=zork
TEST=build, flash and boot, check timestamps are correct

Signed-off-by: Kangheui Won 
Change-Id: Ifced4a84071be8da547e252167ec21cd42f20ccc
---
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/Makefile.inc
A src/soc/amd/picasso/monotonic_timer.c
3 files changed, 22 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/46058/2

[Kangheui Won (Code Review)]

Kangheui Won has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46058 )

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

Patch Set 2:

(1 comment)
> Patch Set 1:
>
> Could you address Eric's comment in the commit message and mark all comments as resolved?

Oops, missed that. Thanks.

https://review.coreboot.org/c/coreboot/+/46058/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/46058/1//COMMIT_MSG@9 
PS1, Line 9: To accommodate better with PSP timestamp, add a layer around tsc to have
> This may need a bit more description for people that cannot access the bug.

Done

[Martin Roth (Code Review)]

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46058 )

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

Patch Set 2: Code-Review+2

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46058 )

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

Patch Set 2: Code-Review+2

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46058 )

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46058/1/src/soc/amd/picasso/monoton... 
File src/soc/amd/picasso/monotonic_timer.c:

https://review.coreboot.org/c/coreboot/+/46058/1/src/soc/amd/picasso/monoton... 
PS1, Line 10: 	mono_time_set_usecs(mt, timestamp_get());
> PSP timestamps will be adjusted to relative to TSC in CB:45059.

Ack

[Aaron Durbin (Code Review)]

Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46058 )

Change subject: soc/amd/picasso: add monotonic_timer
......................................................................

soc/amd/picasso: add monotonic_timer

On Zork(picasso) platform we run verstage on the PSP. It has its own
timer, but the frequency is not matched with TSC.

To ease the work to merge timestamps from the PSP and TSC, add a layer
around tsc to have microsecond granularity for timestamp table. PSP
already records timestamp in microseconds.

BUG=b:159220781
BRANCH=zork
TEST=build, flash and boot, check timestamps are correct

Signed-off-by: Kangheui Won 
Change-Id: Ifced4a84071be8da547e252167ec21cd42f20ccc
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46058
Reviewed-by: Martin Roth 
Reviewed-by: Aaron Durbin 
Reviewed-by: Eric Peers 
Reviewed-by: Rob Barnes 
Tested-by: build bot (Jenkins) 
---
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/Makefile.inc
A src/soc/amd/picasso/monotonic_timer.c
3 files changed, 22 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Martin Roth: Looks good to me, approved
  Aaron Durbin: Looks good to me, approved
  Eric Peers: Looks good to me, but someone else must approve
  Rob Barnes: Looks good to me, but someone else must approve

diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig
index 7e03a41..605b0ea 100644
--- a/src/soc/amd/picasso/Kconfig
+++ b/src/soc/amd/picasso/Kconfig
@@ -24,7 +24,7 @@
    select IOAPIC
    select HAVE_EM100_SUPPORT
    select HAVE_USBDEBUG_OPTIONS
-	select TSC_MONOTONIC_TIMER
+	select COLLECT_TIMESTAMPS_NO_TSC
    select SOC_AMD_COMMON_BLOCK_SPI
    select TSC_SYNC_LFENCE
    select UDELAY_TSC
diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc
index 7ec695a..f0c6ae5 100644
--- a/src/soc/amd/picasso/Makefile.inc
+++ b/src/soc/amd/picasso/Makefile.inc
@@ -18,6 +18,7 @@
 bootblock-y += i2c.c
 bootblock-y += uart.c
 bootblock-$(CONFIG_PICASSO_CONSOLE_UART) += uart_console.c
+bootblock-y += monotonic_timer.c
 bootblock-y += tsc_freq.c
 bootblock-y += gpio.c
 bootblock-y += smi_util.c
@@ -34,6 +35,7 @@
 romstage-y += memmap.c
 romstage-y += uart.c
 romstage-$(CONFIG_PICASSO_CONSOLE_UART) += uart_console.c
+romstage-y += monotonic_timer.c
 romstage-y += tsc_freq.c
 romstage-y += aoac.c
 romstage-y += southbridge.c
@@ -49,6 +51,7 @@
 verstage_x86-y += gpio.c
 verstage_x86-y += uart.c
 verstage_x86-$(CONFIG_PICASSO_CONSOLE_UART) += uart_console.c
+verstage_x86-y += monotonic_timer.c
 verstage_x86-y += tsc_freq.c
 verstage_x86-y += reset.c

@@ -72,6 +75,7 @@
 ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smi_util.c
 ramstage-y += uart.c
 ramstage-$(CONFIG_PICASSO_CONSOLE_UART) += uart_console.c
+ramstage-y += monotonic_timer.c
 ramstage-y += tsc_freq.c
 ramstage-y += finalize.c
 ramstage-y += soc_util.c
@@ -86,6 +90,7 @@

smm-y += smihandler.c
 smm-y += smi_util.c
+smm-y += monotonic_timer.c
 smm-y += tsc_freq.c
 ifeq ($(CONFIG_DEBUG_SMI),y)
 smm-y += uart.c
diff --git a/src/soc/amd/picasso/monotonic_timer.c b/src/soc/amd/picasso/monotonic_timer.c
new file mode 100644
index 0000000..941532c
--- /dev/null
+++ b/src/soc/amd/picasso/monotonic_timer.c
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <cpu/x86/msr.h>
+#include <cpu/x86/tsc.h>
+#include <timer.h>
+#include <timestamp.h>
+
+void timer_monotonic_get(struct mono_time *mt)
+{
+	mono_time_set_usecs(mt, timestamp_get());
+}
+
+uint64_t timestamp_get(void)
+{
+	return rdtscll() / tsc_freq_mhz();
+}