Change in ...coreboot[master]: cpu/intel/speedstep/acpi: Use get_ia32_fsb_x3() function

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31432

Change subject: cpu/intel/speedstep/acpi: Use get_ia32_fsb_x3() function
......................................................................

cpu/intel/speedstep/acpi: Use get_ia32_fsb_x3() function

Change-Id: Ie8c5d5f7dd5b43becc144fd5e62d7de2f1ed3b80
Signed-off-by: Elyes HAOUAS 
---
M src/cpu/intel/speedstep/acpi.c
1 file changed, 7 insertions(+), 24 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/31432/1

diff --git a/src/cpu/intel/speedstep/acpi.c b/src/cpu/intel/speedstep/acpi.c
index b5e2c9e9..7bc67ff 100644
--- a/src/cpu/intel/speedstep/acpi.c
+++ b/src/cpu/intel/speedstep/acpi.c
@@ -21,6 +21,7 @@
 #include <arch/acpigen.h>
 #include <arch/cpu.h>
 #include <cpu/x86/msr.h>
+#include <cpu/intel/fsb.h>
 #include <cpu/intel/speedstep.h>
 #include <device/device.h>
 #include <string.h>
@@ -41,29 +42,6 @@
    return count;
 }

-/**
- * @brief Returns three times the FSB clock in MHz
- *
- * The result of calculations with the returned value shall be divided by 3.
- * This helps to avoid rounding errors.
- */
-static int get_fsb(void)
-{
-	const u32 fsbcode = rdmsr(MSR_FSB_FREQ).lo & 7;
-	switch (fsbcode) {
-	case 0: return  800; /*  / 3 == 266 */
-	case 1: return  400; /*  / 3 == 133 */
-	case 2: return  600; /*  / 3 == 200 */
-	case 3: return  500; /*  / 3 == 166 */
-	case 4: return 1000; /*  / 3 == 333 */
-	case 5: return  300; /*  / 3 == 100 */
-	case 6: return 1200; /*  / 3 == 400 */
-	}
-	printk(BIOS_WARNING,
-	       "Warning: No supported FSB frequency. Assuming 200MHz\n");
-	return 600;
-}
-
 static void gen_pstate_entries(const sst_table_t *const pstates,
    		      const int cpuID, const int cores_per_package,
    		      const uint8_t coordination)
@@ -76,7 +54,12 @@
    		cpuID, cores_per_package, coordination);
    acpigen_write_name("_PSS");

-	const int fsb3 = get_fsb();
+	int fsb3 = get_ia32_fsb_x3();
+	if (fsb3 <= 0) {
+		printk(BIOS_ERR, "CPU or FSB not supported. Assuming 200MHz\n");
+		fsb3 = 600;
+	}
+
    const int min_ratio2 = SPEEDSTEP_DOUBLE_RATIO(
    	pstates->states[pstates->num_states - 1]);
    const int max_ratio2 = SPEEDSTEP_DOUBLE_RATIO(pstates->states[0]);

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

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

Change subject: cpu/intel/speedstep/acpi: Use get_ia32_fsb_x3() function
......................................................................

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/+/31432 )

Change subject: cpu/intel/speedstep/acpi: Use get_ia32_fsb_x3() function
......................................................................

cpu/intel/speedstep/acpi: Use get_ia32_fsb_x3() function

Change-Id: Ie8c5d5f7dd5b43becc144fd5e62d7de2f1ed3b80
Signed-off-by: Elyes HAOUAS 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31432
Reviewed-by: Kyösti Mälkki 
Tested-by: build bot (Jenkins) 
---
M src/cpu/intel/speedstep/acpi.c
1 file changed, 7 insertions(+), 24 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, approved

diff --git a/src/cpu/intel/speedstep/acpi.c b/src/cpu/intel/speedstep/acpi.c
index 5cff412..7111f5ef 100644
--- a/src/cpu/intel/speedstep/acpi.c
+++ b/src/cpu/intel/speedstep/acpi.c
@@ -21,6 +21,7 @@
 #include <arch/acpigen.h>
 #include <arch/cpu.h>
 #include <cpu/x86/msr.h>
+#include <cpu/intel/fsb.h>
 #include <cpu/intel/speedstep.h>
 #include <device/device.h>

@@ -40,29 +41,6 @@
    return count;
 }

-/**
- * @brief Returns three times the FSB clock in MHz
- *
- * The result of calculations with the returned value shall be divided by 3.
- * This helps to avoid rounding errors.
- */
-static int get_fsb(void)
-{
-	const u32 fsbcode = rdmsr(MSR_FSB_FREQ).lo & 7;
-	switch (fsbcode) {
-	case 0: return  800; /*  / 3 == 266 */
-	case 1: return  400; /*  / 3 == 133 */
-	case 2: return  600; /*  / 3 == 200 */
-	case 3: return  500; /*  / 3 == 166 */
-	case 4: return 1000; /*  / 3 == 333 */
-	case 5: return  300; /*  / 3 == 100 */
-	case 6: return 1200; /*  / 3 == 400 */
-	}
-	printk(BIOS_WARNING,
-	       "Warning: No supported FSB frequency. Assuming 200MHz\n");
-	return 600;
-}
-
 static void gen_pstate_entries(const sst_table_t *const pstates,
    		      const int cpuID, const int cores_per_package,
    		      const uint8_t coordination)
@@ -75,7 +53,12 @@
    		cpuID, cores_per_package, coordination);
    acpigen_write_name("_PSS");

-	const int fsb3 = get_fsb();
+	int fsb3 = get_ia32_fsb_x3();
+	if (fsb3 <= 0) {
+		printk(BIOS_ERR, "CPU or FSB not supported. Assuming 200MHz\n");
+		fsb3 = 600;
+	}
+
    const int min_ratio2 = SPEEDSTEP_DOUBLE_RATIO(
    	pstates->states[pstates->num_states - 1]);
    const int max_ratio2 = SPEEDSTEP_DOUBLE_RATIO(pstates->states[0]);