linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Powercap RAPL update for Denverton
@ 2016-05-31 20:41 Jacob Pan
  2016-05-31 20:41 ` [PATCH 1/2] powercap/rapl: handle missing msrs Jacob Pan
  2016-05-31 20:41 ` [PATCH 2/2] powercap/rapl: add support for denverton Jacob Pan
  0 siblings, 2 replies; 9+ messages in thread
From: Jacob Pan @ 2016-05-31 20:41 UTC (permalink / raw)
  To: LKML, Linux PM, Rafael Wysocki, Srinivas Pandruvada; +Cc: Jacob Pan

A few MSRs are missing from Denverton RAPL interface which makes domain
detection fail prematurely. The patchset adds flexibility to the detection
logic such that it will support what functionality remains available.

Jacob Pan (2):
  powercap/rapl: handle missing msrs
  powercap/rapl: add support for denverton

 drivers/powercap/intel_rapl.c | 104 ++++++++++++++++++++++++++++++++----------
 1 file changed, 80 insertions(+), 24 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] powercap/rapl: handle missing msrs
  2016-05-31 20:41 [PATCH 0/2] Powercap RAPL update for Denverton Jacob Pan
@ 2016-05-31 20:41 ` Jacob Pan
  2016-06-13 18:53   ` jacob
  2016-05-31 20:41 ` [PATCH 2/2] powercap/rapl: add support for denverton Jacob Pan
  1 sibling, 1 reply; 9+ messages in thread
From: Jacob Pan @ 2016-05-31 20:41 UTC (permalink / raw)
  To: LKML, Linux PM, Rafael Wysocki, Srinivas Pandruvada; +Cc: Jacob Pan

Some RAPL MSRs may not exist on some CPUs, we need to continue
the topology detection and enumerate what is available.

This patch handles the missing MSRs then report to powercap
layer only the features available.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/powercap/intel_rapl.c | 103 ++++++++++++++++++++++++++++++++----------
 1 file changed, 79 insertions(+), 24 deletions(-)

diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index b0a2dc4..d51a8d4 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -329,14 +329,14 @@ static int release_zone(struct powercap_zone *power_zone)
 
 static int find_nr_power_limit(struct rapl_domain *rd)
 {
-	int i;
+	int i, nr_pl = 0;
 
 	for (i = 0; i < NR_POWER_LIMITS; i++) {
-		if (rd->rpl[i].name == NULL)
-			break;
+		if (rd->rpl[i].name)
+			nr_pl++;
 	}
 
-	return i;
+	return nr_pl;
 }
 
 static int set_domain_enable(struct powercap_zone *power_zone, bool mode)
@@ -411,15 +411,38 @@ static const struct powercap_zone_ops zone_ops[] = {
 	},
 };
 
-static int set_power_limit(struct powercap_zone *power_zone, int id,
+
+/*
+ * Constraint index used by powercap can be different than power limit (PL)
+ * index in that some  PLs maybe missing due to non-existant MSRs. So we
+ * need to convert here by finding the valid PLs only (name populated).
+ */
+static int contraint_to_pl(struct rapl_domain *rd, int cid)
+{
+	int i, j;
+
+	for (i = 0, j = 0; i < NR_POWER_LIMITS; i++) {
+		if ((rd->rpl[i].name) && j++ == cid) {
+			pr_debug("%s: index %d\n", __func__, i);
+			return i;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int set_power_limit(struct powercap_zone *power_zone, int cid,
 			u64 power_limit)
 {
 	struct rapl_domain *rd;
 	struct rapl_package *rp;
 	int ret = 0;
+	int id;
 
 	get_online_cpus();
 	rd = power_zone_to_rapl_domain(power_zone);
+	id = contraint_to_pl(rd, cid);
+
 	rp = rd->rp;
 
 	if (rd->state & DOMAIN_STATE_BIOS_LOCKED) {
@@ -446,16 +469,18 @@ set_exit:
 	return ret;
 }
 
-static int get_current_power_limit(struct powercap_zone *power_zone, int id,
+static int get_current_power_limit(struct powercap_zone *power_zone, int cid,
 					u64 *data)
 {
 	struct rapl_domain *rd;
 	u64 val;
 	int prim;
 	int ret = 0;
+	int id;
 
 	get_online_cpus();
 	rd = power_zone_to_rapl_domain(power_zone);
+	id = contraint_to_pl(rd, cid);
 	switch (rd->rpl[id].prim_id) {
 	case PL1_ENABLE:
 		prim = POWER_LIMIT1;
@@ -477,14 +502,17 @@ static int get_current_power_limit(struct powercap_zone *power_zone, int id,
 	return ret;
 }
 
-static int set_time_window(struct powercap_zone *power_zone, int id,
+static int set_time_window(struct powercap_zone *power_zone, int cid,
 								u64 window)
 {
 	struct rapl_domain *rd;
 	int ret = 0;
+	int id;
 
 	get_online_cpus();
 	rd = power_zone_to_rapl_domain(power_zone);
+	id = contraint_to_pl(rd, cid);
+
 	switch (rd->rpl[id].prim_id) {
 	case PL1_ENABLE:
 		rapl_write_data_raw(rd, TIME_WINDOW1, window);
@@ -499,14 +527,17 @@ static int set_time_window(struct powercap_zone *power_zone, int id,
 	return ret;
 }
 
-static int get_time_window(struct powercap_zone *power_zone, int id, u64 *data)
+static int get_time_window(struct powercap_zone *power_zone, int cid, u64 *data)
 {
 	struct rapl_domain *rd;
 	u64 val;
 	int ret = 0;
+	int id;
 
 	get_online_cpus();
 	rd = power_zone_to_rapl_domain(power_zone);
+	id = contraint_to_pl(rd, cid);
+
 	switch (rd->rpl[id].prim_id) {
 	case PL1_ENABLE:
 		ret = rapl_read_data_raw(rd, TIME_WINDOW1, true, &val);
@@ -525,15 +556,17 @@ static int get_time_window(struct powercap_zone *power_zone, int id, u64 *data)
 	return ret;
 }
 
-static const char *get_constraint_name(struct powercap_zone *power_zone, int id)
+static const char *get_constraint_name(struct powercap_zone *power_zone, int cid)
 {
-	struct rapl_power_limit *rpl;
 	struct rapl_domain *rd;
+	int id;
 
 	rd = power_zone_to_rapl_domain(power_zone);
-	rpl = (struct rapl_power_limit *) &rd->rpl[id];
+	id = contraint_to_pl(rd, cid);
+	if (id >= 0)
+		return rd->rpl[id].name;
 
-	return rpl->name;
+	return NULL;
 }
 
 
@@ -1305,6 +1338,37 @@ static int rapl_check_domain(int cpu, int domain)
 	return 0;
 }
 
+
+/*
+ * Check if power limits are available. Two cases when they are not available:
+ * 1. Locked by BIOS, in this case we still provide read-only access so that
+ *    users can see what limit is set by the BIOS.
+ * 2. Some CPUs make some domains monitoring only which means PLx MSRs may not
+ *    exist at all. In this case, we do not show the contraints in powercap.
+ *
+ * Called after domains are detected and initialized.
+ */
+static void rapl_detect_powerlimit(struct rapl_domain *rd)
+{
+	u64 val64;
+	int i;
+
+	/* check if the domain is locked by BIOS, ignore if MSR doesn't exist */
+	if (!rapl_read_data_raw(rd, FW_LOCK, false, &val64)) {
+		if (val64) {
+			pr_info("RAPL package %d domain %s locked by BIOS\n",
+				rd->rp->id, rd->name);
+			rd->state |= DOMAIN_STATE_BIOS_LOCKED;
+		}
+	}
+	/* check if power limit MSRs exists, otherwise domain is monitoring only */
+	for (i = 0; i < NR_POWER_LIMITS; i++) {
+		int prim = rd->rpl[i].prim_id;
+		if (rapl_read_data_raw(rd, prim, false, &val64))
+			rd->rpl[i].name = NULL;
+	}
+}
+
 /* Detect active and valid domains for the given CPU, caller must
  * ensure the CPU belongs to the targeted package and CPU hotlug is disabled.
  */
@@ -1313,7 +1377,6 @@ static int rapl_detect_domains(struct rapl_package *rp, int cpu)
 	int i;
 	int ret = 0;
 	struct rapl_domain *rd;
-	u64 locked;
 
 	for (i = 0; i < RAPL_DOMAIN_MAX; i++) {
 		/* use physical package id to read counters */
@@ -1338,17 +1401,9 @@ static int rapl_detect_domains(struct rapl_package *rp, int cpu)
 	}
 	rapl_init_domains(rp);
 
-	for (rd = rp->domains; rd < rp->domains + rp->nr_domains; rd++) {
-		/* check if the domain is locked by BIOS */
-		ret = rapl_read_data_raw(rd, FW_LOCK, false, &locked);
-		if (ret)
-			return ret;
-		if (locked) {
-			pr_info("RAPL package %d domain %s locked by BIOS\n",
-				rp->id, rd->name);
-			rd->state |= DOMAIN_STATE_BIOS_LOCKED;
-		}
-	}
+	for (rd = rp->domains; rd < rp->domains + rp->nr_domains; rd++)
+		rapl_detect_powerlimit(rd);
+
 
 
 done:
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] powercap/rapl: add support for denverton
  2016-05-31 20:41 [PATCH 0/2] Powercap RAPL update for Denverton Jacob Pan
  2016-05-31 20:41 ` [PATCH 1/2] powercap/rapl: handle missing msrs Jacob Pan
@ 2016-05-31 20:41 ` Jacob Pan
  2016-05-31 22:19   ` Dave Hansen
  1 sibling, 1 reply; 9+ messages in thread
From: Jacob Pan @ 2016-05-31 20:41 UTC (permalink / raw)
  To: LKML, Linux PM, Rafael Wysocki, Srinivas Pandruvada; +Cc: Jacob Pan

Denverton micro server is Atom based but RAPL interface
is compatible with Core based CPUs.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/powercap/intel_rapl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index d51a8d4..f0fc1a3 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -1137,6 +1137,7 @@ static const struct x86_cpu_id rapl_ids[] __initconst = {
 	RAPL_CPU(0x57, rapl_defaults_hsw_server),/* Knights Landing */
 	RAPL_CPU(0x8E, rapl_defaults_core),/* Kabylake */
 	RAPL_CPU(0x9E, rapl_defaults_core),/* Kabylake */
+	RAPL_CPU(0x5F, rapl_defaults_core),/* Denverton micro server */
 	{}
 };
 MODULE_DEVICE_TABLE(x86cpu, rapl_ids);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] powercap/rapl: add support for denverton
  2016-05-31 20:41 ` [PATCH 2/2] powercap/rapl: add support for denverton Jacob Pan
@ 2016-05-31 22:19   ` Dave Hansen
  2016-06-01  6:57     ` Thomas Gleixner
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Hansen @ 2016-05-31 22:19 UTC (permalink / raw)
  To: Jacob Pan, LKML, Linux PM, Rafael Wysocki, Srinivas Pandruvada,
	the arch/x86 maintainers

On 05/31/2016 01:41 PM, Jacob Pan wrote:
> --- a/drivers/powercap/intel_rapl.c
> +++ b/drivers/powercap/intel_rapl.c
> @@ -1137,6 +1137,7 @@ static const struct x86_cpu_id rapl_ids[] __initconst = {
>  	RAPL_CPU(0x57, rapl_defaults_hsw_server),/* Knights Landing */
>  	RAPL_CPU(0x8E, rapl_defaults_core),/* Kabylake */
>  	RAPL_CPU(0x9E, rapl_defaults_core),/* Kabylake */
> +	RAPL_CPU(0x5F, rapl_defaults_core),/* Denverton micro server */
>  	{}
>  };

Not to derail this individual patch... but do we really want to continue
open-coding CPU model/family combos all over arch/x86?

For instance, arch/x86/events/intel/core.c has:

>         case 142: /* 14nm Kabylake Mobile */
>         case 158: /* 14nm Kabylake Desktop */
>         case 78: /* 14nm Skylake Mobile */
>         case 94: /* 14nm Skylake Desktop */
>         case 85: /* 14nm Skylake Server */

Which duplicates the two Kabylake family numbers from the RAPL_CPU()
context above (just in decimal instead of hex).

Should we just start sticking these things in a header like:

#define X86_INTEL_FAMILY_KABYLAKE1 	0x8E
#define X86_INTEL_FAMILY_KABYLAKE2	0x9E
#define X86_INTEL_FAMILY_DENVERTON 	0x5F

So we have this:

	RAPL_CPU(X86_INTEL_FAMILY_DENVERTON, rapl_defaults_core),

instead of having to explain our magic number in a comment.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] powercap/rapl: add support for denverton
  2016-05-31 22:19   ` Dave Hansen
@ 2016-06-01  6:57     ` Thomas Gleixner
  2016-06-01 15:08       ` Jacob Pan
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2016-06-01  6:57 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Jacob Pan, LKML, Linux PM, Rafael Wysocki, Srinivas Pandruvada,
	the arch/x86 maintainers

On Tue, 31 May 2016, Dave Hansen wrote:
> On 05/31/2016 01:41 PM, Jacob Pan wrote:
> > --- a/drivers/powercap/intel_rapl.c
> > +++ b/drivers/powercap/intel_rapl.c
> > @@ -1137,6 +1137,7 @@ static const struct x86_cpu_id rapl_ids[] __initconst = {
> >  	RAPL_CPU(0x57, rapl_defaults_hsw_server),/* Knights Landing */
> >  	RAPL_CPU(0x8E, rapl_defaults_core),/* Kabylake */
> >  	RAPL_CPU(0x9E, rapl_defaults_core),/* Kabylake */
> > +	RAPL_CPU(0x5F, rapl_defaults_core),/* Denverton micro server */
> >  	{}
> >  };
> 
> Not to derail this individual patch... but do we really want to continue
> open-coding CPU model/family combos all over arch/x86?
> 
> For instance, arch/x86/events/intel/core.c has:
> 
> >         case 142: /* 14nm Kabylake Mobile */
> >         case 158: /* 14nm Kabylake Desktop */
> >         case 78: /* 14nm Skylake Mobile */
> >         case 94: /* 14nm Skylake Desktop */
> >         case 85: /* 14nm Skylake Server */
> 
> Which duplicates the two Kabylake family numbers from the RAPL_CPU()
> context above (just in decimal instead of hex).
> 
> Should we just start sticking these things in a header like:
> 
> #define X86_INTEL_FAMILY_KABYLAKE1 	0x8E
> #define X86_INTEL_FAMILY_KABYLAKE2	0x9E
> #define X86_INTEL_FAMILY_DENVERTON 	0x5F
> 
> So we have this:
> 
> 	RAPL_CPU(X86_INTEL_FAMILY_DENVERTON, rapl_defaults_core),
> 
> instead of having to explain our magic number in a comment.

Yes please. 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] powercap/rapl: add support for denverton
  2016-06-01  6:57     ` Thomas Gleixner
@ 2016-06-01 15:08       ` Jacob Pan
  0 siblings, 0 replies; 9+ messages in thread
From: Jacob Pan @ 2016-06-01 15:08 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Dave Hansen, LKML, Linux PM, Rafael Wysocki, Srinivas Pandruvada,
	the arch/x86 maintainers, jacob.jun.pan

On Wed, 1 Jun 2016 08:57:27 +0200 (CEST)
Thomas Gleixner <tglx@linutronix.de> wrote:

> On Tue, 31 May 2016, Dave Hansen wrote:
> > On 05/31/2016 01:41 PM, Jacob Pan wrote:
> > > --- a/drivers/powercap/intel_rapl.c
> > > +++ b/drivers/powercap/intel_rapl.c
> > > @@ -1137,6 +1137,7 @@ static const struct x86_cpu_id rapl_ids[]
> > > __initconst = { RAPL_CPU(0x57, rapl_defaults_hsw_server),/*
> > > Knights Landing */ RAPL_CPU(0x8E, rapl_defaults_core),/* Kabylake
> > > */ RAPL_CPU(0x9E, rapl_defaults_core),/* Kabylake */
> > > +	RAPL_CPU(0x5F, rapl_defaults_core),/* Denverton micro
> > > server */ {}
> > >  };
> > 
> > Not to derail this individual patch... but do we really want to
> > continue open-coding CPU model/family combos all over arch/x86?
> > 
> > For instance, arch/x86/events/intel/core.c has:
> > 
> > >         case 142: /* 14nm Kabylake Mobile */
> > >         case 158: /* 14nm Kabylake Desktop */
> > >         case 78: /* 14nm Skylake Mobile */
> > >         case 94: /* 14nm Skylake Desktop */
> > >         case 85: /* 14nm Skylake Server */
> > 
> > Which duplicates the two Kabylake family numbers from the RAPL_CPU()
> > context above (just in decimal instead of hex).
> > 
> > Should we just start sticking these things in a header like:
> > 
> > #define X86_INTEL_FAMILY_KABYLAKE1 	0x8E
> > #define X86_INTEL_FAMILY_KABYLAKE2	0x9E
> > #define X86_INTEL_FAMILY_DENVERTON 	0x5F
> > 
> > So we have this:
> > 
> > 	RAPL_CPU(X86_INTEL_FAMILY_DENVERTON, rapl_defaults_core),
> > 
> > instead of having to explain our magic number in a comment.
> 
> Yes please. 
This open coding also applies to other x86 vendors. I can make change
for Intel since in some case, there is not even a comment about
what the model is. e.g.
in amd_nb.h
   (boot_cpu_data.x86 == 0x15 && boot_cpu_data.x86_model < 0x10))

Should the model numbers be in
arch/x86/include/asm/processor.h?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] powercap/rapl: handle missing msrs
  2016-05-31 20:41 ` [PATCH 1/2] powercap/rapl: handle missing msrs Jacob Pan
@ 2016-06-13 18:53   ` jacob
  2016-06-13 21:00     ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ messages in thread
From: jacob @ 2016-06-13 18:53 UTC (permalink / raw)
  To: LKML, Linux PM, Rafael Wysocki, Srinivas Pandruvada

Hi Rafael,

Any feedback? It that is OK, can you take this patch independent of the
second patch (which is going into tip tree)?
[PATCH 2/2] powercap/rapl: add support for denverton

This patch affects some Broxton/Apollo Lake where the missing MSR will
cause the driver fail to load.

On Tue, 31 May 2016 13:41:29 -0700
Jacob Pan <jacob.jun.pan@linux.intel.com> wrote:

> Some RAPL MSRs may not exist on some CPUs, we need to continue
> the topology detection and enumerate what is available.
> 
> This patch handles the missing MSRs then report to powercap
> layer only the features available.
> 
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
>  drivers/powercap/intel_rapl.c | 103
> ++++++++++++++++++++++++++++++++---------- 1 file changed, 79
> insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/powercap/intel_rapl.c
> b/drivers/powercap/intel_rapl.c index b0a2dc4..d51a8d4 100644
> --- a/drivers/powercap/intel_rapl.c
> +++ b/drivers/powercap/intel_rapl.c
> @@ -329,14 +329,14 @@ static int release_zone(struct powercap_zone
> *power_zone) 
>  static int find_nr_power_limit(struct rapl_domain *rd)
>  {
> -	int i;
> +	int i, nr_pl = 0;
>  
>  	for (i = 0; i < NR_POWER_LIMITS; i++) {
> -		if (rd->rpl[i].name == NULL)
> -			break;
> +		if (rd->rpl[i].name)
> +			nr_pl++;
>  	}
>  
> -	return i;
> +	return nr_pl;
>  }
>  
>  static int set_domain_enable(struct powercap_zone *power_zone, bool
> mode) @@ -411,15 +411,38 @@ static const struct powercap_zone_ops
> zone_ops[] = { },
>  };
>  
> -static int set_power_limit(struct powercap_zone *power_zone, int id,
> +
> +/*
> + * Constraint index used by powercap can be different than power
> limit (PL)
> + * index in that some  PLs maybe missing due to non-existant MSRs.
> So we
> + * need to convert here by finding the valid PLs only (name
> populated).
> + */
> +static int contraint_to_pl(struct rapl_domain *rd, int cid)
> +{
> +	int i, j;
> +
> +	for (i = 0, j = 0; i < NR_POWER_LIMITS; i++) {
> +		if ((rd->rpl[i].name) && j++ == cid) {
> +			pr_debug("%s: index %d\n", __func__, i);
> +			return i;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int set_power_limit(struct powercap_zone *power_zone, int cid,
>  			u64 power_limit)
>  {
>  	struct rapl_domain *rd;
>  	struct rapl_package *rp;
>  	int ret = 0;
> +	int id;
>  
>  	get_online_cpus();
>  	rd = power_zone_to_rapl_domain(power_zone);
> +	id = contraint_to_pl(rd, cid);
> +
>  	rp = rd->rp;
>  
>  	if (rd->state & DOMAIN_STATE_BIOS_LOCKED) {
> @@ -446,16 +469,18 @@ set_exit:
>  	return ret;
>  }
>  
> -static int get_current_power_limit(struct powercap_zone *power_zone,
> int id, +static int get_current_power_limit(struct powercap_zone
> *power_zone, int cid, u64 *data)
>  {
>  	struct rapl_domain *rd;
>  	u64 val;
>  	int prim;
>  	int ret = 0;
> +	int id;
>  
>  	get_online_cpus();
>  	rd = power_zone_to_rapl_domain(power_zone);
> +	id = contraint_to_pl(rd, cid);
>  	switch (rd->rpl[id].prim_id) {
>  	case PL1_ENABLE:
>  		prim = POWER_LIMIT1;
> @@ -477,14 +502,17 @@ static int get_current_power_limit(struct
> powercap_zone *power_zone, int id, return ret;
>  }
>  
> -static int set_time_window(struct powercap_zone *power_zone, int id,
> +static int set_time_window(struct powercap_zone *power_zone, int cid,
>  								u64
> window) {
>  	struct rapl_domain *rd;
>  	int ret = 0;
> +	int id;
>  
>  	get_online_cpus();
>  	rd = power_zone_to_rapl_domain(power_zone);
> +	id = contraint_to_pl(rd, cid);
> +
>  	switch (rd->rpl[id].prim_id) {
>  	case PL1_ENABLE:
>  		rapl_write_data_raw(rd, TIME_WINDOW1, window);
> @@ -499,14 +527,17 @@ static int set_time_window(struct powercap_zone
> *power_zone, int id, return ret;
>  }
>  
> -static int get_time_window(struct powercap_zone *power_zone, int id,
> u64 *data) +static int get_time_window(struct powercap_zone
> *power_zone, int cid, u64 *data) {
>  	struct rapl_domain *rd;
>  	u64 val;
>  	int ret = 0;
> +	int id;
>  
>  	get_online_cpus();
>  	rd = power_zone_to_rapl_domain(power_zone);
> +	id = contraint_to_pl(rd, cid);
> +
>  	switch (rd->rpl[id].prim_id) {
>  	case PL1_ENABLE:
>  		ret = rapl_read_data_raw(rd, TIME_WINDOW1, true,
> &val); @@ -525,15 +556,17 @@ static int get_time_window(struct
> powercap_zone *power_zone, int id, u64 *data) return ret;
>  }
>  
> -static const char *get_constraint_name(struct powercap_zone
> *power_zone, int id) +static const char *get_constraint_name(struct
> powercap_zone *power_zone, int cid) {
> -	struct rapl_power_limit *rpl;
>  	struct rapl_domain *rd;
> +	int id;
>  
>  	rd = power_zone_to_rapl_domain(power_zone);
> -	rpl = (struct rapl_power_limit *) &rd->rpl[id];
> +	id = contraint_to_pl(rd, cid);
> +	if (id >= 0)
> +		return rd->rpl[id].name;
>  
> -	return rpl->name;
> +	return NULL;
>  }
>  
>  
> @@ -1305,6 +1338,37 @@ static int rapl_check_domain(int cpu, int
> domain) return 0;
>  }
>  
> +
> +/*
> + * Check if power limits are available. Two cases when they are not
> available:
> + * 1. Locked by BIOS, in this case we still provide read-only access
> so that
> + *    users can see what limit is set by the BIOS.
> + * 2. Some CPUs make some domains monitoring only which means PLx
> MSRs may not
> + *    exist at all. In this case, we do not show the contraints in
> powercap.
> + *
> + * Called after domains are detected and initialized.
> + */
> +static void rapl_detect_powerlimit(struct rapl_domain *rd)
> +{
> +	u64 val64;
> +	int i;
> +
> +	/* check if the domain is locked by BIOS, ignore if MSR
> doesn't exist */
> +	if (!rapl_read_data_raw(rd, FW_LOCK, false, &val64)) {
> +		if (val64) {
> +			pr_info("RAPL package %d domain %s locked by
> BIOS\n",
> +				rd->rp->id, rd->name);
> +			rd->state |= DOMAIN_STATE_BIOS_LOCKED;
> +		}
> +	}
> +	/* check if power limit MSRs exists, otherwise domain is
> monitoring only */
> +	for (i = 0; i < NR_POWER_LIMITS; i++) {
> +		int prim = rd->rpl[i].prim_id;
> +		if (rapl_read_data_raw(rd, prim, false, &val64))
> +			rd->rpl[i].name = NULL;
> +	}
> +}
> +
>  /* Detect active and valid domains for the given CPU, caller must
>   * ensure the CPU belongs to the targeted package and CPU hotlug is
> disabled. */
> @@ -1313,7 +1377,6 @@ static int rapl_detect_domains(struct
> rapl_package *rp, int cpu) int i;
>  	int ret = 0;
>  	struct rapl_domain *rd;
> -	u64 locked;
>  
>  	for (i = 0; i < RAPL_DOMAIN_MAX; i++) {
>  		/* use physical package id to read counters */
> @@ -1338,17 +1401,9 @@ static int rapl_detect_domains(struct
> rapl_package *rp, int cpu) }
>  	rapl_init_domains(rp);
>  
> -	for (rd = rp->domains; rd < rp->domains + rp->nr_domains;
> rd++) {
> -		/* check if the domain is locked by BIOS */
> -		ret = rapl_read_data_raw(rd, FW_LOCK, false,
> &locked);
> -		if (ret)
> -			return ret;
> -		if (locked) {
> -			pr_info("RAPL package %d domain %s locked by
> BIOS\n",
> -				rp->id, rd->name);
> -			rd->state |= DOMAIN_STATE_BIOS_LOCKED;
> -		}
> -	}
> +	for (rd = rp->domains; rd < rp->domains + rp->nr_domains;
> rd++)
> +		rapl_detect_powerlimit(rd);
> +
>  
>  
>  done:

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] powercap/rapl: handle missing msrs
  2016-06-13 18:53   ` jacob
@ 2016-06-13 21:00     ` Rafael J. Wysocki
  2016-06-16 14:02       ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2016-06-13 21:00 UTC (permalink / raw)
  To: jacob; +Cc: LKML, Linux PM, Rafael Wysocki, Srinivas Pandruvada

On Monday, June 13, 2016 11:53:10 AM jacob wrote:
> Hi Rafael,
> 
> Any feedback? It that is OK, can you take this patch independent of the
> second patch (which is going into tip tree)?

I'll do that.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] powercap/rapl: handle missing msrs
  2016-06-13 21:00     ` Rafael J. Wysocki
@ 2016-06-16 14:02       ` Rafael J. Wysocki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2016-06-16 14:02 UTC (permalink / raw)
  To: jacob; +Cc: LKML, Linux PM, Rafael Wysocki, Srinivas Pandruvada

On Monday, June 13, 2016 11:00:26 PM Rafael J. Wysocki wrote:
> On Monday, June 13, 2016 11:53:10 AM jacob wrote:
> > Hi Rafael,
> > 
> > Any feedback? It that is OK, can you take this patch independent of the
> > second patch (which is going into tip tree)?
> 
> I'll do that.

Applied now, thanks!

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-06-16 13:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-31 20:41 [PATCH 0/2] Powercap RAPL update for Denverton Jacob Pan
2016-05-31 20:41 ` [PATCH 1/2] powercap/rapl: handle missing msrs Jacob Pan
2016-06-13 18:53   ` jacob
2016-06-13 21:00     ` Rafael J. Wysocki
2016-06-16 14:02       ` Rafael J. Wysocki
2016-05-31 20:41 ` [PATCH 2/2] powercap/rapl: add support for denverton Jacob Pan
2016-05-31 22:19   ` Dave Hansen
2016-06-01  6:57     ` Thomas Gleixner
2016-06-01 15:08       ` Jacob Pan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).