Change in ...coreboot[master]: soc/amd/picasso: Update calculation for TSC frequency

[Marshall Dawson (Code Review)]

Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33765

Change subject: soc/amd/picasso: Update calculation for TSC frequency
......................................................................

soc/amd/picasso: Update calculation for TSC frequency

Remove the Family 15h step of finding the number of boost states
and get the frequency directly out of the Pstate0 register.

Signed-off-by: Marshall Dawson 
Change-Id: I909743483309279eb8c3bf68852d6082381f0dff
---
M src/soc/amd/picasso/tsc_freq.c
1 file changed, 3 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/33765/1

diff --git a/src/soc/amd/picasso/tsc_freq.c b/src/soc/amd/picasso/tsc_freq.c
index 29121b9..2277b33 100644
--- a/src/soc/amd/picasso/tsc_freq.c
+++ b/src/soc/amd/picasso/tsc_freq.c
@@ -19,26 +19,16 @@
 #include <cpu/amd/msr.h>
 #include <cpu/x86/tsc.h>
 #include <console/console.h>
-#include <soc/pci_devs.h>
-#include <device/pci_ops.h>

unsigned long tsc_freq_mhz(void)
 {
    msr_t msr;
    uint8_t cpufid;
    uint8_t cpudid;
-	uint8_t boost_states;
+	uint8_t high_state;

-	/*
-	 * See the Family 15h Models 70h-7Fh BKDG (PID 55072) definition for
-	 * MSR0000_0010.  The TSC increments at the P0 frequency. According
-	 * to the "Software P-state Numbering" section, P0 is the highest
-	 * non-boosted state.  freq = 100MHz * (CpuFid + 10h) / (2^(CpuDid)).
-	 */
-	boost_states = (pci_read_config32(SOC_PM_DEV, CORE_PERF_BOOST_CTRL)
-			>> 2) & 0x7;
-
-	msr = rdmsr(PSTATE_0_MSR + boost_states);
+	high_state = rdmsr(PS_LIM_REG).lo & 0x7;
+	msr = rdmsr(PSTATE_0_MSR + high_state);
    if (!(msr.hi & 0x80000000))
    	die("Unknown error: cannot determine P-state 0\n");

Change in coreboot[master]: soc/amd/picasso: Update calculation for TSC frequency

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33765 )

Change subject: soc/amd/picasso: Update calculation for TSC frequency
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33765/4/src/soc/amd/picasso/tsc_fre... 
File src/soc/amd/picasso/tsc_freq.c:

https://review.coreboot.org/c/coreboot/+/33765/4/src/soc/amd/picasso/tsc_fre... 
PS4, Line 38: 	return (100 * (cpufid + 0x10)) / (0x01 << cpudid);
If this is constant, you may want to cache the result (declare static variable for return value) as rdmsr is sometimes heavy for execution and worth skipping when possible.

Change in coreboot[master]: soc/amd/picasso: Update TSC and monotonic timer

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33765 )

Change subject: soc/amd/picasso: Update TSC and monotonic timer
......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33765/4/src/soc/amd/picasso/tsc_fre... 
File src/soc/amd/picasso/tsc_freq.c:

https://review.coreboot.org/c/coreboot/+/33765/4/src/soc/amd/picasso/tsc_fre... 
PS4, Line 38: 	return (100 * (cpufid + 0x10)) / (0x01 << cpudid);
> If this is constant, you may want to cache the result (declare static variable for return value) as  […]

Done.  Unsure how long the cycle takes for this register.

Change in coreboot[master]: soc/amd/picasso: Update TSC and monotonic timer

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33765 )

Change subject: soc/amd/picasso: Update TSC and monotonic timer
......................................................................

Patch Set 13: Code-Review+2

Change in coreboot[master]: soc/amd/picasso: Update TSC and monotonic timer

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33765 )

Change subject: soc/amd/picasso: Update TSC and monotonic timer
......................................................................

Patch Set 14: Code-Review+2

Change in coreboot[master]: soc/amd/picasso: Update TSC and monotonic timer

[Martin Roth (Code Review)]

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/33765 )

Change subject: soc/amd/picasso: Update TSC and monotonic timer
......................................................................

soc/amd/picasso: Update TSC and monotonic timer

Picasso's TimeStamp Counter is a new design and different than
Stoney Ridge.  Although advertised as invariant, the ST TSC did
not become so until midway through POST making it an unreliable
source for measuring time.  This is not the case for Picasso.

Remove the Stoney Ridge monotonic timer code and rely on the TSC.

Modify the calculation used in Family 15h of finding the number
of boost states first, and get the frequency directly out of the
Pstate0 register.

Signed-off-by: Marshall Dawson 
Change-Id: I909743483309279eb8c3bf68852d6082381f0dff
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33765
Tested-by: build bot (Jenkins) 
Reviewed-by: Richard Spiegel 
---
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/Makefile.inc
D src/soc/amd/picasso/monotonic_timer.c
M src/soc/amd/picasso/tsc_freq.c
4 files changed, 12 insertions(+), 61 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Richard Spiegel: Looks good to me, approved

diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig
index 26b2ec4..b3ee50c 100644
--- a/src/soc/amd/picasso/Kconfig
+++ b/src/soc/amd/picasso/Kconfig
@@ -29,14 +29,15 @@
    select X86_AMD_FIXED_MTRRS
    select X86_AMD_INIT_SIPI
    select ACPI_AMD_HARDWARE_SLEEP_VALUES
-	select COLLECT_TIMESTAMPS_NO_TSC
    select DRIVERS_I2C_DESIGNWARE
    select GENERIC_GPIO_LIB
-	select GENERIC_UDELAY
    select IOAPIC
    select HAVE_USBDEBUG_OPTIONS
    select SPI_FLASH if HAVE_ACPI_RESUME
+	select TSC_CONSTANT_RATE
+	select TSC_MONOTONIC_TIMER
    select TSC_SYNC_LFENCE
+	select UDELAY_TSC
    select COLLECT_TIMESTAMPS
    select SOC_AMD_PI
    select SOC_AMD_COMMON
@@ -66,10 +67,6 @@
    select VBOOT_VBNV_CMOS
    select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH

-config UDELAY_LAPIC_FIXED_FSB
-	int
-	default 200
-
 config HAVE_BOOTBLOCK
    bool
    default n
diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc
index 67d8904..ee9b407 100644
--- a/src/soc/amd/picasso/Makefile.inc
+++ b/src/soc/amd/picasso/Makefile.inc
@@ -40,7 +40,6 @@
 romstage-y += i2c.c
 romstage-y += romstage.c
 romstage-y += gpio.c
-romstage-y += monotonic_timer.c
 romstage-y += pmutil.c
 romstage-y += reset.c
 romstage-y += smbus.c
@@ -53,7 +52,6 @@

verstage-y += gpio.c
 verstage-y += i2c.c
-verstage-y += monotonic_timer.c
 verstage-y += pmutil.c
 verstage-y += reset.c
 verstage-$(CONFIG_PICASSO_UART) += uart.c
@@ -72,7 +70,6 @@
 ramstage-y += mca.c
 ramstage-$(CONFIG_HAVE_ACPI_TABLES) += acpi.c
 ramstage-y += gpio.c
-ramstage-y += monotonic_timer.c
 ramstage-y += southbridge.c
 ramstage-y += northbridge.c
 ramstage-y += pmutil.c
@@ -89,7 +86,6 @@
 ramstage-$(CONFIG_SPI_FLASH) += spi.c
 ramstage-y += finalize.c

-smm-y += monotonic_timer.c
 smm-y += smihandler.c
 smm-y += smi_util.c
 smm-y += tsc_freq.c
diff --git a/src/soc/amd/picasso/monotonic_timer.c b/src/soc/amd/picasso/monotonic_timer.c
deleted file mode 100644
index 7ea571f..0000000
--- a/src/soc/amd/picasso/monotonic_timer.c
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * This file is part of the coreboot project.
- *
- * Copyright 2018 Google Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include <cpu/x86/msr.h>
-#include <timer.h>
-#include <timestamp.h>
-
-#define CU_PTSC_MSR	0xc0010280
-#define PTSC_FREQ_MHZ	100
-
-void timer_monotonic_get(struct mono_time *mt)
-{
-	mono_time_set_usecs(mt, timestamp_get());
-}
-
-uint64_t timestamp_get(void)
-{
-	unsigned long long val;
-	msr_t msr;
-
-	msr = rdmsr(CU_PTSC_MSR);
-
-	val = ((unsigned long long)msr.hi << 32) | msr.lo;
-
-	return val / PTSC_FREQ_MHZ;
-}
diff --git a/src/soc/amd/picasso/tsc_freq.c b/src/soc/amd/picasso/tsc_freq.c
index 29121b9..6167726 100644
--- a/src/soc/amd/picasso/tsc_freq.c
+++ b/src/soc/amd/picasso/tsc_freq.c
@@ -19,31 +19,27 @@
 #include <cpu/amd/msr.h>
 #include <cpu/x86/tsc.h>
 #include <console/console.h>
-#include <soc/pci_devs.h>
-#include <device/pci_ops.h>
+
+static unsigned long mhz;

unsigned long tsc_freq_mhz(void)
 {
    msr_t msr;
    uint8_t cpufid;
    uint8_t cpudid;
-	uint8_t boost_states;
+	uint8_t high_state;

-	/*
-	 * See the Family 15h Models 70h-7Fh BKDG (PID 55072) definition for
-	 * MSR0000_0010.  The TSC increments at the P0 frequency. According
-	 * to the "Software P-state Numbering" section, P0 is the highest
-	 * non-boosted state.  freq = 100MHz * (CpuFid + 10h) / (2^(CpuDid)).
-	 */
-	boost_states = (pci_read_config32(SOC_PM_DEV, CORE_PERF_BOOST_CTRL)
-			>> 2) & 0x7;
+	if (mhz)
+		return mhz;

-	msr = rdmsr(PSTATE_0_MSR + boost_states);
+	high_state = rdmsr(PS_LIM_REG).lo & 0x7;
+	msr = rdmsr(PSTATE_0_MSR + high_state);
    if (!(msr.hi & 0x80000000))
    	die("Unknown error: cannot determine P-state 0\n");

cpufid = (msr.lo & 0x3f);
    cpudid = (msr.lo & 0x1c0) >> 6;

-	return (100 * (cpufid + 0x10)) / (0x01 << cpudid);
+	mhz = (100 * (cpufid + 0x10)) / (0x01 << cpudid);
+	return mhz;
 }