[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1515565269.585.30.camel@linux.intel.com>
Date: Tue, 09 Jan 2018 22:21:09 -0800
From: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
To: "Sun, XinX" <xinx.sun@...el.com>,
"rjw@...ysocki.net" <rjw@...ysocki.net>,
"jacob.jun.pan@...ux.intel.com" <jacob.jun.pan@...ux.intel.com>
Cc: "Han, Zhen" <zhen.han@...el.com>,
"Wang, ChaoX M" <chaox.m.wang@...el.com>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.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@...el.com>> > ---
---
Change:
Fixed the signed-off tag.
Thanks,
Srinivas
> Thanks & BR,
> Sun Xin
>
> -----Original Message-----
> From: Srinivas Pandruvada [mailto:srinivas.pandruvada@...ux.intel.com
> ]
> Sent: Wednesday, January 10, 2018 9:47 AM
> To: Sun, XinX <xinx.sun@...el.com>; rjw@...ysocki.net; jacob.jun.pan@
> linux.intel.com
> Cc: Han, Zhen <zhen.han@...el.com>; Wang, ChaoX M <chaox.m.wang@...el
> .com>; linux-pm@...r.kernel.org; linux-kernel@...r.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@...el.com wrote:
> >
> > From: Zhen Han <zhen.han@...el.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@...el.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();
> > }
Powered by blists - more mailing lists