Change in ...coreboot[master]: soc/intel/../../cpu: Helper function to set IA32_PERF_CTL (0x199) MSR

[Subrata Banik (Code Review)]

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32281

Change subject: soc/intel/../../cpu: Helper function to set IA32_PERF_CTL (0x199) MSR
......................................................................

soc/intel/../../cpu: Helper function to set IA32_PERF_CTL (0x199) MSR

This patch replaces multiple IA32_PERF_CTL programming with single
helper function.

TEST=Build and boot WHL and CML platform.

Change-Id: I212daa61aa11191dd832630461b517d3dbedd6e1
Signed-off-by: Subrata Banik 
---
M src/soc/intel/common/block/cpu/cpulib.c
1 file changed, 12 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/32281/1

diff --git a/src/soc/intel/common/block/cpu/cpulib.c b/src/soc/intel/common/block/cpu/cpulib.c
index c847390..9964f2b 100644
--- a/src/soc/intel/common/block/cpu/cpulib.c
+++ b/src/soc/intel/common/block/cpu/cpulib.c
@@ -73,6 +73,13 @@
    return (platform_info.hi >> 1) & 3;
 }

+static void set_perf_control_msr(msr_t msr)
+{
+	wrmsr(IA32_PERF_CTL, msr);
+	printk(BIOS_DEBUG, "CPU: frequency set to %d MHz\n",
+	       ((msr.lo >> 8) & 0xff) * CONFIG_CPU_BCLK_MHZ);
+}
+
 /*
  * TURBO_RATIO_LIMIT MSR (0x1AD) Bits 31:0 indicates the
  * factory configured values for of 1-core, 2-core, 3-core
@@ -93,9 +100,7 @@
    perf_ctl.lo = (msr.lo & 0xff) << 8;
    perf_ctl.hi = 0;

-	wrmsr(IA32_PERF_CTL, perf_ctl);
-	printk(BIOS_DEBUG, "CPU: frequency set to %d MHz\n",
-	       ((perf_ctl.lo >> 8) & 0xff) * CONFIG_CPU_BCLK_MHZ);
+	set_perf_control_msr(perf_ctl);
 }

/*
@@ -113,9 +118,7 @@
    perf_ctl.lo = (msr.lo & 0xff) << 8;
    perf_ctl.hi = 0;

-	wrmsr(IA32_PERF_CTL, perf_ctl);
-	printk(BIOS_DEBUG, "CPU: frequency set to %d MHz\n",
-		((perf_ctl.lo >> 8) & 0xff) * CONFIG_CPU_BCLK_MHZ);
+	set_perf_control_msr(perf_ctl);
 }

/*
@@ -133,9 +136,7 @@
    perf_ctl.lo = msr.lo & 0xff00;
    perf_ctl.hi = 0;

-	wrmsr(IA32_PERF_CTL, perf_ctl);
-	printk(BIOS_DEBUG, "CPU: frequency set to %d MHz\n",
-		((perf_ctl.lo >> 8) & 0xff) * CONFIG_CPU_BCLK_MHZ);
+	set_perf_control_msr(perf_ctl);
 }

/*
@@ -152,9 +153,8 @@
    min_ratio = cpu_get_min_ratio();
    perf_ctl.lo = (min_ratio << 8) & 0xff00;
    perf_ctl.hi = 0;
-	wrmsr(IA32_PERF_CTL, perf_ctl);
-	printk(BIOS_DEBUG, "CPU: frequency set to %u MHz\n",
-			    (min_ratio * CONFIG_CPU_BCLK_MHZ));
+
+	set_perf_control_msr(perf_ctl);
 }

/*

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32281 )

Change subject: soc/intel/../../cpu: Helper function to set IA32_PERF_CTL (0x199) MSR
......................................................................

Patch Set 1: Code-Review+2

[Rizwan Qureshi (Code Review)]

Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32281 )

Change subject: soc/intel/../../cpu: Helper function to set IA32_PERF_CTL (0x199) MSR
......................................................................

Patch Set 1: Code-Review+2

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32281 )

Change subject: soc/intel/../../cpu: Helper function to set IA32_PERF_CTL (0x199) MSR
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/32281/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32281/1//COMMIT_MSG@7 
PS1, Line 7: soc/intel/../../cpu: Helper function to set IA32_PERF_CTL (0x199) MSR
Please make it a statement:
> Add helper function …

The prefix does not need to match the directory structure. Maybe:
> soc/intel/cpulib: Factor out IA32_PERF_CTL (0x199) MSR code

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32281 )

Change subject: soc/intel/../../cpu: Helper function to set IA32_PERF_CTL (0x199) MSR
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/32281/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32281/1//COMMIT_MSG@7 
PS1, Line 7: soc/intel/../../cpu: Helper function to set IA32_PERF_CTL (0x199) MSR
> Please make it a statement: […]

Done

Change in ...coreboot[master]: soc/intel/cpulib: Factor out IA32_PERF_CTL (0x199) MSR code

[Subrata Banik (Code Review)]

Hello Aaron Durbin, Patrick Rudolph, Aamir Bohra, Rizwan Qureshi, Duncan Laurie, build bot (Jenkins), Lijian Zhao, Patrick Georgi, Furquan Shaikh,

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

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

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

Change subject: soc/intel/cpulib: Factor out IA32_PERF_CTL (0x199) MSR code
......................................................................

soc/intel/cpulib: Factor out IA32_PERF_CTL (0x199) MSR code

This patch replaces multiple IA32_PERF_CTL programming with single
helper function.

TEST=Build and boot WHL and CML platform.

Change-Id: I212daa61aa11191dd832630461b517d3dbedd6e1
Signed-off-by: Subrata Banik 
---
M src/soc/intel/common/block/cpu/cpulib.c
1 file changed, 12 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/32281/2

Change in ...coreboot[master]: soc/intel/cpulib: Factor out IA32_PERF_CTL (0x199) MSR code

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32281 )

Change subject: soc/intel/cpulib: Factor out IA32_PERF_CTL (0x199) MSR code
......................................................................

Patch Set 2: Code-Review+2

Change in ...coreboot[master]: soc/intel/cpulib: Factor out IA32_PERF_CTL (0x199) MSR code

[Duncan Laurie (Code Review)]

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

Change subject: soc/intel/cpulib: Factor out IA32_PERF_CTL (0x199) MSR code
......................................................................

Patch Set 2: Code-Review+2

Change in ...coreboot[master]: soc/intel/cpulib: Factor out IA32_PERF_CTL (0x199) MSR code

[Subrata Banik (Code Review)]

Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32281 )

Change subject: soc/intel/cpulib: Factor out IA32_PERF_CTL (0x199) MSR code
......................................................................

soc/intel/cpulib: Factor out IA32_PERF_CTL (0x199) MSR code

This patch replaces multiple IA32_PERF_CTL programming with single
helper function.

TEST=Build and boot WHL and CML platform.

Change-Id: I212daa61aa11191dd832630461b517d3dbedd6e1
Signed-off-by: Subrata Banik 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32281
Tested-by: build bot (Jenkins) 
Reviewed-by: Arthur Heymans 
Reviewed-by: Duncan Laurie 
---
M src/soc/intel/common/block/cpu/cpulib.c
1 file changed, 12 insertions(+), 12 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Duncan Laurie: Looks good to me, approved
  Arthur Heymans: Looks good to me, approved

diff --git a/src/soc/intel/common/block/cpu/cpulib.c b/src/soc/intel/common/block/cpu/cpulib.c
index c847390..9964f2b 100644
--- a/src/soc/intel/common/block/cpu/cpulib.c
+++ b/src/soc/intel/common/block/cpu/cpulib.c
@@ -73,6 +73,13 @@
    return (platform_info.hi >> 1) & 3;
 }

+static void set_perf_control_msr(msr_t msr)
+{
+	wrmsr(IA32_PERF_CTL, msr);
+	printk(BIOS_DEBUG, "CPU: frequency set to %d MHz\n",
+	       ((msr.lo >> 8) & 0xff) * CONFIG_CPU_BCLK_MHZ);
+}
+
 /*
  * TURBO_RATIO_LIMIT MSR (0x1AD) Bits 31:0 indicates the
  * factory configured values for of 1-core, 2-core, 3-core
@@ -93,9 +100,7 @@
    perf_ctl.lo = (msr.lo & 0xff) << 8;
    perf_ctl.hi = 0;

-	wrmsr(IA32_PERF_CTL, perf_ctl);
-	printk(BIOS_DEBUG, "CPU: frequency set to %d MHz\n",
-	       ((perf_ctl.lo >> 8) & 0xff) * CONFIG_CPU_BCLK_MHZ);
+	set_perf_control_msr(perf_ctl);
 }

/*
@@ -113,9 +118,7 @@
    perf_ctl.lo = (msr.lo & 0xff) << 8;
    perf_ctl.hi = 0;

-	wrmsr(IA32_PERF_CTL, perf_ctl);
-	printk(BIOS_DEBUG, "CPU: frequency set to %d MHz\n",
-		((perf_ctl.lo >> 8) & 0xff) * CONFIG_CPU_BCLK_MHZ);
+	set_perf_control_msr(perf_ctl);
 }

/*
@@ -133,9 +136,7 @@
    perf_ctl.lo = msr.lo & 0xff00;
    perf_ctl.hi = 0;

-	wrmsr(IA32_PERF_CTL, perf_ctl);
-	printk(BIOS_DEBUG, "CPU: frequency set to %d MHz\n",
-		((perf_ctl.lo >> 8) & 0xff) * CONFIG_CPU_BCLK_MHZ);
+	set_perf_control_msr(perf_ctl);
 }

/*
@@ -152,9 +153,8 @@
    min_ratio = cpu_get_min_ratio();
    perf_ctl.lo = (min_ratio << 8) & 0xff00;
    perf_ctl.hi = 0;
-	wrmsr(IA32_PERF_CTL, perf_ctl);
-	printk(BIOS_DEBUG, "CPU: frequency set to %u MHz\n",
-			    (min_ratio * CONFIG_CPU_BCLK_MHZ));
+
+	set_perf_control_msr(perf_ctl);
 }

/*