Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Srinivas Pandruvada]

On Thu, 2017-07-06 at 21:41 +1000, Tom Lanyon wrote:
> Sorry for the slow response - I've just had a chance to run some more
> tests.
> 
> I tried to disable the SD card reader in the BIOS as suggested
> earlier
> in the thread, but that didn't seem to make a significant change.
> More inline below.
> 
> On Tue, 2017-06-27 at 17:03 +0200, Rafael J. Wysocki wrote:
> > 
> > I would carry out s2idle under turbostat to see how much PC10
> > residency is there while suspended.  That may be a significant
> > factor.
> > 
> > Most likely there is a device preventing the SoC from reaching its
> > deepest low-power states under Linux on your system and it needs to
> > be
> > identified and dealt with.
> I'm not entirely sure how turbostat records metrics so wasn't sure
> how
> to measure correctly.  I kept turbostat running while performing a
> s2idle and recorded the output:
> 
> https://gist.githubusercontent.com/tomlanyon/3238e742a155e7fa27658aa0
> 960bdee4/raw/98b5f050e5eb2f88af47b2afd17080e7dd85d20f/turbostat
> 
> I'm not familiar with the output format - I see some high percentages
> of C10, but nothing in Pkg%pc10. Which is of interest in this
> scenario?
I see that you do have Pkg%pc10 of 86.56 in one interval. The system
was in Pkg%pc8 for multiple intervals before Pkg%pc10. So can you try
this also before your test?

# cd /sys/kernel/debug/pmc_core
# for i in {0..32}; do echo $i > ltr_ignore; done

Thanks,
Srinivas

> 
> On 28 June 2017 at 02:14, Srinivas Pandruvada
>  wrote:
> > 
> > Also make sure that you have no FW loading error for i915.
> > #dmesg | grep i915
> > It will display that Guc FW was loaded etc..
> > The latest FW can be downloaded from
> > https://01.org/linuxgraphics/downloads/firmware
> > 
> > If you don't see PC10 residency, we can try something more.
> I've confirmed that there's no FW errors for the i915.

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Srinivas Pandruvada]

On Thu, 2017-07-06 at 21:41 +1000, Tom Lanyon wrote:
> Sorry for the slow response - I've just had a chance to run some more
> tests.
> 
> I tried to disable the SD card reader in the BIOS as suggested
> earlier
> in the thread, but that didn't seem to make a significant change.
> More inline below.
> 
> On Tue, 2017-06-27 at 17:03 +0200, Rafael J. Wysocki wrote:
> > 
> > I would carry out s2idle under turbostat to see how much PC10
> > residency is there while suspended.  That may be a significant
> > factor.
> > 
> > Most likely there is a device preventing the SoC from reaching its
> > deepest low-power states under Linux on your system and it needs to
> > be
> > identified and dealt with.
> I'm not entirely sure how turbostat records metrics so wasn't sure
> how
> to measure correctly.  I kept turbostat running while performing a
> s2idle and recorded the output:
> 
> https://gist.githubusercontent.com/tomlanyon/3238e742a155e7fa27658aa0
> 960bdee4/raw/98b5f050e5eb2f88af47b2afd17080e7dd85d20f/turbostat
> 
> I'm not familiar with the output format - I see some high percentages
> of C10, but nothing in Pkg%pc10. Which is of interest in this
> scenario?
I see that you do have Pkg%pc10 of 86.56 in one interval. The system
was in Pkg%pc8 for multiple intervals before Pkg%pc10. So can you try
this also before your test?

# cd /sys/kernel/debug/pmc_core
# for i in {0..32}; do echo $i > ltr_ignore; done

Thanks,
Srinivas

> 
> On 28 June 2017 at 02:14, Srinivas Pandruvada
>  wrote:
> > 
> > Also make sure that you have no FW loading error for i915.
> > #dmesg | grep i915
> > It will display that Guc FW was loaded etc..
> > The latest FW can be downloaded from
> > https://01.org/linuxgraphics/downloads/firmware
> > 
> > If you don't see PC10 residency, we can try something more.
> I've confirmed that there's no FW errors for the i915.

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Tom Lanyon]

Sorry for the slow response - I've just had a chance to run some more tests.

I tried to disable the SD card reader in the BIOS as suggested earlier
in the thread, but that didn't seem to make a significant change.
More inline below.

On Tue, 2017-06-27 at 17:03 +0200, Rafael J. Wysocki wrote:
> I would carry out s2idle under turbostat to see how much PC10
> residency is there while suspended.  That may be a significant factor.
>
> Most likely there is a device preventing the SoC from reaching its
> deepest low-power states under Linux on your system and it needs to be
> identified and dealt with.

I'm not entirely sure how turbostat records metrics so wasn't sure how
to measure correctly.  I kept turbostat running while performing a
s2idle and recorded the output:

https://gist.githubusercontent.com/tomlanyon/3238e742a155e7fa27658aa0960bdee4/raw/98b5f050e5eb2f88af47b2afd17080e7dd85d20f/turbostat

I'm not familiar with the output format - I see some high percentages
of C10, but nothing in Pkg%pc10. Which is of interest in this
scenario?

On 28 June 2017 at 02:14, Srinivas Pandruvada
 wrote:
> Also make sure that you have no FW loading error for i915.
> #dmesg | grep i915
> It will display that Guc FW was loaded etc..
> The latest FW can be downloaded from
> https://01.org/linuxgraphics/downloads/firmware
>
> If you don't see PC10 residency, we can try something more.

I've confirmed that there's no FW errors for the i915.

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Tom Lanyon]

Sorry for the slow response - I've just had a chance to run some more tests.

I tried to disable the SD card reader in the BIOS as suggested earlier
in the thread, but that didn't seem to make a significant change.
More inline below.

On Tue, 2017-06-27 at 17:03 +0200, Rafael J. Wysocki wrote:
> I would carry out s2idle under turbostat to see how much PC10
> residency is there while suspended.  That may be a significant factor.
>
> Most likely there is a device preventing the SoC from reaching its
> deepest low-power states under Linux on your system and it needs to be
> identified and dealt with.

I'm not entirely sure how turbostat records metrics so wasn't sure how
to measure correctly.  I kept turbostat running while performing a
s2idle and recorded the output:

https://gist.githubusercontent.com/tomlanyon/3238e742a155e7fa27658aa0960bdee4/raw/98b5f050e5eb2f88af47b2afd17080e7dd85d20f/turbostat

I'm not familiar with the output format - I see some high percentages
of C10, but nothing in Pkg%pc10. Which is of interest in this
scenario?

On 28 June 2017 at 02:14, Srinivas Pandruvada
 wrote:
> Also make sure that you have no FW loading error for i915.
> #dmesg | grep i915
> It will display that Guc FW was loaded etc..
> The latest FW can be downloaded from
> https://01.org/linuxgraphics/downloads/firmware
>
> If you don't see PC10 residency, we can try something more.

I've confirmed that there's no FW errors for the i915.

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Srinivas Pandruvada]

On Tue, 2017-06-27 at 17:03 +0200, Rafael J. Wysocki wrote:
> On Tuesday, June 27, 2017 03:50:33 PM Tom Lanyon wrote:
> > 
> > On 23 June 2017 at 12:40, Linus Torvalds  > .org> wrote:
> > > 
> > > On Thu, Jun 22, 2017 at 4:56 PM, Rafael J. Wysocki  > > .net> wrote:
> > > > 
> > > > 
> > > > Some recent Dell laptops, including the XPS13 model numbers
> > > > 9360 and
> > > > 9365, cannot be woken up from suspend-to-idle by pressing the
> > > > power
> > > > button which is unexpected and makes that feature less usable
> > > > on
> > > > those systems.  [ details removed ]
> > > 
> > > This looks much more reasonable and more likely to work on future
> > > machines too.
> > > 
> > > Of course, who knows what broken machines it will cause problems
> > > on,
> > > but it sounds like the code now does what it's supposed to and
> > > what
> > > Win10 does, so maybe it JustWorks(tm). Hah.
> > 
> > Rafael - thanks for your efforts on this.
> 
> You're welcome!
> 
> > 
> > I wanted to provide some feedback from some quick and naive tests
> > on
> > an XPS 13 9365 in case it was useful, as it seems like there's
> > still
> > some way to go before matching Win10's behaviour.
> > 
> > Linux idling w/ screen ON => 17% battery drain per hour.
> > Linux idling w/ screen OFF => 12% battery drain per hour.
> > Linux during s2idle => 6% battery drain per hour.
> > Win10 during sleep => 1% battery drain per hour.
> > 
> > where Linux = 4.12-rc6 + the latest patch from your acpi-pm-test
> > branch.
> > 
> > So whilst s2idle halves the battery drain compared to the machine
> > staying powered on, it's still significantly more draining than
> > Win10.
> 
> Thanks for the data.
> 
> > 
> > Let me know if there's any more useful analysis I can do.
> 
> I would carry out s2idle under turbostat to see how much PC10
> residency is
> there while suspended.  That may be a significant factor.
> 
> Most likely there is a device preventing the SoC from reaching its
> deepest
> low-power states under Linux on your system and it needs to be
> identified
> and dealt with.
Also make sure that you have no FW loading error for i915.
#dmesg | grep i915
It will display that Guc FW was loaded etc..
The latest FW can be downloaded from
https://01.org/linuxgraphics/downloads/firmware

If you don't see PC10 residency, we can try something more.

Thanks,
Srinivas


> 
> Thanks,
> Rafael
> 

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Srinivas Pandruvada]

On Tue, 2017-06-27 at 17:03 +0200, Rafael J. Wysocki wrote:
> On Tuesday, June 27, 2017 03:50:33 PM Tom Lanyon wrote:
> > 
> > On 23 June 2017 at 12:40, Linus Torvalds  > .org> wrote:
> > > 
> > > On Thu, Jun 22, 2017 at 4:56 PM, Rafael J. Wysocki  > > .net> wrote:
> > > > 
> > > > 
> > > > Some recent Dell laptops, including the XPS13 model numbers
> > > > 9360 and
> > > > 9365, cannot be woken up from suspend-to-idle by pressing the
> > > > power
> > > > button which is unexpected and makes that feature less usable
> > > > on
> > > > those systems.  [ details removed ]
> > > 
> > > This looks much more reasonable and more likely to work on future
> > > machines too.
> > > 
> > > Of course, who knows what broken machines it will cause problems
> > > on,
> > > but it sounds like the code now does what it's supposed to and
> > > what
> > > Win10 does, so maybe it JustWorks(tm). Hah.
> > 
> > Rafael - thanks for your efforts on this.
> 
> You're welcome!
> 
> > 
> > I wanted to provide some feedback from some quick and naive tests
> > on
> > an XPS 13 9365 in case it was useful, as it seems like there's
> > still
> > some way to go before matching Win10's behaviour.
> > 
> > Linux idling w/ screen ON => 17% battery drain per hour.
> > Linux idling w/ screen OFF => 12% battery drain per hour.
> > Linux during s2idle => 6% battery drain per hour.
> > Win10 during sleep => 1% battery drain per hour.
> > 
> > where Linux = 4.12-rc6 + the latest patch from your acpi-pm-test
> > branch.
> > 
> > So whilst s2idle halves the battery drain compared to the machine
> > staying powered on, it's still significantly more draining than
> > Win10.
> 
> Thanks for the data.
> 
> > 
> > Let me know if there's any more useful analysis I can do.
> 
> I would carry out s2idle under turbostat to see how much PC10
> residency is
> there while suspended.  That may be a significant factor.
> 
> Most likely there is a device preventing the SoC from reaching its
> deepest
> low-power states under Linux on your system and it needs to be
> identified
> and dealt with.
Also make sure that you have no FW loading error for i915.
#dmesg | grep i915
It will display that Guc FW was loaded etc..
The latest FW can be downloaded from
https://01.org/linuxgraphics/downloads/firmware

If you don't see PC10 residency, we can try something more.

Thanks,
Srinivas


> 
> Thanks,
> Rafael
> 

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Rafael J. Wysocki]

On Tuesday, June 27, 2017 03:50:33 PM Tom Lanyon wrote:
> On 23 June 2017 at 12:40, Linus Torvalds  
> wrote:
> > On Thu, Jun 22, 2017 at 4:56 PM, Rafael J. Wysocki  
> > wrote:
> >>
> >> Some recent Dell laptops, including the XPS13 model numbers 9360 and
> >> 9365, cannot be woken up from suspend-to-idle by pressing the power
> >> button which is unexpected and makes that feature less usable on
> >> those systems.  [ details removed ]
> >
> > This looks much more reasonable and more likely to work on future machines 
> > too.
> >
> > Of course, who knows what broken machines it will cause problems on,
> > but it sounds like the code now does what it's supposed to and what
> > Win10 does, so maybe it JustWorks(tm). Hah.
> 
> Rafael - thanks for your efforts on this.

You're welcome!

> I wanted to provide some feedback from some quick and naive tests on
> an XPS 13 9365 in case it was useful, as it seems like there's still
> some way to go before matching Win10's behaviour.
> 
> Linux idling w/ screen ON => 17% battery drain per hour.
> Linux idling w/ screen OFF => 12% battery drain per hour.
> Linux during s2idle => 6% battery drain per hour.
> Win10 during sleep => 1% battery drain per hour.
> 
> where Linux = 4.12-rc6 + the latest patch from your acpi-pm-test branch.
> 
> So whilst s2idle halves the battery drain compared to the machine
> staying powered on, it's still significantly more draining than Win10.

Thanks for the data.

> Let me know if there's any more useful analysis I can do.

I would carry out s2idle under turbostat to see how much PC10 residency is
there while suspended.  That may be a significant factor.

Most likely there is a device preventing the SoC from reaching its deepest
low-power states under Linux on your system and it needs to be identified
and dealt with.

Thanks,
Rafael

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Rafael J. Wysocki]

On Tuesday, June 27, 2017 03:50:33 PM Tom Lanyon wrote:
> On 23 June 2017 at 12:40, Linus Torvalds  
> wrote:
> > On Thu, Jun 22, 2017 at 4:56 PM, Rafael J. Wysocki  
> > wrote:
> >>
> >> Some recent Dell laptops, including the XPS13 model numbers 9360 and
> >> 9365, cannot be woken up from suspend-to-idle by pressing the power
> >> button which is unexpected and makes that feature less usable on
> >> those systems.  [ details removed ]
> >
> > This looks much more reasonable and more likely to work on future machines 
> > too.
> >
> > Of course, who knows what broken machines it will cause problems on,
> > but it sounds like the code now does what it's supposed to and what
> > Win10 does, so maybe it JustWorks(tm). Hah.
> 
> Rafael - thanks for your efforts on this.

You're welcome!

> I wanted to provide some feedback from some quick and naive tests on
> an XPS 13 9365 in case it was useful, as it seems like there's still
> some way to go before matching Win10's behaviour.
> 
> Linux idling w/ screen ON => 17% battery drain per hour.
> Linux idling w/ screen OFF => 12% battery drain per hour.
> Linux during s2idle => 6% battery drain per hour.
> Win10 during sleep => 1% battery drain per hour.
> 
> where Linux = 4.12-rc6 + the latest patch from your acpi-pm-test branch.
> 
> So whilst s2idle halves the battery drain compared to the machine
> staying powered on, it's still significantly more draining than Win10.

Thanks for the data.

> Let me know if there's any more useful analysis I can do.

I would carry out s2idle under turbostat to see how much PC10 residency is
there while suspended.  That may be a significant factor.

Most likely there is a device preventing the SoC from reaching its deepest
low-power states under Linux on your system and it needs to be identified
and dealt with.

Thanks,
Rafael

RE: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Mario.Limonciello]

> -Original Message-
> From: Tom Lanyon [mailto:t...@oneshoeco.com]
> Sent: Tuesday, June 27, 2017 12:51 AM
> To: Rafael J. Wysocki <r...@rjwysocki.net>
> Cc: Linux ACPI <linux-a...@vger.kernel.org>; Linux PM  p...@vger.kernel.org>; Andy Shevchenko <andriy.shevche...@linux.intel.com>;
> Darren Hart <dvh...@infradead.org>; LKML <linux-kernel@vger.kernel.org>;
> Srinivas Pandruvada <srinivas.pandruv...@linux.intel.com>; Mika Westerberg
> <mika.westerb...@linux.intel.com>; Limonciello, Mario
> <mario_limoncie...@dell.com>; Jérôme de Bretagne
> <jerome.debreta...@gmail.com>; Zheng, Lv <lv.zh...@intel.com>; Linus Torvalds
> <torva...@linux-foundation.org>
> Subject: Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on 
> recent
> systems
> 
> On 23 June 2017 at 12:40, Linus Torvalds <torva...@linux-foundation.org> 
> wrote:
> > On Thu, Jun 22, 2017 at 4:56 PM, Rafael J. Wysocki <r...@rjwysocki.net> 
> > wrote:
> >>
> >> Some recent Dell laptops, including the XPS13 model numbers 9360 and
> >> 9365, cannot be woken up from suspend-to-idle by pressing the power
> >> button which is unexpected and makes that feature less usable on
> >> those systems.  [ details removed ]
> >
> > This looks much more reasonable and more likely to work on future machines 
> > too.
> >
> > Of course, who knows what broken machines it will cause problems on,
> > but it sounds like the code now does what it's supposed to and what
> > Win10 does, so maybe it JustWorks(tm). Hah.
> 
> Rafael - thanks for your efforts on this.
> 
> I wanted to provide some feedback from some quick and naive tests on
> an XPS 13 9365 in case it was useful, as it seems like there's still
> some way to go before matching Win10's behaviour.
> 
> Linux idling w/ screen ON => 17% battery drain per hour.
> Linux idling w/ screen OFF => 12% battery drain per hour.
> Linux during s2idle => 6% battery drain per hour.
> Win10 during sleep => 1% battery drain per hour.
> 
> where Linux = 4.12-rc6 + the latest patch from your acpi-pm-test branch.
> 
> So whilst s2idle halves the battery drain compared to the machine
> staying powered on, it's still significantly more draining than Win10.
> Let me know if there's any more useful analysis I can do.
> 
> -Tom

Tom,

This is quite useful data points to provide, thanks.

Would you mind doing on more test in your Linux comparison of turning off SD 
card reader in the BIOS setup?

Thanks,

RE: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Mario.Limonciello]

> -Original Message-
> From: Tom Lanyon [mailto:t...@oneshoeco.com]
> Sent: Tuesday, June 27, 2017 12:51 AM
> To: Rafael J. Wysocki 
> Cc: Linux ACPI ; Linux PM  p...@vger.kernel.org>; Andy Shevchenko ;
> Darren Hart ; LKML ;
> Srinivas Pandruvada ; Mika Westerberg
> ; Limonciello, Mario
> ; Jérôme de Bretagne
> ; Zheng, Lv ; Linus Torvalds
> 
> Subject: Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on 
> recent
> systems
> 
> On 23 June 2017 at 12:40, Linus Torvalds  
> wrote:
> > On Thu, Jun 22, 2017 at 4:56 PM, Rafael J. Wysocki  
> > wrote:
> >>
> >> Some recent Dell laptops, including the XPS13 model numbers 9360 and
> >> 9365, cannot be woken up from suspend-to-idle by pressing the power
> >> button which is unexpected and makes that feature less usable on
> >> those systems.  [ details removed ]
> >
> > This looks much more reasonable and more likely to work on future machines 
> > too.
> >
> > Of course, who knows what broken machines it will cause problems on,
> > but it sounds like the code now does what it's supposed to and what
> > Win10 does, so maybe it JustWorks(tm). Hah.
> 
> Rafael - thanks for your efforts on this.
> 
> I wanted to provide some feedback from some quick and naive tests on
> an XPS 13 9365 in case it was useful, as it seems like there's still
> some way to go before matching Win10's behaviour.
> 
> Linux idling w/ screen ON => 17% battery drain per hour.
> Linux idling w/ screen OFF => 12% battery drain per hour.
> Linux during s2idle => 6% battery drain per hour.
> Win10 during sleep => 1% battery drain per hour.
> 
> where Linux = 4.12-rc6 + the latest patch from your acpi-pm-test branch.
> 
> So whilst s2idle halves the battery drain compared to the machine
> staying powered on, it's still significantly more draining than Win10.
> Let me know if there's any more useful analysis I can do.
> 
> -Tom

Tom,

This is quite useful data points to provide, thanks.

Would you mind doing on more test in your Linux comparison of turning off SD 
card reader in the BIOS setup?

Thanks,

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Tom Lanyon]

On 27 June 2017 at 16:47, Andy Shevchenko  wrote:
> Tom, thanks for this.
> I would speculate that the problem might be in the certain device
> drivers. It would be nice to get statistics which wakeup source
> generates more hits.

Is this something I can determine without CONFIG_ACPI_DEBUG being enabled?

I don't get anything in dmesg other than indications that it's
resuming and then sleeping again every second or so.

[   45.463907] PM: Suspending system (freeze)
[   47.703216] PM: suspend of devices complete after 2028.170 msecs
[   47.721329] PM: late suspend of devices complete after 18.108 msecs
[   47.723153] ACPI : EC: interrupt blocked
[   47.757801] PM: noirq suspend of devices complete after 35.746 msecs
[   47.757802] PM: suspend-to-idle
[   48.944708] Suspended for 0.779 seconds
[   48.945030] ACPI : EC: interrupt unblocked
[   48.980728] PM: noirq resume of devices complete after 35.924 msecs
[   48.982265] ACPI : EC: interrupt blocked
[   49.027946] PM: noirq suspend of devices complete after 47.016 msecs

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Tom Lanyon]

On 27 June 2017 at 16:47, Andy Shevchenko  wrote:
> Tom, thanks for this.
> I would speculate that the problem might be in the certain device
> drivers. It would be nice to get statistics which wakeup source
> generates more hits.

Is this something I can determine without CONFIG_ACPI_DEBUG being enabled?

I don't get anything in dmesg other than indications that it's
resuming and then sleeping again every second or so.

[   45.463907] PM: Suspending system (freeze)
[   47.703216] PM: suspend of devices complete after 2028.170 msecs
[   47.721329] PM: late suspend of devices complete after 18.108 msecs
[   47.723153] ACPI : EC: interrupt blocked
[   47.757801] PM: noirq suspend of devices complete after 35.746 msecs
[   47.757802] PM: suspend-to-idle
[   48.944708] Suspended for 0.779 seconds
[   48.945030] ACPI : EC: interrupt unblocked
[   48.980728] PM: noirq resume of devices complete after 35.924 msecs
[   48.982265] ACPI : EC: interrupt blocked
[   49.027946] PM: noirq suspend of devices complete after 47.016 msecs

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Andy Shevchenko]

On Tue, Jun 27, 2017 at 8:50 AM, Tom Lanyon  wrote:
> On 23 June 2017 at 12:40, Linus Torvalds  
> wrote:

> Linux during s2idle => 6% battery drain per hour.
> Win10 during sleep => 1% battery drain per hour.
>
> where Linux = 4.12-rc6 + the latest patch from your acpi-pm-test branch.
>
> So whilst s2idle halves the battery drain compared to the machine
> staying powered on, it's still significantly more draining than Win10.
> Let me know if there's any more useful analysis I can do.

Tom, thanks for this.
I would speculate that the problem might be in the certain device
drivers. It would be nice to get statistics which wakeup source
generates more hits.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Andy Shevchenko]

On Tue, Jun 27, 2017 at 8:50 AM, Tom Lanyon  wrote:
> On 23 June 2017 at 12:40, Linus Torvalds  
> wrote:

> Linux during s2idle => 6% battery drain per hour.
> Win10 during sleep => 1% battery drain per hour.
>
> where Linux = 4.12-rc6 + the latest patch from your acpi-pm-test branch.
>
> So whilst s2idle halves the battery drain compared to the machine
> staying powered on, it's still significantly more draining than Win10.
> Let me know if there's any more useful analysis I can do.

Tom, thanks for this.
I would speculate that the problem might be in the certain device
drivers. It would be nice to get statistics which wakeup source
generates more hits.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Tom Lanyon]

On 23 June 2017 at 12:40, Linus Torvalds  wrote:
> On Thu, Jun 22, 2017 at 4:56 PM, Rafael J. Wysocki  wrote:
>>
>> Some recent Dell laptops, including the XPS13 model numbers 9360 and
>> 9365, cannot be woken up from suspend-to-idle by pressing the power
>> button which is unexpected and makes that feature less usable on
>> those systems.  [ details removed ]
>
> This looks much more reasonable and more likely to work on future machines 
> too.
>
> Of course, who knows what broken machines it will cause problems on,
> but it sounds like the code now does what it's supposed to and what
> Win10 does, so maybe it JustWorks(tm). Hah.

Rafael - thanks for your efforts on this.

I wanted to provide some feedback from some quick and naive tests on
an XPS 13 9365 in case it was useful, as it seems like there's still
some way to go before matching Win10's behaviour.

Linux idling w/ screen ON => 17% battery drain per hour.
Linux idling w/ screen OFF => 12% battery drain per hour.
Linux during s2idle => 6% battery drain per hour.
Win10 during sleep => 1% battery drain per hour.

where Linux = 4.12-rc6 + the latest patch from your acpi-pm-test branch.

So whilst s2idle halves the battery drain compared to the machine
staying powered on, it's still significantly more draining than Win10.
Let me know if there's any more useful analysis I can do.

-Tom

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Tom Lanyon]

On 23 June 2017 at 12:40, Linus Torvalds  wrote:
> On Thu, Jun 22, 2017 at 4:56 PM, Rafael J. Wysocki  wrote:
>>
>> Some recent Dell laptops, including the XPS13 model numbers 9360 and
>> 9365, cannot be woken up from suspend-to-idle by pressing the power
>> button which is unexpected and makes that feature less usable on
>> those systems.  [ details removed ]
>
> This looks much more reasonable and more likely to work on future machines 
> too.
>
> Of course, who knows what broken machines it will cause problems on,
> but it sounds like the code now does what it's supposed to and what
> Win10 does, so maybe it JustWorks(tm). Hah.

Rafael - thanks for your efforts on this.

I wanted to provide some feedback from some quick and naive tests on
an XPS 13 9365 in case it was useful, as it seems like there's still
some way to go before matching Win10's behaviour.

Linux idling w/ screen ON => 17% battery drain per hour.
Linux idling w/ screen OFF => 12% battery drain per hour.
Linux during s2idle => 6% battery drain per hour.
Win10 during sleep => 1% battery drain per hour.

where Linux = 4.12-rc6 + the latest patch from your acpi-pm-test branch.

So whilst s2idle halves the battery drain compared to the machine
staying powered on, it's still significantly more draining than Win10.
Let me know if there's any more useful analysis I can do.

-Tom

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Rafael J. Wysocki]

On Friday, June 23, 2017 02:13:57 PM Rafael J. Wysocki wrote:
> On Friday, June 23, 2017 06:30:35 AM Zheng, Lv wrote:
> > Hi, Rafael
> > 
> 
> [cut]
> 
> > > 
> > > Index: linux-pm/drivers/acpi/ec.c
> > > ===
> > > --- linux-pm.orig/drivers/acpi/ec.c
> > > +++ linux-pm/drivers/acpi/ec.c
> > > @@ -1835,7 +1835,7 @@ static int acpi_ec_suspend(struct device
> > >   struct acpi_ec *ec =
> > >   acpi_driver_data(to_acpi_device(dev));
> > > 
> > > - if (ec_freeze_events)
> > > + if (acpi_sleep_no_ec_events() && ec_freeze_events)
> > >   acpi_ec_disable_event(ec);
> > >   return 0;
> > >  }
> > 
> > I just notice a slight pontential issue.
> > Should we add a similar change to acpi_ec_stop()?
> 
> Yes, it looks like that, thanks!

Actually, no, I don't think so, because acpi_ec_block_transactions() is not
used for suspend-to-idle, but I need a separate variable for that, because
pm_suspend_via_firmware() also returns "false" for hibernation.

Thanks,
Rafael

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Rafael J. Wysocki]

On Friday, June 23, 2017 02:13:57 PM Rafael J. Wysocki wrote:
> On Friday, June 23, 2017 06:30:35 AM Zheng, Lv wrote:
> > Hi, Rafael
> > 
> 
> [cut]
> 
> > > 
> > > Index: linux-pm/drivers/acpi/ec.c
> > > ===
> > > --- linux-pm.orig/drivers/acpi/ec.c
> > > +++ linux-pm/drivers/acpi/ec.c
> > > @@ -1835,7 +1835,7 @@ static int acpi_ec_suspend(struct device
> > >   struct acpi_ec *ec =
> > >   acpi_driver_data(to_acpi_device(dev));
> > > 
> > > - if (ec_freeze_events)
> > > + if (acpi_sleep_no_ec_events() && ec_freeze_events)
> > >   acpi_ec_disable_event(ec);
> > >   return 0;
> > >  }
> > 
> > I just notice a slight pontential issue.
> > Should we add a similar change to acpi_ec_stop()?
> 
> Yes, it looks like that, thanks!

Actually, no, I don't think so, because acpi_ec_block_transactions() is not
used for suspend-to-idle, but I need a separate variable for that, because
pm_suspend_via_firmware() also returns "false" for hibernation.

Thanks,
Rafael

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Rafael J. Wysocki]

On Friday, June 23, 2017 06:30:35 AM Zheng, Lv wrote:
> Hi, Rafael
> 

[cut]

> > 
> > Index: linux-pm/drivers/acpi/ec.c
> > ===
> > --- linux-pm.orig/drivers/acpi/ec.c
> > +++ linux-pm/drivers/acpi/ec.c
> > @@ -1835,7 +1835,7 @@ static int acpi_ec_suspend(struct device
> > struct acpi_ec *ec =
> > acpi_driver_data(to_acpi_device(dev));
> > 
> > -   if (ec_freeze_events)
> > +   if (acpi_sleep_no_ec_events() && ec_freeze_events)
> > acpi_ec_disable_event(ec);
> > return 0;
> >  }
> 
> I just notice a slight pontential issue.
> Should we add a similar change to acpi_ec_stop()?

Yes, it looks like that, thanks!

Rafael

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Rafael J. Wysocki]

On Friday, June 23, 2017 06:30:35 AM Zheng, Lv wrote:
> Hi, Rafael
> 

[cut]

> > 
> > Index: linux-pm/drivers/acpi/ec.c
> > ===
> > --- linux-pm.orig/drivers/acpi/ec.c
> > +++ linux-pm/drivers/acpi/ec.c
> > @@ -1835,7 +1835,7 @@ static int acpi_ec_suspend(struct device
> > struct acpi_ec *ec =
> > acpi_driver_data(to_acpi_device(dev));
> > 
> > -   if (ec_freeze_events)
> > +   if (acpi_sleep_no_ec_events() && ec_freeze_events)
> > acpi_ec_disable_event(ec);
> > return 0;
> >  }
> 
> I just notice a slight pontential issue.
> Should we add a similar change to acpi_ec_stop()?

Yes, it looks like that, thanks!

Rafael

RE: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Zheng, Lv]

Hi, Rafael

> From: Rafael J. Wysocki [mailto:r...@rjwysocki.net]
> Subject: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent 
> systems
> 
> From: Rafael J. Wysocki 
> 
> Some recent Dell laptops, including the XPS13 model numbers 9360 and
> 9365, cannot be woken up from suspend-to-idle by pressing the power
> button which is unexpected and makes that feature less usable on
> those systems.  Moreover, on the 9365 ACPI S3 (suspend-to-RAM) is
> not expected to be used at all (the OS these systems ship with never
> exercises the ACPI S3 path in the firmware) and suspend-to-idle is
> the only viable system suspend mechanism there.
> 
> The reason why the power button wakeup from suspend-to-idle doesn't
> work on those systems is because their power button events are
> signaled by the EC (Embedded Controller), whose GPE (General Purpose
> Event) line is disabled during suspend-to-idle transitions in Linux.
> That is done on purpose, because in general the EC tends to be noisy
> for various reasons (battery and thermal updates and similar, for
> example) and all events signaled by it would kick the CPUs out of
> deep idle states while in suspend-to-idle, which effectively might
> defeat its purpose.
> 
> Of course, on the Dell systems in question the EC GPE must be enabled
> during suspend-to-idle transitions for the button press events to
> be signaled while suspended at all, but fortunately there is a way
> out of this puzzle.
> 
> First of all, those systems have the ACPI_FADT_LOW_POWER_S0 flag set
> in their ACPI tables, which means that the OS is expected to prefer
> the "low power S0 idle" system state over ACPI S3 on them.  That
> causes the most recent versions of other OSes to simply ignore ACPI
> S3 on those systems, so it is reasonable to expect that it should not
> be necessary to block GPEs during suspend-to-idle on them.
> 
> Second, in addition to that, the systems in question provide a special
> firmware interface that can be used to indicate to the platform that
> the OS is transitioning into a system-wide low-power state in which
> certain types of activity are not desirable or that it is leaving
> such a state and that (in principle) should allow the platform to
> adjust its operation mode accordingly.
> 
> That interface is a special _DSM object under a System Power
> Management Controller device (PNP0D80).  The expected way to use it
> is to invoke function 0 from it on system initialization, functions
> 3 and 5 during suspend transitions and functions 4 and 6 during
> resume transitions (to reverse the actions carried out by the
> former).  In particular, function 5 from the "Low-Power S0" device
> _DSM is expected to cause the platform to put itself into a low-power
> operation mode which should include making the EC less verbose (so to
> speak).  Next, on resume, function 6 switches the platform back to
> the "working-state" operation mode.
> 
> In accordance with the above, modify the ACPI suspend-to-idle code
> to look for the "Low-Power S0" _DSM interface on platforms with the
> ACPI_FADT_LOW_POWER_S0 flag set in the ACPI tables.  If it's there,
> use it during suspend-to-idle transitions as prescribed and avoid
> changing the GPE configuration in that case.  [That should reflect
> what the most recent versions of other OSes do.]
> 
> Also modify the ACPI EC driver to make it handle events during
> suspend-to-idle in the usual way if the "Low-Power S0" _DSM interface
> is going to be used to make the power button events work while
> suspended on the Dell machines mentioned above
> 
> Link: 
> http://www.uefi.org/sites/default/files/resources/Intel_ACPI_Low_Power_S0_Idle.pdf
> Signed-off-by: Rafael J. Wysocki 
> ---
> 
> This is a replacement for https://patchwork.kernel.org/patch/9797909/
> 
> The changelog describes what is going on (and now the "Low-Power S0" _DSM
> specification is public, so it can be used officially here) and it gets the 
> job
> done on the XPS13 9360.  [The additional sort of "bonus" is that the machine
> looks "suspended" in s2idle now, as one of the effects of the _DSM appears
> to be turning off the lights in a quite literal sense.]
> 
> The patch is based on https://patchwork.kernel.org/patch/9797913/ and
> https://patchwork.kernel.org/patch/9797903/ on top of the current linux-next.
> 
> Thanks,
> Rafael
> 
> ---
>  drivers/acpi/ec.c   |2
>  drivers/acpi/internal.h |2
>  drivers/acpi/sleep.c|  107 
> ++--
>  3 files changed, 107 insertions(+), 4 deletions(-)
> 
> Index: linux-pm/drivers/acpi/sleep.c
> ===
> --- linux-pm.orig/drivers/acpi/sleep.c
> +++ linux-pm/drivers/acpi/sleep.c
> @@ -652,6 +652,84 @@ static const struct platform_suspend_ops
> 
>  static bool s2idle_wakeup;
> 
> +/*
> + * On platforms supporting the Low Power S0 Idle interface there is an 

RE: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Zheng, Lv]

Hi, Rafael

> From: Rafael J. Wysocki [mailto:r...@rjwysocki.net]
> Subject: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent 
> systems
> 
> From: Rafael J. Wysocki 
> 
> Some recent Dell laptops, including the XPS13 model numbers 9360 and
> 9365, cannot be woken up from suspend-to-idle by pressing the power
> button which is unexpected and makes that feature less usable on
> those systems.  Moreover, on the 9365 ACPI S3 (suspend-to-RAM) is
> not expected to be used at all (the OS these systems ship with never
> exercises the ACPI S3 path in the firmware) and suspend-to-idle is
> the only viable system suspend mechanism there.
> 
> The reason why the power button wakeup from suspend-to-idle doesn't
> work on those systems is because their power button events are
> signaled by the EC (Embedded Controller), whose GPE (General Purpose
> Event) line is disabled during suspend-to-idle transitions in Linux.
> That is done on purpose, because in general the EC tends to be noisy
> for various reasons (battery and thermal updates and similar, for
> example) and all events signaled by it would kick the CPUs out of
> deep idle states while in suspend-to-idle, which effectively might
> defeat its purpose.
> 
> Of course, on the Dell systems in question the EC GPE must be enabled
> during suspend-to-idle transitions for the button press events to
> be signaled while suspended at all, but fortunately there is a way
> out of this puzzle.
> 
> First of all, those systems have the ACPI_FADT_LOW_POWER_S0 flag set
> in their ACPI tables, which means that the OS is expected to prefer
> the "low power S0 idle" system state over ACPI S3 on them.  That
> causes the most recent versions of other OSes to simply ignore ACPI
> S3 on those systems, so it is reasonable to expect that it should not
> be necessary to block GPEs during suspend-to-idle on them.
> 
> Second, in addition to that, the systems in question provide a special
> firmware interface that can be used to indicate to the platform that
> the OS is transitioning into a system-wide low-power state in which
> certain types of activity are not desirable or that it is leaving
> such a state and that (in principle) should allow the platform to
> adjust its operation mode accordingly.
> 
> That interface is a special _DSM object under a System Power
> Management Controller device (PNP0D80).  The expected way to use it
> is to invoke function 0 from it on system initialization, functions
> 3 and 5 during suspend transitions and functions 4 and 6 during
> resume transitions (to reverse the actions carried out by the
> former).  In particular, function 5 from the "Low-Power S0" device
> _DSM is expected to cause the platform to put itself into a low-power
> operation mode which should include making the EC less verbose (so to
> speak).  Next, on resume, function 6 switches the platform back to
> the "working-state" operation mode.
> 
> In accordance with the above, modify the ACPI suspend-to-idle code
> to look for the "Low-Power S0" _DSM interface on platforms with the
> ACPI_FADT_LOW_POWER_S0 flag set in the ACPI tables.  If it's there,
> use it during suspend-to-idle transitions as prescribed and avoid
> changing the GPE configuration in that case.  [That should reflect
> what the most recent versions of other OSes do.]
> 
> Also modify the ACPI EC driver to make it handle events during
> suspend-to-idle in the usual way if the "Low-Power S0" _DSM interface
> is going to be used to make the power button events work while
> suspended on the Dell machines mentioned above
> 
> Link: 
> http://www.uefi.org/sites/default/files/resources/Intel_ACPI_Low_Power_S0_Idle.pdf
> Signed-off-by: Rafael J. Wysocki 
> ---
> 
> This is a replacement for https://patchwork.kernel.org/patch/9797909/
> 
> The changelog describes what is going on (and now the "Low-Power S0" _DSM
> specification is public, so it can be used officially here) and it gets the 
> job
> done on the XPS13 9360.  [The additional sort of "bonus" is that the machine
> looks "suspended" in s2idle now, as one of the effects of the _DSM appears
> to be turning off the lights in a quite literal sense.]
> 
> The patch is based on https://patchwork.kernel.org/patch/9797913/ and
> https://patchwork.kernel.org/patch/9797903/ on top of the current linux-next.
> 
> Thanks,
> Rafael
> 
> ---
>  drivers/acpi/ec.c   |2
>  drivers/acpi/internal.h |2
>  drivers/acpi/sleep.c|  107 
> ++--
>  3 files changed, 107 insertions(+), 4 deletions(-)
> 
> Index: linux-pm/drivers/acpi/sleep.c
> ===
> --- linux-pm.orig/drivers/acpi/sleep.c
> +++ linux-pm/drivers/acpi/sleep.c
> @@ -652,6 +652,84 @@ static const struct platform_suspend_ops
> 
>  static bool s2idle_wakeup;
> 
> +/*
> + * On platforms supporting the Low Power S0 Idle interface there is an ACPI
> + * device object with the PNP0D80 compatible 

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Linus Torvalds]

On Thu, Jun 22, 2017 at 4:56 PM, Rafael J. Wysocki  wrote:
>
> Some recent Dell laptops, including the XPS13 model numbers 9360 and
> 9365, cannot be woken up from suspend-to-idle by pressing the power
> button which is unexpected and makes that feature less usable on
> those systems.  [ details removed ]

This looks much more reasonable and more likely to work on future machines too.

Of course, who knows what broken machines it will cause problems on,
but it sounds like the code now does what it's supposed to and what
Win10 does, so maybe it JustWorks(tm). Hah.

Anyway - thanks.

 Linus

Re: [PATCH] ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems

[Linus Torvalds]

On Thu, Jun 22, 2017 at 4:56 PM, Rafael J. Wysocki  wrote:
>
> Some recent Dell laptops, including the XPS13 model numbers 9360 and
> 9365, cannot be woken up from suspend-to-idle by pressing the power
> button which is unexpected and makes that feature less usable on
> those systems.  [ details removed ]

This looks much more reasonable and more likely to work on future machines too.

Of course, who knows what broken machines it will cause problems on,
but it sounds like the code now does what it's supposed to and what
Win10 does, so maybe it JustWorks(tm). Hah.

Anyway - thanks.

 Linus