Skip to contentSkip to conversation

platform/x86/intel/pmc: Disable C1 auto-demotion during suspend

PATCH In review David E. Box 3 participants 5 messages Oct 1, 2024 – Oct 3, 2024

Subsystems (2)
x86 platform drivers Intel PMC core driver
lore
Share & tools
lore

David E. Box

author
#0 lore raw

PATCH 1/1 platform/x86/intel/pmc: Disable C1 auto-demotion during suspend

On some platforms, aggressive C1 auto-demotion may lead to failure to enter the deepest C-state during suspend-to-idle, causing high power consumption. To prevent this, disable C1 auto-demotion during suspend and re-enable on resume.

Signed-off-by David E. Box <[email protected]>
---
 drivers/platform/x86/intel/pmc/arl.c |  3 +--
 drivers/platform/x86/intel/pmc/cnp.c | 28 +++++++++++++++++++++++++++-
 drivers/platform/x86/intel/pmc/lnl.c |  3 +--
 drivers/platform/x86/intel/pmc/mtl.c |  3 +--
 4 files changed, 30 insertions(+), 7 deletions(-)
drivers/platform/x86/intel/pmc/arl.c +1 -2
index e10527c4e3e0..05dec4f5019f 100644
@@ -687,9 +687,8 @@ static void arl_d3_fixup(void)
 static int arl_resume(struct pmc_dev *pmcdev)
 {
 	arl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);
 }
 
 int arl_core_init(struct pmc_dev *pmcdev)
drivers/platform/x86/intel/pmc/cnp.c +27 -1
index 513c02670c5a..5b8b3ac7f061 100644
@@ -7,7 +7,8 @@
  * All Rights Reserved.
  *
  */
-
+#define DEBUG
+#include <linux/suspend.h>
 #include "core.h"
 
 /* Cannon Lake: PGD PFET Enable Ack Status Register(s) bitmap */

@@ -206,8 +207,24 @@ const struct pmc_reg_map cnp_reg_map = {
 	.etr3_offset = ETR3_OFFSET,
 };
 
+
+static DEFINE_PER_CPU(u64, pkg_cst_config);
+
 void cnl_suspend(struct pmc_dev *pmcdev)
 {
+	if (!pm_suspend_via_firmware()) {
+		u64 val;
+		int cpunum;
+
+		for_each_online_cpu(cpunum) {
+			rdmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL, &val);
+			per_cpu(pkg_cst_config, cpunum) = val;
+			val &= ~NHM_C1_AUTO_DEMOTE;
+			wrmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL, val);
+			pr_debug("%s: cpu:%d cst %llx\n", __func__, cpunum, val);
+		}
+	}
+
 	/*
 	 * Due to a hardware limitation, the GBE LTR blocks PC10
 	 * when a cable is attached. To unblock PC10 during suspend,

@@ -220,6 +237,15 @@ int cnl_resume(struct pmc_dev *pmcdev)
 {
 	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
+	if (!pm_suspend_via_firmware()) {
+		int cpunum;
+
+		for_each_online_cpu(cpunum) {
+			pr_debug("%s: cpu:%d cst %llx\n", __func__, cpunum, per_cpu(pkg_cst_config, cpunum));
+			wrmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL, per_cpu(pkg_cst_config, cpunum));
+		}
+	}
+
 	return pmc_core_resume_common(pmcdev);
 }
drivers/platform/x86/intel/pmc/lnl.c +1 -2
index e7a8077d1a3e..be029f12cdf4 100644
@@ -546,9 +546,8 @@ static void lnl_d3_fixup(void)
 static int lnl_resume(struct pmc_dev *pmcdev)
 {
 	lnl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);
 }
 
 int lnl_core_init(struct pmc_dev *pmcdev)
drivers/platform/x86/intel/pmc/mtl.c +1 -2
index 91f2fa728f5c..fc6a89b8979f 100644
@@ -988,9 +988,8 @@ static void mtl_d3_fixup(void)
 static int mtl_resume(struct pmc_dev *pmcdev)
 {
 	mtl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);
 }
 
 int mtl_core_init(struct pmc_dev *pmcdev)
Signature · 1 line
2.43.0
#1 lore raw
Quoted 33 lines · David E. Box

On some platforms, aggressive C1 auto-demotion may lead to failure to enter the deepest C-state during suspend-to-idle, causing high power consumption. To prevent this, disable C1 auto-demotion during suspend and re-enable on resume.

Signed-off-by David E. Box <[email protected]>
---
  drivers/platform/x86/intel/pmc/arl.c |  3 +--
  drivers/platform/x86/intel/pmc/cnp.c | 28 +++++++++++++++++++++++++++-
  drivers/platform/x86/intel/pmc/lnl.c |  3 +--
  drivers/platform/x86/intel/pmc/mtl.c |  3 +--
  4 files changed, 30 insertions(+), 7 deletions(-)
drivers/platform/x86/intel/pmc/arl.c +1 -2
index e10527c4e3e0..05dec4f5019f 100644
@@ -687,9 +687,8 @@ static void arl_d3_fixup(void)
  static int arl_resume(struct pmc_dev *pmcdev)
  {
  	arl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);
  }
 
  int arl_core_init(struct pmc_dev *pmcdev)
drivers/platform/x86/intel/pmc/cnp.c +0 -0
index 513c02670c5a..5b8b3ac7f061 100644

drivers/platform/x86/intel/pmc/cnp.c line 10

Quoted 5 lines
drivers/platform/x86/intel/pmc/cnp.c +1 -1
@@ -7,7 +7,8 @@
   * All Rights Reserved.
   *
   */
-
+#define DEBUG

Did you mean to pull this out before submitting?

Quoted 75 lines
drivers/platform/x86/intel/pmc/cnp.c +26 -0
+#include <linux/suspend.h>
  #include "core.h"
 
  /* Cannon Lake: PGD PFET Enable Ack Status Register(s) bitmap */

@@ -206,8 +207,24 @@ const struct pmc_reg_map cnp_reg_map = {
  	.etr3_offset = ETR3_OFFSET,
  };
 
+
+static DEFINE_PER_CPU(u64, pkg_cst_config);
+
  void cnl_suspend(struct pmc_dev *pmcdev)
  {
+	if (!pm_suspend_via_firmware()) {
+		u64 val;
+		int cpunum;
+
+		for_each_online_cpu(cpunum) {
+			rdmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL, &val);
+			per_cpu(pkg_cst_config, cpunum) = val;
+			val &= ~NHM_C1_AUTO_DEMOTE;
+			wrmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL, val);
+			pr_debug("%s: cpu:%d cst %llx\n", __func__, cpunum, val);
+		}
+	}
+
  	/*
  	 * Due to a hardware limitation, the GBE LTR blocks PC10
  	 * when a cable is attached. To unblock PC10 during suspend,

@@ -220,6 +237,15 @@ int cnl_resume(struct pmc_dev *pmcdev)
  {
  	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
+	if (!pm_suspend_via_firmware()) {
+		int cpunum;
+
+		for_each_online_cpu(cpunum) {
+			pr_debug("%s: cpu:%d cst %llx\n", __func__, cpunum, per_cpu(pkg_cst_config, cpunum));
+			wrmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL, per_cpu(pkg_cst_config, cpunum));
+		}
+	}
+
  	return pmc_core_resume_common(pmcdev);
  }
drivers/platform/x86/intel/pmc/lnl.c +1 -2
index e7a8077d1a3e..be029f12cdf4 100644
@@ -546,9 +546,8 @@ static void lnl_d3_fixup(void)
  static int lnl_resume(struct pmc_dev *pmcdev)
  {
  	lnl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);
  }
 
  int lnl_core_init(struct pmc_dev *pmcdev)
drivers/platform/x86/intel/pmc/mtl.c +1 -2
index 91f2fa728f5c..fc6a89b8979f 100644
@@ -988,9 +988,8 @@ static void mtl_d3_fixup(void)
  static int mtl_resume(struct pmc_dev *pmcdev)
  {
  	mtl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);
  }
 
  int mtl_core_init(struct pmc_dev *pmcdev)
author
#2 lore raw
Quoted 35 lines · Mario Limonciello
Quoted 34 lines · David E. Box

On some platforms, aggressive C1 auto-demotion may lead to failure to enter the deepest C-state during suspend-to-idle, causing high power consumption. To prevent this, disable C1 auto-demotion during suspend and re-enable on resume.

Signed-off-by David E. Box <[email protected]>

---

  drivers/platform/x86/intel/pmc/arl.c |  3 +--
  drivers/platform/x86/intel/pmc/cnp.c | 28 +++++++++++++++++++++++++++-
  drivers/platform/x86/intel/pmc/lnl.c |  3 +--
  drivers/platform/x86/intel/pmc/mtl.c |  3 +--
  4 files changed, 30 insertions(+), 7 deletions(-)
drivers/platform/x86/intel/pmc/arl.c +0 -1
index e10527c4e3e0..05dec4f5019f 100644
@@ -687,9 +687,8 @@ static void arl_d3_fixup(void)
   static int arl_resume(struct pmc_dev *pmcdev)
   {
   	arl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);

drivers/platform/x86/intel/pmc/arl.c +1 -1
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);

  }

  int arl_core_init(struct pmc_dev *pmcdev)

drivers/platform/x86/intel/pmc/cnp.c +0 -0
index 513c02670c5a..5b8b3ac7f061 100644
Quoted 8 lines

drivers/platform/x86/intel/pmc/cnp.c line 10

Quoted 6 lines
drivers/platform/x86/intel/pmc/cnp.c +1 -1
@@ -7,7 +7,8 @@
    * All Rights Reserved.
    *
    */
-
+#define DEBUG

Did you mean to pull this out before submitting?

Yep. Thanks.

Quoted 84 lines
Quoted 83 lines
drivers/platform/x86/intel/pmc/cnp.c +1 -0
+#include <linux/suspend.h>
   #include "core.h"


  /* Cannon Lake: PGD PFET Enable Ack Status Register(s) bitmap */

drivers/platform/x86/intel/pmc/cnp.c +0 -0
@@ -206,8 +207,24 @@ const struct pmc_reg_map cnp_reg_map = {
   	.etr3_offset = ETR3_OFFSET,
   };

drivers/platform/x86/intel/pmc/cnp.c +3 -0
+
+static DEFINE_PER_CPU(u64, pkg_cst_config);
+

  void cnl_suspend(struct pmc_dev *pmcdev)
  {

drivers/platform/x86/intel/pmc/cnp.c +6 -0
+	if (!pm_suspend_via_firmware()) {
+		u64 val;
+		int cpunum;
+
+		for_each_online_cpu(cpunum) {
+			rdmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL,

&val);

drivers/platform/x86/intel/pmc/cnp.c +3 -0
+			per_cpu(pkg_cst_config, cpunum) = val;
+			val &= ~NHM_C1_AUTO_DEMOTE;
+			wrmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL,

val);

drivers/platform/x86/intel/pmc/cnp.c +1 -0
+			pr_debug("%s: cpu:%d cst %llx\n", __func__, cpunum,

val);

drivers/platform/x86/intel/pmc/cnp.c +3 -0
+		}
+	}
+

   /*
   * Due to a hardware limitation, the GBE LTR blocks PC10
   * when a cable is attached. To unblock PC10 during suspend,

drivers/platform/x86/intel/pmc/cnp.c +0 -0
@@ -220,6 +237,15 @@ int cnl_resume(struct pmc_dev *pmcdev)
   {
   	pmc_core_send_ltr_ignore(pmcdev, 3, 0);

drivers/platform/x86/intel/pmc/cnp.c +5 -0
+	if (!pm_suspend_via_firmware()) {
+		int cpunum;
+
+		for_each_online_cpu(cpunum) {
+			pr_debug("%s: cpu:%d cst %llx\n", __func__, cpunum,

per_cpu(pkg_cst_config, cpunum));

drivers/platform/x86/intel/pmc/cnp.c +1 -0
+			wrmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL,

per_cpu(pkg_cst_config, cpunum));

drivers/platform/x86/intel/pmc/cnp.c +3 -0
+		}
+	}
+

   return pmc_core_resume_common(pmcdev);
  }

drivers/platform/x86/intel/pmc/lnl.c +0 -1
index e7a8077d1a3e..be029f12cdf4 100644
@@ -546,9 +546,8 @@ static void lnl_d3_fixup(void)
   static int lnl_resume(struct pmc_dev *pmcdev)
   {
   	lnl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);

drivers/platform/x86/intel/pmc/lnl.c +1 -1
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);

  }

  int lnl_core_init(struct pmc_dev *pmcdev)

drivers/platform/x86/intel/pmc/mtl.c +0 -1
index 91f2fa728f5c..fc6a89b8979f 100644
@@ -988,9 +988,8 @@ static void mtl_d3_fixup(void)
   static int mtl_resume(struct pmc_dev *pmcdev)
   {
   	mtl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);

drivers/platform/x86/intel/pmc/mtl.c +1 -1
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);

  }

  int mtl_core_init(struct pmc_dev *pmcdev)

maintainer
#3 lore raw
In reply to David E. Box
Quoted 33 lines · David E. Box

On some platforms, aggressive C1 auto-demotion may lead to failure to enter the deepest C-state during suspend-to-idle, causing high power consumption. To prevent this, disable C1 auto-demotion during suspend and re-enable on resume.

Signed-off-by David E. Box <[email protected]>
---
 drivers/platform/x86/intel/pmc/arl.c |  3 +--
 drivers/platform/x86/intel/pmc/cnp.c | 28 +++++++++++++++++++++++++++-
 drivers/platform/x86/intel/pmc/lnl.c |  3 +--
 drivers/platform/x86/intel/pmc/mtl.c |  3 +--
 4 files changed, 30 insertions(+), 7 deletions(-)
drivers/platform/x86/intel/pmc/arl.c +1 -2
index e10527c4e3e0..05dec4f5019f 100644
@@ -687,9 +687,8 @@ static void arl_d3_fixup(void)
 static int arl_resume(struct pmc_dev *pmcdev)
 {
 	arl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);
 }
 
 int arl_core_init(struct pmc_dev *pmcdev)
drivers/platform/x86/intel/pmc/cnp.c +0 -0
index 513c02670c5a..5b8b3ac7f061 100644

drivers/platform/x86/intel/pmc/cnp.c line 10

Quoted 5 lines
drivers/platform/x86/intel/pmc/cnp.c +1 -1
@@ -7,7 +7,8 @@
  * All Rights Reserved.
  *
  */
-
+#define DEBUG

Debug leftover.

drivers/platform/x86/intel/pmc/cnp.c const struct pmc_reg_map cnp_reg_map = { · line 210

Quoted 9 lines
drivers/platform/x86/intel/pmc/cnp.c +2 -0
+#include <linux/suspend.h>
 #include "core.h"
 
 /* Cannon Lake: PGD PFET Enable Ack Status Register(s) bitmap */

@@ -206,8 +207,24 @@ const struct pmc_reg_map cnp_reg_map = {
 	.etr3_offset = ETR3_OFFSET,
 };
 
+

Extra newline.

drivers/platform/x86/intel/pmc/cnp.c const struct pmc_reg_map cnp_reg_map = { · line 221

Quoted 11 lines
drivers/platform/x86/intel/pmc/cnp.c +9 -0
+static DEFINE_PER_CPU(u64, pkg_cst_config);
+
 void cnl_suspend(struct pmc_dev *pmcdev)
 {
+	if (!pm_suspend_via_firmware()) {
+		u64 val;
+		int cpunum;
+
+		for_each_online_cpu(cpunum) {
+			rdmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL, &val);
+			per_cpu(pkg_cst_config, cpunum) = val;

I'm not sure about this. It feels abuse of per cpu stuff as here a single thread is just recording something for every CPU (not the CPUs themselves).

(But my thinking of what's the purpose of per cpu stuff might be incomplete so I'm open to changing my mind about this if proven otherwise.)

Any thoughts about this Hans? Or Andy?

Quoted 10 lines
drivers/platform/x86/intel/pmc/cnp.c +6 -0
+			val &= ~NHM_C1_AUTO_DEMOTE;
+			wrmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL, val);
+			pr_debug("%s: cpu:%d cst %llx\n", __func__, cpunum, val);
+		}
+	}
+
 	/*
 	 * Due to a hardware limitation, the GBE LTR blocks PC10
 	 * when a cable is attached. To unblock PC10 during suspend,

drivers/platform/x86/intel/pmc/cnp.c int cnl_resume(struct pmc_dev *pmcdev) · line 245

Quoted 9 lines
drivers/platform/x86/intel/pmc/cnp.c +6 -0
@@ -220,6 +237,15 @@ int cnl_resume(struct pmc_dev *pmcdev)
 {
 	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
+	if (!pm_suspend_via_firmware()) {
+		int cpunum;
+
+		for_each_online_cpu(cpunum) {
+			pr_debug("%s: cpu:%d cst %llx\n", __func__, cpunum, per_cpu(pkg_cst_config, cpunum));
+			wrmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL, per_cpu(pkg_cst_config, cpunum));

Is the set of onlined CPUs iterated by the suspend and resume handlers guaranteed to be the same?

Signature · 1 line
 i.
Quoted 37 lines
drivers/platform/x86/intel/pmc/cnp.c +3 -0
+		}
+	}
+
 	return pmc_core_resume_common(pmcdev);
 }
drivers/platform/x86/intel/pmc/lnl.c +1 -2
index e7a8077d1a3e..be029f12cdf4 100644
@@ -546,9 +546,8 @@ static void lnl_d3_fixup(void)
 static int lnl_resume(struct pmc_dev *pmcdev)
 {
 	lnl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);
 }
 
 int lnl_core_init(struct pmc_dev *pmcdev)
drivers/platform/x86/intel/pmc/mtl.c +1 -2
index 91f2fa728f5c..fc6a89b8979f 100644
@@ -988,9 +988,8 @@ static void mtl_d3_fixup(void)
 static int mtl_resume(struct pmc_dev *pmcdev)
 {
 	mtl_d3_fixup();
-	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
 
-	return pmc_core_resume_common(pmcdev);
+	return cnl_resume(pmcdev);
 }
 
 int mtl_core_init(struct pmc_dev *pmcdev)
maintainer
#4 lore raw
Quoted 14 lines · Ilpo Järvinen
Quoted 12 lines · David E. Box

On some platforms, aggressive C1 auto-demotion may lead to failure to enter the deepest C-state during suspend-to-idle, causing high power consumption. To prevent this, disable C1 auto-demotion during suspend and re-enable on resume.

Signed-off-by David E. Box <[email protected]>

---

drivers/platform/x86/intel/pmc/cnp.c +0 -0
@@ -220,6 +237,15 @@ int cnl_resume(struct pmc_dev *pmcdev)
 {
 	pmc_core_send_ltr_ignore(pmcdev, 3, 0);
Quoted 9 lines

drivers/platform/x86/intel/pmc/cnp.c int cnl_resume(struct pmc_dev *pmcdev) · line 245

Quoted 6 lines
drivers/platform/x86/intel/pmc/cnp.c +6 -0
+	if (!pm_suspend_via_firmware()) {
+		int cpunum;
+
+		for_each_online_cpu(cpunum) {
+			pr_debug("%s: cpu:%d cst %llx\n", __func__, cpunum, per_cpu(pkg_cst_config, cpunum));
+			wrmsrl_on_cpu(cpunum, MSR_PKG_CST_CONFIG_CONTROL, per_cpu(pkg_cst_config, cpunum));

Is the set of onlined CPUs iterated by the suspend and resume handlers guaranteed to be the same?

I asked about this from more knowledgeable people than me and got a response that during suspend/resume userspace cannot offline any CPUs so it should be guaranteed to remain the same for resume.

Signature · 1 line
 i.