| Message ID | 1478813718-12521-1-git-send-email-mario.limonciello@dell.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Mario,
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.9-rc4 next-20161110]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Mario-Limonciello/acpi-sleep-Use-the-FADT-to-prefer-Suspend-to-Idle-instead-of-S3/20161111-055555
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-x006-201645 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from include/linux/linkage.h:4:0,
from include/linux/kernel.h:6,
from include/linux/delay.h:10,
from drivers/acpi/sleep.c:13:
drivers/acpi/sleep.c: In function 'acpi_sleep_init':
>> drivers/acpi/sleep.c:917:6: error: implicit declaration of function 'suspend_to_idle_preferred' [-Werror=implicit-function-declaration]
if (suspend_to_idle_preferred()) {
^
include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/acpi/sleep.c:917:2: note: in expansion of macro 'if'
if (suspend_to_idle_preferred()) {
^~
Cyclomatic Complexity 1 arch/x86/include/asm/paravirt.h:arch_local_irq_disable
Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_sleep_dmi_check
Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_sleep_suspend_setup
Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_sleep_syscore_init
Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_sleep_hibernate_setup
Cyclomatic Complexity 9 drivers/acpi/sleep.c:acpi_sleep_tts_switch
Cyclomatic Complexity 9 drivers/acpi/sleep.c:acpi_sleep_pts_switch
Cyclomatic Complexity 1 drivers/acpi/sleep.c:sleep_notify_reboot
Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_power_off
Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_sleep_prepare
Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_power_off_prepare
Cyclomatic Complexity 5 drivers/acpi/sleep.c:acpi_sleep_state_supported
Cyclomatic Complexity 6 drivers/acpi/sleep.c:acpi_sleep_init
cc1: some warnings being treated as errors
vim +/suspend_to_idle_preferred +917 drivers/acpi/sleep.c
911 pm_power_off_prepare = acpi_power_off_prepare;
912 pm_power_off = acpi_power_off;
913 } else {
914 acpi_no_s5 = true;
915 }
916
> 917 if (suspend_to_idle_preferred()) {
918 sleep_states[ACPI_STATE_S3] = 0;
919 sleep_states[ACPI_STATE_S1] = 0;
920 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
> -----Original Message----- > From: kbuild test robot [mailto:lkp@intel.com] > Sent: Thursday, November 10, 2016 4:15 PM > To: Limonciello, Mario <Mario_Limonciello@Dell.com> > Cc: kbuild-all@01.org; linux-acpi@vger.kernel.org; Len Brown > <lenb@kernel.org>; Rafael J . Wysocki <rjw@rjwysocki.net>; Andy > Lutomirski <luto@kernel.org>; Limonciello, Mario > <Mario_Limonciello@Dell.com> > Subject: Re: [PATCH] acpi/sleep: Use the FADT to prefer Suspend-to-Idle > instead of S3 > > Hi Mario, > > [auto build test ERROR on pm/linux-next] > [also build test ERROR on v4.9-rc4 next-20161110] > [if your patch is applied to the wrong git tree, please drop us a note to help > improve the system] > > url: https://github.com/0day-ci/linux/commits/Mario-Limonciello/acpi- > sleep-Use-the-FADT-to-prefer-Suspend-to-Idle-instead-of-S3/20161111- > 055555 > base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git > linux-next > config: i386-randconfig-x006-201645 (attached as .config) > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All error/warnings (new ones prefixed by >>): > > In file included from include/linux/linkage.h:4:0, > from include/linux/kernel.h:6, > from include/linux/delay.h:10, > from drivers/acpi/sleep.c:13: > drivers/acpi/sleep.c: In function 'acpi_sleep_init': > >> drivers/acpi/sleep.c:917:6: error: implicit declaration of function > 'suspend_to_idle_preferred' [-Werror=implicit-function-declaration] > if (suspend_to_idle_preferred()) { > ^ > include/linux/compiler.h:149:30: note: in definition of macro '__trace_if' > if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ > ^~~~ > >> drivers/acpi/sleep.c:917:2: note: in expansion of macro 'if' > if (suspend_to_idle_preferred()) { > ^~ > Cyclomatic Complexity 1 > arch/x86/include/asm/paravirt.h:arch_local_irq_disable > Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_sleep_dmi_check > Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_sleep_suspend_setup > Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_sleep_syscore_init > Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_sleep_hibernate_setup > Cyclomatic Complexity 9 drivers/acpi/sleep.c:acpi_sleep_tts_switch > Cyclomatic Complexity 9 drivers/acpi/sleep.c:acpi_sleep_pts_switch > Cyclomatic Complexity 1 drivers/acpi/sleep.c:sleep_notify_reboot > Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_power_off > Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_sleep_prepare > Cyclomatic Complexity 1 drivers/acpi/sleep.c:acpi_power_off_prepare > Cyclomatic Complexity 5 drivers/acpi/sleep.c:acpi_sleep_state_supported > Cyclomatic Complexity 6 drivers/acpi/sleep.c:acpi_sleep_init > cc1: some warnings being treated as errors > > vim +/suspend_to_idle_preferred +917 drivers/acpi/sleep.c > > 911 pm_power_off_prepare = acpi_power_off_prepare; > 912 pm_power_off = acpi_power_off; > 913 } else { > 914 acpi_no_s5 = true; > 915 } > 916 > > 917 if (suspend_to_idle_preferred()) { > 918 sleep_states[ACPI_STATE_S3] = 0; > 919 sleep_states[ACPI_STATE_S1] = 0; > 920 } > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation In v2 I'll resubmit with the test moved into acpi_sleep_suspend_setup(void) so that this works properly even if CONFIG_ACPI_SLEEP isn't enabled. I'll look for any other comments however about this approach before I submit v2. Thanks, -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 10, 2016 at 10:35 PM, Mario Limonciello <mario.limonciello@dell.com> wrote: > If the ACPI_FADT_LOW_POWER_S0 bit is set, this indicates the platform > should get identical or better performance using Suspend-To-Idle. > > By removing S3 and S1 userspace will prefer suspend-to-idle in > these situations too. > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> > --- > drivers/acpi/sleep.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c > index deb0ff7..ff1d8f1 100644 > --- a/drivers/acpi/sleep.c > +++ b/drivers/acpi/sleep.c > @@ -171,6 +171,12 @@ static int __init init_nvs_nosave(const struct dmi_system_id *d) > return 0; > } > > +static bool suspend_to_idle_preferred(void) > +{ > + return (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0); > +} > + > + > static struct dmi_system_id acpisleep_dmi_table[] __initdata = { > { > .callback = init_old_suspend_ordering, > @@ -908,6 +914,11 @@ int __init acpi_sleep_init(void) > acpi_no_s5 = true; > } > > + if (suspend_to_idle_preferred()) { > + sleep_states[ACPI_STATE_S3] = 0; > + sleep_states[ACPI_STATE_S1] = 0; > + } > + > supported[0] = 0; > for (i = 0; i < ACPI_S_STATE_COUNT; i++) { > if (sleep_states[i]) > -- I'd do that in a different way. Let me cut a patch for that (I haven't had the time to do that yet) and we'll see. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiByand5c29ja2lAZ21haWwuY29t IFttYWlsdG86cmp3eXNvY2tpQGdtYWlsLmNvbV0gT24gQmVoYWxmIE9mDQo+IFJhZmFlbCBKLiBX eXNvY2tpDQo+IFNlbnQ6IFRodXJzZGF5LCBOb3ZlbWJlciAxMCwgMjAxNiA1OjU0IFBNDQo+IFRv OiBMaW1vbmNpZWxsbywgTWFyaW8gPE1hcmlvX0xpbW9uY2llbGxvQERlbGwuY29tPg0KPiBDYzog QUNQSSBEZXZlbCBNYWxpbmcgTGlzdCA8bGludXgtYWNwaUB2Z2VyLmtlcm5lbC5vcmc+OyBMZW4g QnJvd24NCj4gPGxlbmJAa2VybmVsLm9yZz47IFJhZmFlbCBKIC4gV3lzb2NraSA8cmp3QHJqd3lz b2NraS5uZXQ+OyBBbmR5DQo+IEx1dG9taXJza2kgPGx1dG9Aa2VybmVsLm9yZz4NCj4gU3ViamVj dDogUmU6IFtQQVRDSF0gYWNwaS9zbGVlcDogVXNlIHRoZSBGQURUIHRvIHByZWZlciBTdXNwZW5k LXRvLUlkbGUNCj4gaW5zdGVhZCBvZiBTMw0KPiANCj4gT24gVGh1LCBOb3YgMTAsIDIwMTYgYXQg MTA6MzUgUE0sIE1hcmlvIExpbW9uY2llbGxvDQo+IDxtYXJpby5saW1vbmNpZWxsb0BkZWxsLmNv bT4gd3JvdGU6DQo+ID4gSWYgdGhlIEFDUElfRkFEVF9MT1dfUE9XRVJfUzAgYml0IGlzIHNldCwg dGhpcyBpbmRpY2F0ZXMgdGhlIHBsYXRmb3JtDQo+ID4gc2hvdWxkIGdldCBpZGVudGljYWwgb3Ig YmV0dGVyIHBlcmZvcm1hbmNlIHVzaW5nIFN1c3BlbmQtVG8tSWRsZS4NCj4gPg0KPiA+IEJ5IHJl bW92aW5nIFMzIGFuZCBTMSB1c2Vyc3BhY2Ugd2lsbCBwcmVmZXIgc3VzcGVuZC10by1pZGxlIGlu DQo+ID4gdGhlc2Ugc2l0dWF0aW9ucyB0b28uDQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBNYXJp byBMaW1vbmNpZWxsbyA8bWFyaW8ubGltb25jaWVsbG9AZGVsbC5jb20+DQo+ID4gLS0tDQo+ID4g IGRyaXZlcnMvYWNwaS9zbGVlcC5jIHwgMTEgKysrKysrKysrKysNCj4gPiAgMSBmaWxlIGNoYW5n ZWQsIDExIGluc2VydGlvbnMoKykNCj4gPg0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2FjcGkv c2xlZXAuYyBiL2RyaXZlcnMvYWNwaS9zbGVlcC5jDQo+ID4gaW5kZXggZGViMGZmNy4uZmYxZDhm MSAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL2FjcGkvc2xlZXAuYw0KPiA+ICsrKyBiL2RyaXZl cnMvYWNwaS9zbGVlcC5jDQo+ID4gQEAgLTE3MSw2ICsxNzEsMTIgQEAgc3RhdGljIGludCBfX2lu aXQgaW5pdF9udnNfbm9zYXZlKGNvbnN0IHN0cnVjdA0KPiBkbWlfc3lzdGVtX2lkICpkKQ0KPiA+ ICAgICAgICAgcmV0dXJuIDA7DQo+ID4gIH0NCj4gPg0KPiA+ICtzdGF0aWMgYm9vbCBzdXNwZW5k X3RvX2lkbGVfcHJlZmVycmVkKHZvaWQpDQo+ID4gK3sNCj4gPiArICAgICAgIHJldHVybiAoYWNw aV9nYmxfRkFEVC5mbGFncyAmIEFDUElfRkFEVF9MT1dfUE9XRVJfUzApOw0KPiA+ICt9DQo+ID4g Kw0KPiA+ICsNCj4gPiAgc3RhdGljIHN0cnVjdCBkbWlfc3lzdGVtX2lkIGFjcGlzbGVlcF9kbWlf dGFibGVbXSBfX2luaXRkYXRhID0gew0KPiA+ICAgICAgICAgew0KPiA+ICAgICAgICAgLmNhbGxi YWNrID0gaW5pdF9vbGRfc3VzcGVuZF9vcmRlcmluZywNCj4gPiBAQCAtOTA4LDYgKzkxNCwxMSBA QCBpbnQgX19pbml0IGFjcGlfc2xlZXBfaW5pdCh2b2lkKQ0KPiA+ICAgICAgICAgICAgICAgICBh Y3BpX25vX3M1ID0gdHJ1ZTsNCj4gPiAgICAgICAgIH0NCj4gPg0KPiA+ICsgICAgICAgaWYgKHN1 c3BlbmRfdG9faWRsZV9wcmVmZXJyZWQoKSkgew0KPiA+ICsgICAgICAgICAgICAgICBzbGVlcF9z dGF0ZXNbQUNQSV9TVEFURV9TM10gPSAwOw0KPiA+ICsgICAgICAgICAgICAgICBzbGVlcF9zdGF0 ZXNbQUNQSV9TVEFURV9TMV0gPSAwOw0KPiA+ICsgICAgICAgfQ0KPiA+ICsNCj4gPiAgICAgICAg IHN1cHBvcnRlZFswXSA9IDA7DQo+ID4gICAgICAgICBmb3IgKGkgPSAwOyBpIDwgQUNQSV9TX1NU QVRFX0NPVU5UOyBpKyspIHsNCj4gPiAgICAgICAgICAgICAgICAgaWYgKHNsZWVwX3N0YXRlc1tp XSkNCj4gPiAtLQ0KPiANCj4gSSdkIGRvIHRoYXQgaW4gYSBkaWZmZXJlbnQgd2F5Lg0KPiANCj4g TGV0IG1lIGN1dCBhIHBhdGNoIGZvciB0aGF0IChJIGhhdmVuJ3QgaGFkIHRoZSB0aW1lIHRvIGRv IHRoYXQgeWV0KQ0KPiBhbmQgd2UnbGwgc2VlLg0KPiANCj4gVGhhbmtzLA0KPiBSYWZhZWwNCg0K T0ssIGxvb2sgZm9yd2FyZCB0byBzZWVpbmcgdGhlIGFwcHJvYWNoIHlvdSB3YW50IHRvIGdvIHdp dGggdGhpcw0KaW5zdGVhZC4NCg0KVGhhbmtzLA0K -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, November 16, 2016 08:18:13 PM Mario.Limonciello@dell.com wrote: > > -----Original Message----- > > From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of > > Rafael J. Wysocki > > Sent: Thursday, November 10, 2016 5:54 PM > > To: Limonciello, Mario <Mario_Limonciello@Dell.com> > > Cc: ACPI Devel Maling List <linux-acpi@vger.kernel.org>; Len Brown > > <lenb@kernel.org>; Rafael J . Wysocki <rjw@rjwysocki.net>; Andy > > Lutomirski <luto@kernel.org> > > Subject: Re: [PATCH] acpi/sleep: Use the FADT to prefer Suspend-to-Idle > > instead of S3 > > > > On Thu, Nov 10, 2016 at 10:35 PM, Mario Limonciello > > <mario.limonciello@dell.com> wrote: > > > If the ACPI_FADT_LOW_POWER_S0 bit is set, this indicates the platform > > > should get identical or better performance using Suspend-To-Idle. > > > > > > By removing S3 and S1 userspace will prefer suspend-to-idle in > > > these situations too. > > > > > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> > > > --- > > > drivers/acpi/sleep.c | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c > > > index deb0ff7..ff1d8f1 100644 > > > --- a/drivers/acpi/sleep.c > > > +++ b/drivers/acpi/sleep.c > > > @@ -171,6 +171,12 @@ static int __init init_nvs_nosave(const struct > > dmi_system_id *d) > > > return 0; > > > } > > > > > > +static bool suspend_to_idle_preferred(void) > > > +{ > > > + return (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0); > > > +} > > > + > > > + > > > static struct dmi_system_id acpisleep_dmi_table[] __initdata = { > > > { > > > .callback = init_old_suspend_ordering, > > > @@ -908,6 +914,11 @@ int __init acpi_sleep_init(void) > > > acpi_no_s5 = true; > > > } > > > > > > + if (suspend_to_idle_preferred()) { > > > + sleep_states[ACPI_STATE_S3] = 0; > > > + sleep_states[ACPI_STATE_S1] = 0; > > > + } > > > + > > > supported[0] = 0; > > > for (i = 0; i < ACPI_S_STATE_COUNT; i++) { > > > if (sleep_states[i]) > > > -- > > > > I'd do that in a different way. > > > > Let me cut a patch for that (I haven't had the time to do that yet) > > and we'll see. > > > > Thanks, > > Rafael > > OK, look forward to seeing the approach you want to go with this > instead. Please have a look at https://patchwork.kernel.org/patch/9433421/ https://patchwork.kernel.org/patch/9433429/ Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index deb0ff7..ff1d8f1 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -171,6 +171,12 @@ static int __init init_nvs_nosave(const struct dmi_system_id *d) return 0; } +static bool suspend_to_idle_preferred(void) +{ + return (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0); +} + + static struct dmi_system_id acpisleep_dmi_table[] __initdata = { { .callback = init_old_suspend_ordering, @@ -908,6 +914,11 @@ int __init acpi_sleep_init(void) acpi_no_s5 = true; } + if (suspend_to_idle_preferred()) { + sleep_states[ACPI_STATE_S3] = 0; + sleep_states[ACPI_STATE_S1] = 0; + } + supported[0] = 0; for (i = 0; i < ACPI_S_STATE_COUNT; i++) { if (sleep_states[i])
If the ACPI_FADT_LOW_POWER_S0 bit is set, this indicates the platform should get identical or better performance using Suspend-To-Idle. By removing S3 and S1 userspace will prefer suspend-to-idle in these situations too. Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> --- drivers/acpi/sleep.c | 11 +++++++++++ 1 file changed, 11 insertions(+)