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

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

[Xenial] [PATCH 1/1] 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.11.0


--
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

NACK: [Xenial] [PATCH 1/1] thermal/powerclamp: remove cpu whitelist

Seth Forshee
On Fri, Apr 07, 2017 at 03:21:27PM +0100, Kai-Heng Feng wrote:

> 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]>

It looks to me like there are some follow-up commits needed, one to fix
a bug in this patch (loading powerclamp on cpus which don't support it)
and another to fix a bug introduced by the previous fix.

3105f234e0ab thermal/powerclamp: correct cpu support check
ec638db8cb9d thermal/powerclamp: add back module device table

Thanks,
Seth

--
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

Re: NACK: [Xenial] [PATCH 1/1] thermal/powerclamp: remove cpu whitelist

Kai-Heng Feng


On Mon, Apr 10, 2017 at 11:43 PM Seth Forshee <[hidden email]> wrote:

It looks to me like there are some follow-up commits needed, one to fix
a bug in this patch (loading powerclamp on cpus which don't support it)
and another to fix a bug introduced by the previous fix.

3105f234e0ab thermal/powerclamp: correct cpu support check
ec638db8cb9d thermal/powerclamp: add back module device table

You are right, I'll resend patches after test these commits on a real machine.
 

Thanks,
Seth

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