[Xenial] [PATCH 0/3] KBL: intel_powerclamp driver support

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Xenial] [PATCH 0/3] KBL: intel_powerclamp driver support

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1591641

We need powerclamp support for KBL to get better power and thermal control.

Impact:
Non-optimal power and thermal control on KBL system.

Fix:
Enable powerclamp on KBL.

Test:
Before:
$ lsmod | grep powerclamp
/* Nothing */

After:
$ lsmod | grep powerclamp
intel_powerclamp       16384  0

v2:
Cherry pick some following fixes as Seth Forsee suggested.

Eric Ernst (1):
  thermal/powerclamp: correct cpu support check

Jacob Pan (2):
  thermal/powerclamp: remove cpu whitelist
  thermal/powerclamp: add back module device table

 drivers/thermal/intel_powerclamp.c | 50 +++++++-------------------------------
 1 file changed, 9 insertions(+), 41 deletions(-)

--
2.12.2


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 1/3] thermal/powerclamp: remove cpu whitelist

Kai-Heng Feng
From: Jacob Pan <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1591641

Powerclamp works by aligning idle time to achieve package level
idle states, aka cstates. As long as one of the package cstates
is available, synchronized idle injection is meaningful.

This patch replaces the CPU whitelist with CPU feature and
package cstate counter check such that we don't have to modify
this whitelist for every new CPU.

Signed-off-by: Jacob Pan <[hidden email]>
Signed-off-by: Zhang Rui <[hidden email]>
(cherry picked from commit b721ca0d192754deccb89fb01c77e41e6fd91ad9)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/thermal/intel_powerclamp.c | 47 ++++++++------------------------------
 1 file changed, 9 insertions(+), 38 deletions(-)

diff --git a/drivers/thermal/intel_powerclamp.c b/drivers/thermal/intel_powerclamp.c
index 6c79588251d5..015ce2eb6eb7 100644
--- a/drivers/thermal/intel_powerclamp.c
+++ b/drivers/thermal/intel_powerclamp.c
@@ -510,12 +510,6 @@ static int start_power_clamp(void)
  unsigned long cpu;
  struct task_struct *thread;
 
- /* check if pkg cstate counter is completely 0, abort in this case */
- if (!has_pkg_state_counter()) {
- pr_err("pkg cstate counter not functional, abort\n");
- return -EINVAL;
- }
-
  set_target_ratio = clamp(set_target_ratio, 0U, MAX_TARGET_RATIO - 1);
  /* prevent cpu hotplug */
  get_online_cpus();
@@ -672,35 +666,11 @@ static struct thermal_cooling_device_ops powerclamp_cooling_ops = {
  .set_cur_state = powerclamp_set_cur_state,
 };
 
-/* runs on Nehalem and later */
 static const struct x86_cpu_id intel_powerclamp_ids[] __initconst = {
- { X86_VENDOR_INTEL, 6, 0x1a},
- { X86_VENDOR_INTEL, 6, 0x1c},
- { X86_VENDOR_INTEL, 6, 0x1e},
- { X86_VENDOR_INTEL, 6, 0x1f},
- { X86_VENDOR_INTEL, 6, 0x25},
- { X86_VENDOR_INTEL, 6, 0x26},
- { X86_VENDOR_INTEL, 6, 0x2a},
- { X86_VENDOR_INTEL, 6, 0x2c},
- { X86_VENDOR_INTEL, 6, 0x2d},
- { X86_VENDOR_INTEL, 6, 0x2e},
- { X86_VENDOR_INTEL, 6, 0x2f},
- { X86_VENDOR_INTEL, 6, 0x37},
- { X86_VENDOR_INTEL, 6, 0x3a},
- { X86_VENDOR_INTEL, 6, 0x3c},
- { X86_VENDOR_INTEL, 6, 0x3d},
- { X86_VENDOR_INTEL, 6, 0x3e},
- { X86_VENDOR_INTEL, 6, 0x3f},
- { X86_VENDOR_INTEL, 6, 0x45},
- { X86_VENDOR_INTEL, 6, 0x46},
- { X86_VENDOR_INTEL, 6, 0x47},
- { X86_VENDOR_INTEL, 6, 0x4c},
- { X86_VENDOR_INTEL, 6, 0x4d},
- { X86_VENDOR_INTEL, 6, 0x4e},
- { X86_VENDOR_INTEL, 6, 0x4f},
- { X86_VENDOR_INTEL, 6, 0x56},
- { X86_VENDOR_INTEL, 6, 0x57},
- { X86_VENDOR_INTEL, 6, 0x5e},
+ { X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_MWAIT },
+ { X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_ARAT },
+ { X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_NONSTOP_TSC },
+ { X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_CONSTANT_TSC},
  {}
 };
 MODULE_DEVICE_TABLE(x86cpu, intel_powerclamp_ids);
@@ -712,11 +682,12 @@ static int __init powerclamp_probe(void)
  boot_cpu_data.x86, boot_cpu_data.x86_model);
  return -ENODEV;
  }
- if (!boot_cpu_has(X86_FEATURE_NONSTOP_TSC) ||
- !boot_cpu_has(X86_FEATURE_CONSTANT_TSC) ||
- !boot_cpu_has(X86_FEATURE_MWAIT) ||
- !boot_cpu_has(X86_FEATURE_ARAT))
+
+ /* The goal for idle time alignment is to achieve package cstate. */
+ if (!has_pkg_state_counter()) {
+ pr_info("No package C-state available");
  return -ENODEV;
+ }
 
  /* find the deepest mwait value */
  find_target_mwait();
--
2.12.2


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/3] thermal/powerclamp: correct cpu support check

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: Eric Ernst <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1591641

Initial logic for checking CPU match resulted in OR of CPU features
rather than the intended AND.

Updated to use boot_cpu_has macro rather than x86_match_cpu.

In addition, MWAIT is the only required CPU feature for idle
injection to work.  Drop other feature requirements since they are
only needed for optimal efficiency.

CC: [hidden email] #v4.7
Signed-off-by: Eric Ernst <[hidden email]>
Acked-by: Jacob Pan <[hidden email]>
Signed-off-by: Zhang Rui <[hidden email]>
(cherry picked from commit 3105f234e0aba43e44e277c20f9b32ee8add43d4)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/thermal/intel_powerclamp.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/thermal/intel_powerclamp.c b/drivers/thermal/intel_powerclamp.c
index 015ce2eb6eb7..4b61c89b42d3 100644
--- a/drivers/thermal/intel_powerclamp.c
+++ b/drivers/thermal/intel_powerclamp.c
@@ -666,20 +666,10 @@ static struct thermal_cooling_device_ops powerclamp_cooling_ops = {
  .set_cur_state = powerclamp_set_cur_state,
 };
 
-static const struct x86_cpu_id intel_powerclamp_ids[] __initconst = {
- { X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_MWAIT },
- { X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_ARAT },
- { X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_NONSTOP_TSC },
- { X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_CONSTANT_TSC},
- {}
-};
-MODULE_DEVICE_TABLE(x86cpu, intel_powerclamp_ids);
-
 static int __init powerclamp_probe(void)
 {
- if (!x86_match_cpu(intel_powerclamp_ids)) {
- pr_err("Intel powerclamp does not run on family %d model %d\n",
- boot_cpu_data.x86, boot_cpu_data.x86_model);
+ if (!boot_cpu_has(X86_FEATURE_MWAIT)) {
+ pr_err("CPU does not support MWAIT");
  return -ENODEV;
  }
 
--
2.12.2


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 3/3] thermal/powerclamp: add back module device table

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: Jacob Pan <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1591641

Commit 3105f234e0aba43e44e277c20f9b32ee8add43d4 replaced module
cpu id table with a cpu feature check, which is logically correct.
But we need the module device table to allow module auto loading.

Cc: [hidden email] # 4.8
Fixes:3105f234 thermal/powerclamp: correct cpu support check
Signed-off-by: Jacob Pan <[hidden email]>
Signed-off-by: Zhang Rui <[hidden email]>
(cherry picked from commit ec638db8cb9ddd5ca08b23f2835b6c9c15eb616d)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/thermal/intel_powerclamp.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/intel_powerclamp.c b/drivers/thermal/intel_powerclamp.c
index 4b61c89b42d3..5306add74838 100644
--- a/drivers/thermal/intel_powerclamp.c
+++ b/drivers/thermal/intel_powerclamp.c
@@ -666,9 +666,16 @@ static struct thermal_cooling_device_ops powerclamp_cooling_ops = {
  .set_cur_state = powerclamp_set_cur_state,
 };
 
+static const struct x86_cpu_id __initconst intel_powerclamp_ids[] = {
+ { X86_VENDOR_INTEL, X86_FAMILY_ANY, X86_MODEL_ANY, X86_FEATURE_MWAIT },
+ {}
+};
+MODULE_DEVICE_TABLE(x86cpu, intel_powerclamp_ids);
+
 static int __init powerclamp_probe(void)
 {
- if (!boot_cpu_has(X86_FEATURE_MWAIT)) {
+
+ if (!x86_match_cpu(intel_powerclamp_ids)) {
  pr_err("CPU does not support MWAIT");
  return -ENODEV;
  }
--
2.12.2


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK: [Xenial] [PATCH 0/3] KBL: intel_powerclamp driver support

Seth Forshee
In reply to this post by Kai-Heng Feng
On Thu, Apr 13, 2017 at 04:24:52PM +0800, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1591641
>
> We need powerclamp support for KBL to get better power and thermal control.
>
> Impact:
> Non-optimal power and thermal control on KBL system.
>
> Fix:
> Enable powerclamp on KBL.
>
> Test:
> Before:
> $ lsmod | grep powerclamp
> /* Nothing */
>
> After:
> $ lsmod | grep powerclamp
> intel_powerclamp       16384  0
>
> v2:
> Cherry pick some following fixes as Seth Forsee suggested.
>
> Eric Ernst (1):
>   thermal/powerclamp: correct cpu support check
>
> Jacob Pan (2):
>   thermal/powerclamp: remove cpu whitelist
>   thermal/powerclamp: add back module device table
>
>  drivers/thermal/intel_powerclamp.c | 50 +++++++-------------------------------
>  1 file changed, 9 insertions(+), 41 deletions(-)

Clean, straightforward cherry-picks.

Acked-by: Seth Forshee <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK: [Xenial] [PATCH 0/3] KBL: intel_powerclamp driver support

Colin Ian King-2
In reply to this post by Kai-Heng Feng
On 13/04/17 09:24, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1591641
>
> We need powerclamp support for KBL to get better power and thermal control.
>
> Impact:
> Non-optimal power and thermal control on KBL system.
>
> Fix:
> Enable powerclamp on KBL.
>
> Test:
> Before:
> $ lsmod | grep powerclamp
> /* Nothing */
>
> After:
> $ lsmod | grep powerclamp
> intel_powerclamp       16384  0
>
> v2:
> Cherry pick some following fixes as Seth Forsee suggested.
>
> Eric Ernst (1):
>   thermal/powerclamp: correct cpu support check
>
> Jacob Pan (2):
>   thermal/powerclamp: remove cpu whitelist
>   thermal/powerclamp: add back module device table
>
>  drivers/thermal/intel_powerclamp.c | 50 +++++++-------------------------------
>  1 file changed, 9 insertions(+), 41 deletions(-)
>

Looks good to me, cherry picks are clean.

Acked-by: Colin Ian King <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

APPLIED[X/master-next]: [Xenial] [PATCH 0/3] KBL: intel_powerclamp driver support

Stefan Bader-2
In reply to this post by Kai-Heng Feng



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Loading...