linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powercap: add suspend and resume mechanism for SOC power limit
@ 2018-01-09  8:25 xinx.sun
  2018-01-09 11:39 ` Rafael J. Wysocki
  2018-01-10  0:21 ` Rafael J. Wysocki
  0 siblings, 2 replies; 10+ messages in thread
From: xinx.sun @ 2018-01-09  8:25 UTC (permalink / raw)
  To: rjw; +Cc: zhen.han, xinx.sun, chaox.m.wang, linux-pm, linux-kernel

From: Zhen Han <zhen.han@intel.com>

PL1 and PL2 could be throlled or de-throttled by
Thermal management to control SOC temperature.
However, currently, their value will be reset to default value
after once system suspend and resume.
Add pm_notifier to save PL1, PL2 before system suspect and restore
PL1, PL2 after system resume.

Signed-off-by: Sun Xinx <xinx.sun@intel.com>
---
 drivers/powercap/intel_rapl.c | 97 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index d1694f1..0188cff 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -29,6 +29,7 @@
 #include <linux/sysfs.h>
 #include <linux/cpu.h>
 #include <linux/powercap.h>
+#include <linux/suspend.h>
 #include <asm/iosf_mbi.h>
 
 #include <asm/processor.h>
@@ -155,6 +156,7 @@ struct rapl_power_limit {
 	int prim_id; /* primitive ID used to enable */
 	struct rapl_domain *domain;
 	const char *name;
+	u64 last_power_limit;
 };
 
 static const char pl1_name[] = "long_term";
@@ -1533,6 +1535,92 @@ static int rapl_cpu_down_prep(unsigned int cpu)
 
 static enum cpuhp_state pcap_rapl_online;
 
+static void power_limit_state_save(void)
+{
+	struct rapl_package *rp;
+	struct rapl_domain *rd;
+	int nr_pl, ret, i;
+
+	get_online_cpus();
+	list_for_each_entry(rp, &rapl_packages, plist) {
+		if (!rp->power_zone)
+			continue;
+		rd = power_zone_to_rapl_domain(rp->power_zone);
+		nr_pl = find_nr_power_limit(rd);
+		for (i = 0; i < nr_pl; i++) {
+			switch (rd->rpl[i].prim_id) {
+			case PL1_ENABLE:
+				ret = rapl_read_data_raw(rd,
+						POWER_LIMIT1,
+						true,
+						&rd->rpl[i].last_power_limit);
+				if (ret)
+					rd->rpl[i].last_power_limit = 0;
+				break;
+			case PL2_ENABLE:
+				ret = rapl_read_data_raw(rd,
+						POWER_LIMIT2,
+						true,
+						&rd->rpl[i].last_power_limit);
+				if (ret)
+					rd->rpl[i].last_power_limit = 0;
+				break;
+			}
+		}
+	}
+	put_online_cpus();
+}
+
+static void power_limit_state_restore(void)
+{
+	struct rapl_package *rp;
+	struct rapl_domain *rd;
+	int nr_pl, i;
+
+	get_online_cpus();
+	list_for_each_entry(rp, &rapl_packages, plist) {
+		if (!rp->power_zone)
+			continue;
+		rd = power_zone_to_rapl_domain(rp->power_zone);
+		nr_pl = find_nr_power_limit(rd);
+		for (i = 0; i < nr_pl; i++) {
+			switch (rd->rpl[i].prim_id) {
+			case PL1_ENABLE:
+				if (rd->rpl[i].last_power_limit)
+					rapl_write_data_raw(rd,
+						POWER_LIMIT1,
+						rd->rpl[i].last_power_limit);
+				break;
+			case PL2_ENABLE:
+				if (rd->rpl[i].last_power_limit)
+					rapl_write_data_raw(rd,
+						POWER_LIMIT2,
+						rd->rpl[i].last_power_limit);
+				break;
+			}
+		}
+	}
+	put_online_cpus();
+}
+
+static int rapl_pm_callback(struct notifier_block *nb,
+	unsigned long mode, void *_unused)
+{
+	switch (mode) {
+	case PM_SUSPEND_PREPARE:
+		power_limit_state_save();
+		break;
+	case PM_POST_SUSPEND:
+		power_limit_state_restore();
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block rapl_pm_notifier = {
+	.notifier_call = rapl_pm_callback,
+};
+
 static int __init rapl_init(void)
 {
 	const struct x86_cpu_id *id;
@@ -1560,8 +1648,16 @@ static int __init rapl_init(void)
 
 	/* Don't bail out if PSys is not supported */
 	rapl_register_psys();
+
+	ret = register_pm_notifier(&rapl_pm_notifier);
+	if (ret)
+		goto err_unreg_all;
+
 	return 0;
 
+err_unreg_all:
+	cpuhp_remove_state(pcap_rapl_online);
+
 err_unreg:
 	rapl_unregister_powercap();
 	return ret;
@@ -1569,6 +1665,7 @@ static int __init rapl_init(void)
 
 static void __exit rapl_exit(void)
 {
+	unregister_pm_notifier(&rapl_pm_notifier);
 	cpuhp_remove_state(pcap_rapl_online);
 	rapl_unregister_powercap();
 }
-- 
1.9.1

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

* Re: [PATCH] powercap: add suspend and resume mechanism for SOC power limit
  2018-01-09  8:25 [PATCH] powercap: add suspend and resume mechanism for SOC power limit xinx.sun
@ 2018-01-09 11:39 ` Rafael J. Wysocki
  2018-01-09 19:00   ` Srinivas Pandruvada
  2018-01-10  0:21 ` Rafael J. Wysocki
  1 sibling, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2018-01-09 11:39 UTC (permalink / raw)
  To: xinx.sun, Jacob Pan, Srinivas Pandruvada
  Cc: zhen.han, chaox.m.wang, linux-pm, linux-kernel

On Tuesday, January 9, 2018 9:25:10 AM CET xinx.sun@intel.com wrote:
> From: Zhen Han <zhen.han@intel.com>
> 
> PL1 and PL2 could be throlled or de-throttled by
> Thermal management to control SOC temperature.
> However, currently, their value will be reset to default value
> after once system suspend and resume.
> Add pm_notifier to save PL1, PL2 before system suspect and restore
> PL1, PL2 after system resume.
> 
> Signed-off-by: Sun Xinx <xinx.sun@intel.com>

Jacob, Srinivas, please have a look at this.

> ---
>  drivers/powercap/intel_rapl.c | 97 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
> 
> diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
> index d1694f1..0188cff 100644
> --- a/drivers/powercap/intel_rapl.c
> +++ b/drivers/powercap/intel_rapl.c
> @@ -29,6 +29,7 @@
>  #include <linux/sysfs.h>
>  #include <linux/cpu.h>
>  #include <linux/powercap.h>
> +#include <linux/suspend.h>
>  #include <asm/iosf_mbi.h>
>  
>  #include <asm/processor.h>
> @@ -155,6 +156,7 @@ struct rapl_power_limit {
>  	int prim_id; /* primitive ID used to enable */
>  	struct rapl_domain *domain;
>  	const char *name;
> +	u64 last_power_limit;
>  };
>  
>  static const char pl1_name[] = "long_term";
> @@ -1533,6 +1535,92 @@ static int rapl_cpu_down_prep(unsigned int cpu)
>  
>  static enum cpuhp_state pcap_rapl_online;
>  
> +static void power_limit_state_save(void)
> +{
> +	struct rapl_package *rp;
> +	struct rapl_domain *rd;
> +	int nr_pl, ret, i;
> +
> +	get_online_cpus();
> +	list_for_each_entry(rp, &rapl_packages, plist) {
> +		if (!rp->power_zone)
> +			continue;
> +		rd = power_zone_to_rapl_domain(rp->power_zone);
> +		nr_pl = find_nr_power_limit(rd);
> +		for (i = 0; i < nr_pl; i++) {
> +			switch (rd->rpl[i].prim_id) {
> +			case PL1_ENABLE:
> +				ret = rapl_read_data_raw(rd,
> +						POWER_LIMIT1,
> +						true,
> +						&rd->rpl[i].last_power_limit);
> +				if (ret)
> +					rd->rpl[i].last_power_limit = 0;
> +				break;
> +			case PL2_ENABLE:
> +				ret = rapl_read_data_raw(rd,
> +						POWER_LIMIT2,
> +						true,
> +						&rd->rpl[i].last_power_limit);
> +				if (ret)
> +					rd->rpl[i].last_power_limit = 0;
> +				break;
> +			}
> +		}
> +	}
> +	put_online_cpus();
> +}
> +
> +static void power_limit_state_restore(void)
> +{
> +	struct rapl_package *rp;
> +	struct rapl_domain *rd;
> +	int nr_pl, i;
> +
> +	get_online_cpus();
> +	list_for_each_entry(rp, &rapl_packages, plist) {
> +		if (!rp->power_zone)
> +			continue;
> +		rd = power_zone_to_rapl_domain(rp->power_zone);
> +		nr_pl = find_nr_power_limit(rd);
> +		for (i = 0; i < nr_pl; i++) {
> +			switch (rd->rpl[i].prim_id) {
> +			case PL1_ENABLE:
> +				if (rd->rpl[i].last_power_limit)
> +					rapl_write_data_raw(rd,
> +						POWER_LIMIT1,
> +						rd->rpl[i].last_power_limit);
> +				break;
> +			case PL2_ENABLE:
> +				if (rd->rpl[i].last_power_limit)
> +					rapl_write_data_raw(rd,
> +						POWER_LIMIT2,
> +						rd->rpl[i].last_power_limit);
> +				break;
> +			}
> +		}
> +	}
> +	put_online_cpus();
> +}
> +
> +static int rapl_pm_callback(struct notifier_block *nb,
> +	unsigned long mode, void *_unused)
> +{
> +	switch (mode) {
> +	case PM_SUSPEND_PREPARE:
> +		power_limit_state_save();
> +		break;
> +	case PM_POST_SUSPEND:
> +		power_limit_state_restore();
> +		break;
> +	}
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block rapl_pm_notifier = {
> +	.notifier_call = rapl_pm_callback,
> +};
> +
>  static int __init rapl_init(void)
>  {
>  	const struct x86_cpu_id *id;
> @@ -1560,8 +1648,16 @@ static int __init rapl_init(void)
>  
>  	/* Don't bail out if PSys is not supported */
>  	rapl_register_psys();
> +
> +	ret = register_pm_notifier(&rapl_pm_notifier);
> +	if (ret)
> +		goto err_unreg_all;
> +
>  	return 0;
>  
> +err_unreg_all:
> +	cpuhp_remove_state(pcap_rapl_online);
> +
>  err_unreg:
>  	rapl_unregister_powercap();
>  	return ret;
> @@ -1569,6 +1665,7 @@ static int __init rapl_init(void)
>  
>  static void __exit rapl_exit(void)
>  {
> +	unregister_pm_notifier(&rapl_pm_notifier);
>  	cpuhp_remove_state(pcap_rapl_online);
>  	rapl_unregister_powercap();
>  }
> 

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

* Re: [PATCH] powercap: add suspend and resume mechanism for SOC power limit
  2018-01-09 11:39 ` Rafael J. Wysocki
@ 2018-01-09 19:00   ` Srinivas Pandruvada
  0 siblings, 0 replies; 10+ messages in thread
From: Srinivas Pandruvada @ 2018-01-09 19:00 UTC (permalink / raw)
  To: Rafael J. Wysocki, xinx.sun, Jacob Pan
  Cc: zhen.han, chaox.m.wang, linux-pm, linux-kernel

On Tue, 2018-01-09 at 12:39 +0100, Rafael J. Wysocki wrote:
> On Tuesday, January 9, 2018 9:25:10 AM CET xinx.sun@intel.com wrote:
> > From: Zhen Han <zhen.han@intel.com>
> > 
> > PL1 and PL2 could be throlled or de-throttled by
> > Thermal management to control SOC temperature.
> > However, currently, their value will be reset to default value
> > after once system suspend and resume.
> > Add pm_notifier to save PL1, PL2 before system suspect and restore
> > PL1, PL2 after system resume.
> > 
> > Signed-off-by: Sun Xinx <xinx.sun@intel.com>
> 
> Jacob, Srinivas, please have a look at this.
I have already looked at this. Looks good to me.

Thanks,
Srinivas

> 
> > ---
> >  drivers/powercap/intel_rapl.c | 97
> > +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 97 insertions(+)
> > 
> > diff --git a/drivers/powercap/intel_rapl.c
> > b/drivers/powercap/intel_rapl.c
> > index d1694f1..0188cff 100644
> > --- a/drivers/powercap/intel_rapl.c
> > +++ b/drivers/powercap/intel_rapl.c
> > @@ -29,6 +29,7 @@
> >  #include <linux/sysfs.h>
> >  #include <linux/cpu.h>
> >  #include <linux/powercap.h>
> > +#include <linux/suspend.h>
> >  #include <asm/iosf_mbi.h>
> >  
> >  #include <asm/processor.h>
> > @@ -155,6 +156,7 @@ struct rapl_power_limit {
> >  	int prim_id; /* primitive ID used to enable */
> >  	struct rapl_domain *domain;
> >  	const char *name;
> > +	u64 last_power_limit;
> >  };
> >  
> >  static const char pl1_name[] = "long_term";
> > @@ -1533,6 +1535,92 @@ static int rapl_cpu_down_prep(unsigned int
> > cpu)
> >  
> >  static enum cpuhp_state pcap_rapl_online;
> >  
> > +static void power_limit_state_save(void)
> > +{
> > +	struct rapl_package *rp;
> > +	struct rapl_domain *rd;
> > +	int nr_pl, ret, i;
> > +
> > +	get_online_cpus();
> > +	list_for_each_entry(rp, &rapl_packages, plist) {
> > +		if (!rp->power_zone)
> > +			continue;
> > +		rd = power_zone_to_rapl_domain(rp->power_zone);
> > +		nr_pl = find_nr_power_limit(rd);
> > +		for (i = 0; i < nr_pl; i++) {
> > +			switch (rd->rpl[i].prim_id) {
> > +			case PL1_ENABLE:
> > +				ret = rapl_read_data_raw(rd,
> > +						POWER_LIMIT1,
> > +						true,
> > +						&rd-
> > >rpl[i].last_power_limit);
> > +				if (ret)
> > +					rd-
> > >rpl[i].last_power_limit = 0;
> > +				break;
> > +			case PL2_ENABLE:
> > +				ret = rapl_read_data_raw(rd,
> > +						POWER_LIMIT2,
> > +						true,
> > +						&rd-
> > >rpl[i].last_power_limit);
> > +				if (ret)
> > +					rd-
> > >rpl[i].last_power_limit = 0;
> > +				break;
> > +			}
> > +		}
> > +	}
> > +	put_online_cpus();
> > +}
> > +
> > +static void power_limit_state_restore(void)
> > +{
> > +	struct rapl_package *rp;
> > +	struct rapl_domain *rd;
> > +	int nr_pl, i;
> > +
> > +	get_online_cpus();
> > +	list_for_each_entry(rp, &rapl_packages, plist) {
> > +		if (!rp->power_zone)
> > +			continue;
> > +		rd = power_zone_to_rapl_domain(rp->power_zone);
> > +		nr_pl = find_nr_power_limit(rd);
> > +		for (i = 0; i < nr_pl; i++) {
> > +			switch (rd->rpl[i].prim_id) {
> > +			case PL1_ENABLE:
> > +				if (rd->rpl[i].last_power_limit)
> > +					rapl_write_data_raw(rd,
> > +						POWER_LIMIT1,
> > +						rd-
> > >rpl[i].last_power_limit);
> > +				break;
> > +			case PL2_ENABLE:
> > +				if (rd->rpl[i].last_power_limit)
> > +					rapl_write_data_raw(rd,
> > +						POWER_LIMIT2,
> > +						rd-
> > >rpl[i].last_power_limit);
> > +				break;
> > +			}
> > +		}
> > +	}
> > +	put_online_cpus();
> > +}
> > +
> > +static int rapl_pm_callback(struct notifier_block *nb,
> > +	unsigned long mode, void *_unused)
> > +{
> > +	switch (mode) {
> > +	case PM_SUSPEND_PREPARE:
> > +		power_limit_state_save();
> > +		break;
> > +	case PM_POST_SUSPEND:
> > +		power_limit_state_restore();
> > +		break;
> > +	}
> > +	return NOTIFY_OK;
> > +}
> > +
> > +static struct notifier_block rapl_pm_notifier = {
> > +	.notifier_call = rapl_pm_callback,
> > +};
> > +
> >  static int __init rapl_init(void)
> >  {
> >  	const struct x86_cpu_id *id;
> > @@ -1560,8 +1648,16 @@ static int __init rapl_init(void)
> >  
> >  	/* Don't bail out if PSys is not supported */
> >  	rapl_register_psys();
> > +
> > +	ret = register_pm_notifier(&rapl_pm_notifier);
> > +	if (ret)
> > +		goto err_unreg_all;
> > +
> >  	return 0;
> >  
> > +err_unreg_all:
> > +	cpuhp_remove_state(pcap_rapl_online);
> > +
> >  err_unreg:
> >  	rapl_unregister_powercap();
> >  	return ret;
> > @@ -1569,6 +1665,7 @@ static int __init rapl_init(void)
> >  
> >  static void __exit rapl_exit(void)
> >  {
> > +	unregister_pm_notifier(&rapl_pm_notifier);
> >  	cpuhp_remove_state(pcap_rapl_online);
> >  	rapl_unregister_powercap();
> >  }
> > 
> 
> 

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

* Re: [PATCH] powercap: add suspend and resume mechanism for SOC power limit
  2018-01-09  8:25 [PATCH] powercap: add suspend and resume mechanism for SOC power limit xinx.sun
  2018-01-09 11:39 ` Rafael J. Wysocki
@ 2018-01-10  0:21 ` Rafael J. Wysocki
  2018-01-10  0:23   ` Sun, XinX
  1 sibling, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2018-01-10  0:21 UTC (permalink / raw)
  To: xinx.sun
  Cc: Rafael J. Wysocki, zhen.han, chaox.m.wang, Linux PM,
	Linux Kernel Mailing List

On Tue, Jan 9, 2018 at 9:25 AM,  <xinx.sun@intel.com> wrote:
> From: Zhen Han <zhen.han@intel.com>
>
> PL1 and PL2 could be throlled or de-throttled by
> Thermal management to control SOC temperature.
> However, currently, their value will be reset to default value
> after once system suspend and resume.
> Add pm_notifier to save PL1, PL2 before system suspect and restore
> PL1, PL2 after system resume.
>
> Signed-off-by: Sun Xinx <xinx.sun@intel.com>

There is a formal issue here.

We need a Signed-off-by: tag from Zhen Han too.

Thanks,
Rafael

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

* RE: [PATCH] powercap: add suspend and resume mechanism for SOC power limit
  2018-01-10  0:21 ` Rafael J. Wysocki
@ 2018-01-10  0:23   ` Sun, XinX
  0 siblings, 0 replies; 10+ messages in thread
From: Sun, XinX @ 2018-01-10  0:23 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Han, Zhen, Wang, ChaoX M, Linux PM,
	Linux Kernel Mailing List

Ok, got it.

-----Original Message-----
From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of Rafael J. Wysocki
Sent: Wednesday, January 10, 2018 8:21 AM
To: Sun, XinX <xinx.sun@intel.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>; Han, Zhen <zhen.han@intel.com>; Wang, ChaoX M <chaox.m.wang@intel.com>; Linux PM <linux-pm@vger.kernel.org>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] powercap: add suspend and resume mechanism for SOC power limit

On Tue, Jan 9, 2018 at 9:25 AM,  <xinx.sun@intel.com> wrote:
> From: Zhen Han <zhen.han@intel.com>
>
> PL1 and PL2 could be throlled or de-throttled by Thermal management to 
> control SOC temperature.
> However, currently, their value will be reset to default value after 
> once system suspend and resume.
> Add pm_notifier to save PL1, PL2 before system suspect and restore 
> PL1, PL2 after system resume.
>
> Signed-off-by: Sun Xinx <xinx.sun@intel.com>

There is a formal issue here.

We need a Signed-off-by: tag from Zhen Han too.

Thanks,
Rafael

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

* RE: [PATCH] powercap: add suspend and resume mechanism for SOC power limit
  2018-01-10  6:21     ` Srinivas Pandruvada
@ 2018-01-10  7:00       ` Sun, XinX
  0 siblings, 0 replies; 10+ messages in thread
From: Sun, XinX @ 2018-01-10  7:00 UTC (permalink / raw)
  To: Srinivas Pandruvada, rjw, jacob.jun.pan
  Cc: Han, Zhen, Wang, ChaoX M, linux-pm, linux-kernel

Thanks for your suggestion.
I will pay attention about this.

BR,
Sun Xin

-----Original Message-----
From: Srinivas Pandruvada [mailto:srinivas.pandruvada@linux.intel.com] 
Sent: Wednesday, January 10, 2018 2:21 PM
To: Sun, XinX <xinx.sun@intel.com>; rjw@rjwysocki.net; jacob.jun.pan@linux.intel.com
Cc: Han, Zhen <zhen.han@intel.com>; Wang, ChaoX M <chaox.m.wang@intel.com>; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] powercap: add suspend and resume mechanism for SOC power limit

On Wed, 2018-01-10 at 01:53 +0000, Sun, XinX wrote:
> According to Rafael's comment.
> > 
> > There is a formal issue here.
> > We need a Signed-off-by: tag from Zhen Han too.
> I changed Signed-off in commit message.
> 
This is not the correct procedure to do such things after sending the patch.
Basically you can change prefix to "[PATCH]" to "[Update][PATCH]".

Then after "---" add change history.

E.g.

Signed-off-by: Zhen Han <zhen.han@intel.com>> > ---
---
Change: 
	Fixed the signed-off tag.




Thanks,
Srinivas

> Thanks & BR,
> Sun Xin
> 
> -----Original Message-----
> From: Srinivas Pandruvada [mailto:srinivas.pandruvada@linux.intel.com
> ]
> Sent: Wednesday, January 10, 2018 9:47 AM
> To: Sun, XinX <xinx.sun@intel.com>; rjw@rjwysocki.net; jacob.jun.pan@ 
> linux.intel.com
> Cc: Han, Zhen <zhen.han@intel.com>; Wang, ChaoX M <chaox.m.wang@intel 
> .com>; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] powercap: add suspend and resume mechanism for 
> SOC power limit
> 
> On Wed, 2018-01-10 at 08:38 +0800, xinx.sun@intel.com wrote:
> > 
> > From: Zhen Han <zhen.han@intel.com>
> > 
> > PL1 and PL2 could be throlled or de-throttled by Thermal management 
> > to control SOC temperature.
> > However, currently, their value will be reset to default value after 
> > once system suspend and resume.
> > Add pm_notifier to save PL1, PL2 before system suspect and restore 
> > PL1, PL2 after system resume.
> > 
> Why are you posting this patch again?
> If there any change from your prior post?
> 
> Thanks,
> Srinivas
> 
> > 
> > Signed-off-by: Zhen Han <zhen.han@intel.com>
> > ---
> >  drivers/powercap/intel_rapl.c | 97
> > +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 97 insertions(+)
> > 
> > diff --git a/drivers/powercap/intel_rapl.c 
> > b/drivers/powercap/intel_rapl.c index d1694f1..0188cff 100644
> > --- a/drivers/powercap/intel_rapl.c
> > +++ b/drivers/powercap/intel_rapl.c
> > @@ -29,6 +29,7 @@
> >  #include <linux/sysfs.h>
> >  #include <linux/cpu.h>
> >  #include <linux/powercap.h>
> > +#include <linux/suspend.h>
> >  #include <asm/iosf_mbi.h>
> >  
> >  #include <asm/processor.h>
> > @@ -155,6 +156,7 @@ struct rapl_power_limit {
> >  	int prim_id; /* primitive ID used to enable */
> >  	struct rapl_domain *domain;
> >  	const char *name;
> > +	u64 last_power_limit;
> >  };
> >  
> >  static const char pl1_name[] = "long_term"; @@ -1533,6 +1535,92 @@ 
> > static int rapl_cpu_down_prep(unsigned int
> > cpu)
> >  
> >  static enum cpuhp_state pcap_rapl_online;
> >  
> > +static void power_limit_state_save(void) {
> > +	struct rapl_package *rp;
> > +	struct rapl_domain *rd;
> > +	int nr_pl, ret, i;
> > +
> > +	get_online_cpus();
> > +	list_for_each_entry(rp, &rapl_packages, plist) {
> > +		if (!rp->power_zone)
> > +			continue;
> > +		rd = power_zone_to_rapl_domain(rp->power_zone);
> > +		nr_pl = find_nr_power_limit(rd);
> > +		for (i = 0; i < nr_pl; i++) {
> > +			switch (rd->rpl[i].prim_id) {
> > +			case PL1_ENABLE:
> > +				ret = rapl_read_data_raw(rd,
> > +						POWER_LIMIT1,
> > +						true,
> > +						&rd-
> > > 
> > > rpl[i].last_power_limit);
> > +				if (ret)
> > +					rd-
> > >rpl[i].last_power_limit
> > = 0;
> > +				break;
> > +			case PL2_ENABLE:
> > +				ret = rapl_read_data_raw(rd,
> > +						POWER_LIMIT2,
> > +						true,
> > +						&rd-
> > > 
> > > rpl[i].last_power_limit);
> > +				if (ret)
> > +					rd-
> > >rpl[i].last_power_limit
> > = 0;
> > +				break;
> > +			}
> > +		}
> > +	}
> > +	put_online_cpus();
> > +}
> > +
> > +static void power_limit_state_restore(void) {
> > +	struct rapl_package *rp;
> > +	struct rapl_domain *rd;
> > +	int nr_pl, i;
> > +
> > +	get_online_cpus();
> > +	list_for_each_entry(rp, &rapl_packages, plist) {
> > +		if (!rp->power_zone)
> > +			continue;
> > +		rd = power_zone_to_rapl_domain(rp->power_zone);
> > +		nr_pl = find_nr_power_limit(rd);
> > +		for (i = 0; i < nr_pl; i++) {
> > +			switch (rd->rpl[i].prim_id) {
> > +			case PL1_ENABLE:
> > +				if (rd->rpl[i].last_power_limit)
> > +					rapl_write_data_raw(rd,
> > +						POWER_LIMIT1,
> > +						rd-
> > > 
> > > rpl[i].last_power_limit);
> > +				break;
> > +			case PL2_ENABLE:
> > +				if (rd->rpl[i].last_power_limit)
> > +					rapl_write_data_raw(rd,
> > +						POWER_LIMIT2,
> > +						rd-
> > > 
> > > rpl[i].last_power_limit);
> > +				break;
> > +			}
> > +		}
> > +	}
> > +	put_online_cpus();
> > +}
> > +
> > +static int rapl_pm_callback(struct notifier_block *nb,
> > +	unsigned long mode, void *_unused) {
> > +	switch (mode) {
> > +	case PM_SUSPEND_PREPARE:
> > +		power_limit_state_save();
> > +		break;
> > +	case PM_POST_SUSPEND:
> > +		power_limit_state_restore();
> > +		break;
> > +	}
> > +	return NOTIFY_OK;
> > +}
> > +
> > +static struct notifier_block rapl_pm_notifier = {
> > +	.notifier_call = rapl_pm_callback, };
> > +
> >  static int __init rapl_init(void)
> >  {
> >  	const struct x86_cpu_id *id;
> > @@ -1560,8 +1648,16 @@ static int __init rapl_init(void)
> >  
> >  	/* Don't bail out if PSys is not supported */
> >  	rapl_register_psys();
> > +
> > +	ret = register_pm_notifier(&rapl_pm_notifier);
> > +	if (ret)
> > +		goto err_unreg_all;
> > +
> >  	return 0;
> >  
> > +err_unreg_all:
> > +	cpuhp_remove_state(pcap_rapl_online);
> > +
> >  err_unreg:
> >  	rapl_unregister_powercap();
> >  	return ret;
> > @@ -1569,6 +1665,7 @@ static int __init rapl_init(void)
> >  
> >  static void __exit rapl_exit(void)
> >  {
> > +	unregister_pm_notifier(&rapl_pm_notifier);
> >  	cpuhp_remove_state(pcap_rapl_online);
> >  	rapl_unregister_powercap();
> >  }

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

* Re: [PATCH] powercap: add suspend and resume mechanism for SOC power limit
  2018-01-10  1:53   ` Sun, XinX
@ 2018-01-10  6:21     ` Srinivas Pandruvada
  2018-01-10  7:00       ` Sun, XinX
  0 siblings, 1 reply; 10+ messages in thread
From: Srinivas Pandruvada @ 2018-01-10  6:21 UTC (permalink / raw)
  To: Sun, XinX, rjw, jacob.jun.pan
  Cc: Han, Zhen, Wang, ChaoX M, linux-pm, linux-kernel

On Wed, 2018-01-10 at 01:53 +0000, Sun, XinX wrote:
> According to Rafael's comment.
> > 
> > There is a formal issue here.
> > We need a Signed-off-by: tag from Zhen Han too.
> I changed Signed-off in commit message.
> 
This is not the correct procedure to do such things after sending the
patch.
Basically you can change prefix to "[PATCH]" to "[Update][PATCH]".

Then after "---" add change history.

E.g.

Signed-off-by: Zhen Han <zhen.han@intel.com>> > ---
---
Change: 
	Fixed the signed-off tag.




Thanks,
Srinivas

> Thanks & BR,
> Sun Xin
> 
> -----Original Message-----
> From: Srinivas Pandruvada [mailto:srinivas.pandruvada@linux.intel.com
> ] 
> Sent: Wednesday, January 10, 2018 9:47 AM
> To: Sun, XinX <xinx.sun@intel.com>; rjw@rjwysocki.net; jacob.jun.pan@
> linux.intel.com
> Cc: Han, Zhen <zhen.han@intel.com>; Wang, ChaoX M <chaox.m.wang@intel
> .com>; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] powercap: add suspend and resume mechanism for
> SOC power limit
> 
> On Wed, 2018-01-10 at 08:38 +0800, xinx.sun@intel.com wrote:
> > 
> > From: Zhen Han <zhen.han@intel.com>
> > 
> > PL1 and PL2 could be throlled or de-throttled by Thermal management
> > to 
> > control SOC temperature.
> > However, currently, their value will be reset to default value
> > after 
> > once system suspend and resume.
> > Add pm_notifier to save PL1, PL2 before system suspect and restore 
> > PL1, PL2 after system resume.
> > 
> Why are you posting this patch again?
> If there any change from your prior post?
> 
> Thanks,
> Srinivas
> 
> > 
> > Signed-off-by: Zhen Han <zhen.han@intel.com>
> > ---
> >  drivers/powercap/intel_rapl.c | 97
> > +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 97 insertions(+)
> > 
> > diff --git a/drivers/powercap/intel_rapl.c 
> > b/drivers/powercap/intel_rapl.c index d1694f1..0188cff 100644
> > --- a/drivers/powercap/intel_rapl.c
> > +++ b/drivers/powercap/intel_rapl.c
> > @@ -29,6 +29,7 @@
> >  #include <linux/sysfs.h>
> >  #include <linux/cpu.h>
> >  #include <linux/powercap.h>
> > +#include <linux/suspend.h>
> >  #include <asm/iosf_mbi.h>
> >  
> >  #include <asm/processor.h>
> > @@ -155,6 +156,7 @@ struct rapl_power_limit {
> >  	int prim_id; /* primitive ID used to enable */
> >  	struct rapl_domain *domain;
> >  	const char *name;
> > +	u64 last_power_limit;
> >  };
> >  
> >  static const char pl1_name[] = "long_term"; @@ -1533,6 +1535,92
> > @@ 
> > static int rapl_cpu_down_prep(unsigned int
> > cpu)
> >  
> >  static enum cpuhp_state pcap_rapl_online;
> >  
> > +static void power_limit_state_save(void) {
> > +	struct rapl_package *rp;
> > +	struct rapl_domain *rd;
> > +	int nr_pl, ret, i;
> > +
> > +	get_online_cpus();
> > +	list_for_each_entry(rp, &rapl_packages, plist) {
> > +		if (!rp->power_zone)
> > +			continue;
> > +		rd = power_zone_to_rapl_domain(rp->power_zone);
> > +		nr_pl = find_nr_power_limit(rd);
> > +		for (i = 0; i < nr_pl; i++) {
> > +			switch (rd->rpl[i].prim_id) {
> > +			case PL1_ENABLE:
> > +				ret = rapl_read_data_raw(rd,
> > +						POWER_LIMIT1,
> > +						true,
> > +						&rd-
> > > 
> > > rpl[i].last_power_limit);
> > +				if (ret)
> > +					rd-
> > >rpl[i].last_power_limit
> > = 0;
> > +				break;
> > +			case PL2_ENABLE:
> > +				ret = rapl_read_data_raw(rd,
> > +						POWER_LIMIT2,
> > +						true,
> > +						&rd-
> > > 
> > > rpl[i].last_power_limit);
> > +				if (ret)
> > +					rd-
> > >rpl[i].last_power_limit
> > = 0;
> > +				break;
> > +			}
> > +		}
> > +	}
> > +	put_online_cpus();
> > +}
> > +
> > +static void power_limit_state_restore(void) {
> > +	struct rapl_package *rp;
> > +	struct rapl_domain *rd;
> > +	int nr_pl, i;
> > +
> > +	get_online_cpus();
> > +	list_for_each_entry(rp, &rapl_packages, plist) {
> > +		if (!rp->power_zone)
> > +			continue;
> > +		rd = power_zone_to_rapl_domain(rp->power_zone);
> > +		nr_pl = find_nr_power_limit(rd);
> > +		for (i = 0; i < nr_pl; i++) {
> > +			switch (rd->rpl[i].prim_id) {
> > +			case PL1_ENABLE:
> > +				if (rd->rpl[i].last_power_limit)
> > +					rapl_write_data_raw(rd,
> > +						POWER_LIMIT1,
> > +						rd-
> > > 
> > > rpl[i].last_power_limit);
> > +				break;
> > +			case PL2_ENABLE:
> > +				if (rd->rpl[i].last_power_limit)
> > +					rapl_write_data_raw(rd,
> > +						POWER_LIMIT2,
> > +						rd-
> > > 
> > > rpl[i].last_power_limit);
> > +				break;
> > +			}
> > +		}
> > +	}
> > +	put_online_cpus();
> > +}
> > +
> > +static int rapl_pm_callback(struct notifier_block *nb,
> > +	unsigned long mode, void *_unused)
> > +{
> > +	switch (mode) {
> > +	case PM_SUSPEND_PREPARE:
> > +		power_limit_state_save();
> > +		break;
> > +	case PM_POST_SUSPEND:
> > +		power_limit_state_restore();
> > +		break;
> > +	}
> > +	return NOTIFY_OK;
> > +}
> > +
> > +static struct notifier_block rapl_pm_notifier = {
> > +	.notifier_call = rapl_pm_callback,
> > +};
> > +
> >  static int __init rapl_init(void)
> >  {
> >  	const struct x86_cpu_id *id;
> > @@ -1560,8 +1648,16 @@ static int __init rapl_init(void)
> >  
> >  	/* Don't bail out if PSys is not supported */
> >  	rapl_register_psys();
> > +
> > +	ret = register_pm_notifier(&rapl_pm_notifier);
> > +	if (ret)
> > +		goto err_unreg_all;
> > +
> >  	return 0;
> >  
> > +err_unreg_all:
> > +	cpuhp_remove_state(pcap_rapl_online);
> > +
> >  err_unreg:
> >  	rapl_unregister_powercap();
> >  	return ret;
> > @@ -1569,6 +1665,7 @@ static int __init rapl_init(void)
> >  
> >  static void __exit rapl_exit(void)
> >  {
> > +	unregister_pm_notifier(&rapl_pm_notifier);
> >  	cpuhp_remove_state(pcap_rapl_online);
> >  	rapl_unregister_powercap();
> >  }

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

* RE: [PATCH] powercap: add suspend and resume mechanism for SOC power limit
  2018-01-10  1:47 ` Srinivas Pandruvada
@ 2018-01-10  1:53   ` Sun, XinX
  2018-01-10  6:21     ` Srinivas Pandruvada
  0 siblings, 1 reply; 10+ messages in thread
From: Sun, XinX @ 2018-01-10  1:53 UTC (permalink / raw)
  To: Srinivas Pandruvada, rjw, jacob.jun.pan
  Cc: Han, Zhen, Wang, ChaoX M, linux-pm, linux-kernel

According to Rafael's comment.
>There is a formal issue here.
>We need a Signed-off-by: tag from Zhen Han too.

I changed Signed-off in commit message.

Thanks & BR,
Sun Xin

-----Original Message-----
From: Srinivas Pandruvada [mailto:srinivas.pandruvada@linux.intel.com] 
Sent: Wednesday, January 10, 2018 9:47 AM
To: Sun, XinX <xinx.sun@intel.com>; rjw@rjwysocki.net; jacob.jun.pan@linux.intel.com
Cc: Han, Zhen <zhen.han@intel.com>; Wang, ChaoX M <chaox.m.wang@intel.com>; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] powercap: add suspend and resume mechanism for SOC power limit

On Wed, 2018-01-10 at 08:38 +0800, xinx.sun@intel.com wrote:
> From: Zhen Han <zhen.han@intel.com>
> 
> PL1 and PL2 could be throlled or de-throttled by Thermal management to 
> control SOC temperature.
> However, currently, their value will be reset to default value after 
> once system suspend and resume.
> Add pm_notifier to save PL1, PL2 before system suspect and restore 
> PL1, PL2 after system resume.
> 
Why are you posting this patch again?
If there any change from your prior post?

Thanks,
Srinivas

> Signed-off-by: Zhen Han <zhen.han@intel.com>
> ---
>  drivers/powercap/intel_rapl.c | 97
> +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
> 
> diff --git a/drivers/powercap/intel_rapl.c 
> b/drivers/powercap/intel_rapl.c index d1694f1..0188cff 100644
> --- a/drivers/powercap/intel_rapl.c
> +++ b/drivers/powercap/intel_rapl.c
> @@ -29,6 +29,7 @@
>  #include <linux/sysfs.h>
>  #include <linux/cpu.h>
>  #include <linux/powercap.h>
> +#include <linux/suspend.h>
>  #include <asm/iosf_mbi.h>
>  
>  #include <asm/processor.h>
> @@ -155,6 +156,7 @@ struct rapl_power_limit {
>  	int prim_id; /* primitive ID used to enable */
>  	struct rapl_domain *domain;
>  	const char *name;
> +	u64 last_power_limit;
>  };
>  
>  static const char pl1_name[] = "long_term"; @@ -1533,6 +1535,92 @@ 
> static int rapl_cpu_down_prep(unsigned int
> cpu)
>  
>  static enum cpuhp_state pcap_rapl_online;
>  
> +static void power_limit_state_save(void) {
> +	struct rapl_package *rp;
> +	struct rapl_domain *rd;
> +	int nr_pl, ret, i;
> +
> +	get_online_cpus();
> +	list_for_each_entry(rp, &rapl_packages, plist) {
> +		if (!rp->power_zone)
> +			continue;
> +		rd = power_zone_to_rapl_domain(rp->power_zone);
> +		nr_pl = find_nr_power_limit(rd);
> +		for (i = 0; i < nr_pl; i++) {
> +			switch (rd->rpl[i].prim_id) {
> +			case PL1_ENABLE:
> +				ret = rapl_read_data_raw(rd,
> +						POWER_LIMIT1,
> +						true,
> +						&rd-
> >rpl[i].last_power_limit);
> +				if (ret)
> +					rd->rpl[i].last_power_limit
> = 0;
> +				break;
> +			case PL2_ENABLE:
> +				ret = rapl_read_data_raw(rd,
> +						POWER_LIMIT2,
> +						true,
> +						&rd-
> >rpl[i].last_power_limit);
> +				if (ret)
> +					rd->rpl[i].last_power_limit
> = 0;
> +				break;
> +			}
> +		}
> +	}
> +	put_online_cpus();
> +}
> +
> +static void power_limit_state_restore(void) {
> +	struct rapl_package *rp;
> +	struct rapl_domain *rd;
> +	int nr_pl, i;
> +
> +	get_online_cpus();
> +	list_for_each_entry(rp, &rapl_packages, plist) {
> +		if (!rp->power_zone)
> +			continue;
> +		rd = power_zone_to_rapl_domain(rp->power_zone);
> +		nr_pl = find_nr_power_limit(rd);
> +		for (i = 0; i < nr_pl; i++) {
> +			switch (rd->rpl[i].prim_id) {
> +			case PL1_ENABLE:
> +				if (rd->rpl[i].last_power_limit)
> +					rapl_write_data_raw(rd,
> +						POWER_LIMIT1,
> +						rd-
> >rpl[i].last_power_limit);
> +				break;
> +			case PL2_ENABLE:
> +				if (rd->rpl[i].last_power_limit)
> +					rapl_write_data_raw(rd,
> +						POWER_LIMIT2,
> +						rd-
> >rpl[i].last_power_limit);
> +				break;
> +			}
> +		}
> +	}
> +	put_online_cpus();
> +}
> +
> +static int rapl_pm_callback(struct notifier_block *nb,
> +	unsigned long mode, void *_unused)
> +{
> +	switch (mode) {
> +	case PM_SUSPEND_PREPARE:
> +		power_limit_state_save();
> +		break;
> +	case PM_POST_SUSPEND:
> +		power_limit_state_restore();
> +		break;
> +	}
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block rapl_pm_notifier = {
> +	.notifier_call = rapl_pm_callback,
> +};
> +
>  static int __init rapl_init(void)
>  {
>  	const struct x86_cpu_id *id;
> @@ -1560,8 +1648,16 @@ static int __init rapl_init(void)
>  
>  	/* Don't bail out if PSys is not supported */
>  	rapl_register_psys();
> +
> +	ret = register_pm_notifier(&rapl_pm_notifier);
> +	if (ret)
> +		goto err_unreg_all;
> +
>  	return 0;
>  
> +err_unreg_all:
> +	cpuhp_remove_state(pcap_rapl_online);
> +
>  err_unreg:
>  	rapl_unregister_powercap();
>  	return ret;
> @@ -1569,6 +1665,7 @@ static int __init rapl_init(void)
>  
>  static void __exit rapl_exit(void)
>  {
> +	unregister_pm_notifier(&rapl_pm_notifier);
>  	cpuhp_remove_state(pcap_rapl_online);
>  	rapl_unregister_powercap();
>  }

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

* Re: [PATCH] powercap: add suspend and resume mechanism for SOC power limit
  2018-01-10  0:38 xinx.sun
@ 2018-01-10  1:47 ` Srinivas Pandruvada
  2018-01-10  1:53   ` Sun, XinX
  0 siblings, 1 reply; 10+ messages in thread
From: Srinivas Pandruvada @ 2018-01-10  1:47 UTC (permalink / raw)
  To: xinx.sun, rjw, jacob.jun.pan
  Cc: zhen.han, chaox.m.wang, linux-pm, linux-kernel

On Wed, 2018-01-10 at 08:38 +0800, xinx.sun@intel.com wrote:
> From: Zhen Han <zhen.han@intel.com>
> 
> PL1 and PL2 could be throlled or de-throttled by
> Thermal management to control SOC temperature.
> However, currently, their value will be reset to default value
> after once system suspend and resume.
> Add pm_notifier to save PL1, PL2 before system suspect and restore
> PL1, PL2 after system resume.
> 
Why are you posting this patch again?
If there any change from your prior post?

Thanks,
Srinivas

> Signed-off-by: Zhen Han <zhen.han@intel.com>
> ---
>  drivers/powercap/intel_rapl.c | 97
> +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
> 
> diff --git a/drivers/powercap/intel_rapl.c
> b/drivers/powercap/intel_rapl.c
> index d1694f1..0188cff 100644
> --- a/drivers/powercap/intel_rapl.c
> +++ b/drivers/powercap/intel_rapl.c
> @@ -29,6 +29,7 @@
>  #include <linux/sysfs.h>
>  #include <linux/cpu.h>
>  #include <linux/powercap.h>
> +#include <linux/suspend.h>
>  #include <asm/iosf_mbi.h>
>  
>  #include <asm/processor.h>
> @@ -155,6 +156,7 @@ struct rapl_power_limit {
>  	int prim_id; /* primitive ID used to enable */
>  	struct rapl_domain *domain;
>  	const char *name;
> +	u64 last_power_limit;
>  };
>  
>  static const char pl1_name[] = "long_term";
> @@ -1533,6 +1535,92 @@ static int rapl_cpu_down_prep(unsigned int
> cpu)
>  
>  static enum cpuhp_state pcap_rapl_online;
>  
> +static void power_limit_state_save(void)
> +{
> +	struct rapl_package *rp;
> +	struct rapl_domain *rd;
> +	int nr_pl, ret, i;
> +
> +	get_online_cpus();
> +	list_for_each_entry(rp, &rapl_packages, plist) {
> +		if (!rp->power_zone)
> +			continue;
> +		rd = power_zone_to_rapl_domain(rp->power_zone);
> +		nr_pl = find_nr_power_limit(rd);
> +		for (i = 0; i < nr_pl; i++) {
> +			switch (rd->rpl[i].prim_id) {
> +			case PL1_ENABLE:
> +				ret = rapl_read_data_raw(rd,
> +						POWER_LIMIT1,
> +						true,
> +						&rd-
> >rpl[i].last_power_limit);
> +				if (ret)
> +					rd->rpl[i].last_power_limit
> = 0;
> +				break;
> +			case PL2_ENABLE:
> +				ret = rapl_read_data_raw(rd,
> +						POWER_LIMIT2,
> +						true,
> +						&rd-
> >rpl[i].last_power_limit);
> +				if (ret)
> +					rd->rpl[i].last_power_limit
> = 0;
> +				break;
> +			}
> +		}
> +	}
> +	put_online_cpus();
> +}
> +
> +static void power_limit_state_restore(void)
> +{
> +	struct rapl_package *rp;
> +	struct rapl_domain *rd;
> +	int nr_pl, i;
> +
> +	get_online_cpus();
> +	list_for_each_entry(rp, &rapl_packages, plist) {
> +		if (!rp->power_zone)
> +			continue;
> +		rd = power_zone_to_rapl_domain(rp->power_zone);
> +		nr_pl = find_nr_power_limit(rd);
> +		for (i = 0; i < nr_pl; i++) {
> +			switch (rd->rpl[i].prim_id) {
> +			case PL1_ENABLE:
> +				if (rd->rpl[i].last_power_limit)
> +					rapl_write_data_raw(rd,
> +						POWER_LIMIT1,
> +						rd-
> >rpl[i].last_power_limit);
> +				break;
> +			case PL2_ENABLE:
> +				if (rd->rpl[i].last_power_limit)
> +					rapl_write_data_raw(rd,
> +						POWER_LIMIT2,
> +						rd-
> >rpl[i].last_power_limit);
> +				break;
> +			}
> +		}
> +	}
> +	put_online_cpus();
> +}
> +
> +static int rapl_pm_callback(struct notifier_block *nb,
> +	unsigned long mode, void *_unused)
> +{
> +	switch (mode) {
> +	case PM_SUSPEND_PREPARE:
> +		power_limit_state_save();
> +		break;
> +	case PM_POST_SUSPEND:
> +		power_limit_state_restore();
> +		break;
> +	}
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block rapl_pm_notifier = {
> +	.notifier_call = rapl_pm_callback,
> +};
> +
>  static int __init rapl_init(void)
>  {
>  	const struct x86_cpu_id *id;
> @@ -1560,8 +1648,16 @@ static int __init rapl_init(void)
>  
>  	/* Don't bail out if PSys is not supported */
>  	rapl_register_psys();
> +
> +	ret = register_pm_notifier(&rapl_pm_notifier);
> +	if (ret)
> +		goto err_unreg_all;
> +
>  	return 0;
>  
> +err_unreg_all:
> +	cpuhp_remove_state(pcap_rapl_online);
> +
>  err_unreg:
>  	rapl_unregister_powercap();
>  	return ret;
> @@ -1569,6 +1665,7 @@ static int __init rapl_init(void)
>  
>  static void __exit rapl_exit(void)
>  {
> +	unregister_pm_notifier(&rapl_pm_notifier);
>  	cpuhp_remove_state(pcap_rapl_online);
>  	rapl_unregister_powercap();
>  }

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

* [PATCH] powercap: add suspend and resume mechanism for SOC power limit
@ 2018-01-10  0:38 xinx.sun
  2018-01-10  1:47 ` Srinivas Pandruvada
  0 siblings, 1 reply; 10+ messages in thread
From: xinx.sun @ 2018-01-10  0:38 UTC (permalink / raw)
  To: rjw, jacob.jun.pan, srinivas.pandruvada
  Cc: zhen.han, xinx.sun, chaox.m.wang, linux-pm, linux-kernel

From: Zhen Han <zhen.han@intel.com>

PL1 and PL2 could be throlled or de-throttled by
Thermal management to control SOC temperature.
However, currently, their value will be reset to default value
after once system suspend and resume.
Add pm_notifier to save PL1, PL2 before system suspect and restore
PL1, PL2 after system resume.

Signed-off-by: Zhen Han <zhen.han@intel.com>
---
 drivers/powercap/intel_rapl.c | 97 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index d1694f1..0188cff 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -29,6 +29,7 @@
 #include <linux/sysfs.h>
 #include <linux/cpu.h>
 #include <linux/powercap.h>
+#include <linux/suspend.h>
 #include <asm/iosf_mbi.h>
 
 #include <asm/processor.h>
@@ -155,6 +156,7 @@ struct rapl_power_limit {
 	int prim_id; /* primitive ID used to enable */
 	struct rapl_domain *domain;
 	const char *name;
+	u64 last_power_limit;
 };
 
 static const char pl1_name[] = "long_term";
@@ -1533,6 +1535,92 @@ static int rapl_cpu_down_prep(unsigned int cpu)
 
 static enum cpuhp_state pcap_rapl_online;
 
+static void power_limit_state_save(void)
+{
+	struct rapl_package *rp;
+	struct rapl_domain *rd;
+	int nr_pl, ret, i;
+
+	get_online_cpus();
+	list_for_each_entry(rp, &rapl_packages, plist) {
+		if (!rp->power_zone)
+			continue;
+		rd = power_zone_to_rapl_domain(rp->power_zone);
+		nr_pl = find_nr_power_limit(rd);
+		for (i = 0; i < nr_pl; i++) {
+			switch (rd->rpl[i].prim_id) {
+			case PL1_ENABLE:
+				ret = rapl_read_data_raw(rd,
+						POWER_LIMIT1,
+						true,
+						&rd->rpl[i].last_power_limit);
+				if (ret)
+					rd->rpl[i].last_power_limit = 0;
+				break;
+			case PL2_ENABLE:
+				ret = rapl_read_data_raw(rd,
+						POWER_LIMIT2,
+						true,
+						&rd->rpl[i].last_power_limit);
+				if (ret)
+					rd->rpl[i].last_power_limit = 0;
+				break;
+			}
+		}
+	}
+	put_online_cpus();
+}
+
+static void power_limit_state_restore(void)
+{
+	struct rapl_package *rp;
+	struct rapl_domain *rd;
+	int nr_pl, i;
+
+	get_online_cpus();
+	list_for_each_entry(rp, &rapl_packages, plist) {
+		if (!rp->power_zone)
+			continue;
+		rd = power_zone_to_rapl_domain(rp->power_zone);
+		nr_pl = find_nr_power_limit(rd);
+		for (i = 0; i < nr_pl; i++) {
+			switch (rd->rpl[i].prim_id) {
+			case PL1_ENABLE:
+				if (rd->rpl[i].last_power_limit)
+					rapl_write_data_raw(rd,
+						POWER_LIMIT1,
+						rd->rpl[i].last_power_limit);
+				break;
+			case PL2_ENABLE:
+				if (rd->rpl[i].last_power_limit)
+					rapl_write_data_raw(rd,
+						POWER_LIMIT2,
+						rd->rpl[i].last_power_limit);
+				break;
+			}
+		}
+	}
+	put_online_cpus();
+}
+
+static int rapl_pm_callback(struct notifier_block *nb,
+	unsigned long mode, void *_unused)
+{
+	switch (mode) {
+	case PM_SUSPEND_PREPARE:
+		power_limit_state_save();
+		break;
+	case PM_POST_SUSPEND:
+		power_limit_state_restore();
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block rapl_pm_notifier = {
+	.notifier_call = rapl_pm_callback,
+};
+
 static int __init rapl_init(void)
 {
 	const struct x86_cpu_id *id;
@@ -1560,8 +1648,16 @@ static int __init rapl_init(void)
 
 	/* Don't bail out if PSys is not supported */
 	rapl_register_psys();
+
+	ret = register_pm_notifier(&rapl_pm_notifier);
+	if (ret)
+		goto err_unreg_all;
+
 	return 0;
 
+err_unreg_all:
+	cpuhp_remove_state(pcap_rapl_online);
+
 err_unreg:
 	rapl_unregister_powercap();
 	return ret;
@@ -1569,6 +1665,7 @@ static int __init rapl_init(void)
 
 static void __exit rapl_exit(void)
 {
+	unregister_pm_notifier(&rapl_pm_notifier);
 	cpuhp_remove_state(pcap_rapl_online);
 	rapl_unregister_powercap();
 }
-- 
1.9.1

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

end of thread, other threads:[~2018-01-10  7:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-09  8:25 [PATCH] powercap: add suspend and resume mechanism for SOC power limit xinx.sun
2018-01-09 11:39 ` Rafael J. Wysocki
2018-01-09 19:00   ` Srinivas Pandruvada
2018-01-10  0:21 ` Rafael J. Wysocki
2018-01-10  0:23   ` Sun, XinX
2018-01-10  0:38 xinx.sun
2018-01-10  1:47 ` Srinivas Pandruvada
2018-01-10  1:53   ` Sun, XinX
2018-01-10  6:21     ` Srinivas Pandruvada
2018-01-10  7:00       ` Sun, XinX

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