Change in ...coreboot[master]: soc/intel/cannonlake: Disable CpuRatio and SaGv in recovery

[Duncan Laurie (Code Review)]

Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31055

Change subject: soc/intel/cannonlake: Disable CpuRatio and SaGv in recovery
......................................................................

soc/intel/cannonlake: Disable CpuRatio and SaGv in recovery

Disabling CpuRatio UPD for FSP will ensure it does not force a
hard reset to set the CPU Flex Ratio at boot.  This is important
in a recovery mode boot where the SOC will lose power and need
to set the flex ratio again.

Disabling SaGv makes recovery mode training faster and mirrors
the setting that was done on Skylake.

BUG=b:123305400
TEST=reliably enter recovery mode on sarien

Change-Id: Ie9664493a980af9acce82faff81f4c4b1355be73
Signed-off-by: Duncan Laurie 
---
M src/soc/intel/cannonlake/romstage/fsp_params.c
1 file changed, 8 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/31055/1

diff --git a/src/soc/intel/cannonlake/romstage/fsp_params.c b/src/soc/intel/cannonlake/romstage/fsp_params.c
index 91810e8..ed7ece5 100644
--- a/src/soc/intel/cannonlake/romstage/fsp_params.c
+++ b/src/soc/intel/cannonlake/romstage/fsp_params.c
@@ -20,6 +20,7 @@
 #include <soc/iomap.h>
 #include <soc/pci_devs.h>
 #include <soc/romstage.h>
+#include <vendorcode/google/chromeos/chromeos.h>

static void soc_memory_init_params(FSP_M_CONFIG *m_cfg, const config_t *config)
 {
@@ -54,6 +55,13 @@
 #if IS_ENABLED(CONFIG_SOC_INTEL_COFFEELAKE)
    m_cfg->SkipMpInit = !chip_get_fsp_mp_init();
 #endif
+
+	/* Disable CPU Flex Ratio and SaGv in recovery mode */
+	if (vboot_recovery_mode_enabled()) {
+		m_cfg->CpuRatio = 0;
+		m_cfg->SaGv = 0;
+	}
+
    /* If ISH is enabled, enable ISH elements */
    if (!dev)
    	m_cfg->PchIshEnable = 0;

[Duncan Laurie (Code Review)]

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31055 )

Change subject: soc/intel/cannonlake: Disable CpuRatio and SaGv in recovery
......................................................................

Patch Set 1:

This seems to result in an FSP error if the SOC was not actually reset on the recovery path.  (i.e. for a user requested recovery with 'crossystem recovery_request')

[Duncan Laurie (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins),

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

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

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

Change subject: soc/intel/cannonlake: Disable CpuRatio and SaGv in recovery
......................................................................

soc/intel/cannonlake: Disable CpuRatio and SaGv in recovery

Disabling CpuRatio UPD for FSP will ensure it does not force a
hard reset to set the CPU Flex Ratio at boot.  This is important
in a recovery mode boot where the SOC will lose power and need
to set the flex ratio again.

Disabling SaGv makes recovery mode training faster and mirrors
the setting that was done on Skylake.

BUG=b:123305400
TEST=reliably enter recovery mode on sarien

Change-Id: Ie9664493a980af9acce82faff81f4c4b1355be73
Signed-off-by: Duncan Laurie 
---
M src/soc/intel/cannonlake/romstage/fsp_params.c
1 file changed, 17 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/31055/2

[Stefan Reinauer (Code Review)]

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31055 )

Change subject: soc/intel/cannonlake: Disable CpuRatio and SaGv in recovery
......................................................................

Patch Set 2: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31055 )

Change subject: soc/intel/cannonlake: Disable CpuRatio and SaGv in recovery
......................................................................

soc/intel/cannonlake: Disable CpuRatio and SaGv in recovery

Disabling CpuRatio UPD for FSP will ensure it does not force a
hard reset to set the CPU Flex Ratio at boot.  This is important
in a recovery mode boot where the SOC will lose power and need
to set the flex ratio again.

Disabling SaGv makes recovery mode training faster and mirrors
the setting that was done on Skylake.

BUG=b:123305400
TEST=reliably enter recovery mode on sarien

Change-Id: Ie9664493a980af9acce82faff81f4c4b1355be73
Signed-off-by: Duncan Laurie 
Reviewed-on: https://review.coreboot.org/c/31055
Tested-by: build bot (Jenkins) 
Reviewed-by: Stefan Reinauer 
---
M src/soc/intel/cannonlake/romstage/fsp_params.c
1 file changed, 17 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Stefan Reinauer: Looks good to me, approved

diff --git a/src/soc/intel/cannonlake/romstage/fsp_params.c b/src/soc/intel/cannonlake/romstage/fsp_params.c
index 91810e8..bdaa4af 100644
--- a/src/soc/intel/cannonlake/romstage/fsp_params.c
+++ b/src/soc/intel/cannonlake/romstage/fsp_params.c
@@ -17,9 +17,11 @@
 #include <chip.h>
 #include <console/console.h>
 #include <fsp/util.h>
+#include <intelblocks/pmclib.h>
 #include <soc/iomap.h>
 #include <soc/pci_devs.h>
 #include <soc/romstage.h>
+#include <vendorcode/google/chromeos/chromeos.h>

static void soc_memory_init_params(FSP_M_CONFIG *m_cfg, const config_t *config)
 {
@@ -54,6 +56,21 @@
 #if IS_ENABLED(CONFIG_SOC_INTEL_COFFEELAKE)
    m_cfg->SkipMpInit = !chip_get_fsp_mp_init();
 #endif
+
+	/* Disable CPU Flex Ratio and SaGv in recovery mode */
+	if (vboot_recovery_mode_enabled()) {
+		struct chipset_power_state *ps = pmc_get_power_state();
+
+		/*
+		 * Only disable when coming from S5 (cold reset) otherwise
+		 * the flex ratio may be locked and FSP will return an error.
+		 */
+		if (ps && ps->prev_sleep_state == ACPI_S5) {
+			m_cfg->CpuRatio = 0;
+			m_cfg->SaGv = 0;
+		}
+	}
+
    /* If ISH is enabled, enable ISH elements */
    if (!dev)
    	m_cfg->PchIshEnable = 0;