Bug 199057

(In reply to Robin Lee from comment #0)
> With "deep" mem_sleep, every thing works well on my ThinkPad X1 Tablet.
> But since commit included in 4.14
> e870c6c87cf9484090d28f2a68aa29e008960c93
> "ACPI / PM: Prefer suspend-to-idle over S3 on some systems",
> the preference is changed to 's2idle'.
> 
> 's2idle' seems to work by itself, but does not co-operate with the power
> button and
> gravity sensor. The most annoying thing is that the breathing LED light is
> just shut off at
> s2idle.
Do you mean, the system could not be woken up via power button, or? Not quite sure what your concern is here.
> 
> Though this can be simply fixed by setting mem_sleep_default=deep in cmd
> line,
> this breaks user experience for casual Linux user.
> 
> I am ready to provide more information.

(In reply to Chen Yu from comment #1)
> (In reply to Robin Lee from comment #0)
> > With "deep" mem_sleep, every thing works well on my ThinkPad X1 Tablet.
> > But since commit included in 4.14
> > e870c6c87cf9484090d28f2a68aa29e008960c93
> > "ACPI / PM: Prefer suspend-to-idle over S3 on some systems",
> > the preference is changed to 's2idle'.
> > 
> > 's2idle' seems to work by itself, but does not co-operate with the power
> > button and
> > gravity sensor. The most annoying thing is that the breathing LED light is
> > just shut off at
> > s2idle.
> Do you mean, the system could not be woken up via power button, or? Not
> quite sure what your concern is here.
The system sometimes could not be woken up via power button when in 's2idle' mem_sleep.
I don't actually concern with what the strange issues with 's2idle' mem_sleep. My tablet works fine with 'deep' mem_sleep.
What I actually care about is why my tablet was detected and set to 's2idle' mem_sleep by default.
> > 
> > Though this can be simply fixed by setting mem_sleep_default=deep in cmd
> > line,
> > this breaks user experience for casual Linux user.
> > 
> > I am ready to provide more information.

(In reply to Robin Lee from comment #2)
> (In reply to Chen Yu from comment #1)
> > (In reply to Robin Lee from comment #0)
> > > With "deep" mem_sleep, every thing works well on my ThinkPad X1 Tablet.
> > > But since commit included in 4.14
> > > e870c6c87cf9484090d28f2a68aa29e008960c93
> > > "ACPI / PM: Prefer suspend-to-idle over S3 on some systems",
> > > the preference is changed to 's2idle'.
> > > 
> > > 's2idle' seems to work by itself, but does not co-operate with the power
> > > button and
> > > gravity sensor. The most annoying thing is that the breathing LED light
> is
> > > just shut off at
> > > s2idle.
> > Do you mean, the system could not be woken up via power button, or? Not
> > quite sure what your concern is here.
> The system sometimes could not be woken up via power button when in 's2idle'
> mem_sleep.
> I don't actually concern with what the strange issues with 's2idle'
> mem_sleep. My tablet works fine with 'deep' mem_sleep.
> What I actually care about is why my tablet was detected and set to 's2idle'
> mem_sleep by default.
Because your system has told the linux:
(1) the ACPI_FADT_LOW_POWER_S0 flag is set in the FADT and
(2) the Low Power Idle S0 _DSM interface has been discovered and
Which means that it support s0ix AKA suspend to idle, so.. 
not sure if we should maintain a list in the code, add Rafael too.

Might have to provide  mem_sleep_default=deep in your case, because s2idle is what the BIOS told the OS to choose... If you;d like to debug on s2idle I;m ok to help too.

(In reply to Chen Yu from comment #4)
> Might have to provide  mem_sleep_default=deep in your case,

Sorry, but that is not an acceptable answer. Things should just work without the user needing to specify special kernel commandline options.

> because s2idle
> is what the BIOS told the OS to choose... If you;d like to debug on s2idle
> I;m ok to help too.

So the way I see this there are 2 possible answers here:

1) Add a dmi based list of models where s2idle is not the right choice independent of what the BIOS says; or

2) Work with Robin to make s2idle work better and resolve the issues he is seeing

I think 2 is the preferred solution because that will hopefully also make things work better on other model laptops. But if we cannot figure out how to make s2idle work as well as s3 on this laptop, I think having a dmi override is a good solution too.

(In reply to Hans de Goede from comment #5)
> (In reply to Chen Yu from comment #4)
> > Might have to provide  mem_sleep_default=deep in your case,
> 
> Sorry, but that is not an acceptable answer. Things should just work without
> the user needing to specify special kernel commandline options.
> 
> > because s2idle
> > is what the BIOS told the OS to choose... If you;d like to debug on s2idle
> > I;m ok to help too.
> 
> So the way I see this there are 2 possible answers here:
> 
> 1) Add a dmi based list of models where s2idle is not the right choice
> independent of what the BIOS says; or
> 
> 2) Work with Robin to make s2idle work better and resolve the issues he is
> seeing
> 
> I think 2 is the preferred solution because that will hopefully also make
> things work better on other model laptops. But if we cannot figure out how
> to make s2idle work as well as s3 on this laptop, I think having a dmi
> override is a good solution too.
OK, that make sense.

@Robin,
Could you please provide your dmidecode and also the acpidump file?

Created attachment 275043 [details]
acpidump output

Created attachment 275045 [details]
dmidecode output

Created attachment 275199 [details]
Blacklist s2idle for Thinkpad X1

@Robin
Could you please try this quirk?
thanks.

(In reply to Chen Yu from comment #9)
> Created attachment 275199 [details]
> Blacklist s2idle for Thinkpad X1
> 
> @Robin
> Could you please try this quirk?
> thanks.

Yes, I tested this patch and it changed the default back to 'deep' as expected.

I believe I may have the same problem with a Dell XPS 13 9370, s2idle is being detected (and there's a bunch of issues with it waking up from sleep in this mode), while deep sleep works fine when I switch to that. Let me know if you'd like me to attach my dmidecode and acpidump file too. In case it's useful, this does appear in the acpidump:

Low Power S0 Idle (V5) : 1

I should also point out, I have found my Dell XPS 13 9370 consumes about 50% of its battery overnight when left in suspend to idle. This is why I (and probably every other Dell XPS 13 9370 user should) prefer suspend to RAM. Perhaps suspend to idle isn't properly putting all devices in low power mode?  Otherwise, it seems odd that s2idle would be the preferred option, do other OS's do the same thing? (I only have Linux installed on my laptop so I can't answer that unfortunately).

Sorry, I just noticed in the patch for this issue that immediately above, there's an exception for the previous XPS 13 (9360), and an associated bugzilla ticket:

https://bugzilla.kernel.org/show_bug.cgi?id=196907

Going off the discussion in that ticket, the issue seems to be unrelated to this ticket, sorry for the noise, I'll create a new ticket.

> Perhaps suspend to idle isn't properly putting all devices in low power mode?

Please check Bug 201579 and Bug 202519. So if S0ix still not works for you after following https://01.org/blogs/qwang59/2018/how-achieve-s0ix-states-linux you need to fill bugreport about this. And if S0ix works but power consumption is still high you need to fill bugreport about this.

> do other OS's do the same thing?

Yes.

> This is why I (and probably every other Dell XPS 13 9370 user should) prefer
> suspend to RAM.

Deep sleep could work ok at first glance, for example for me mem_sleep_default=deep solve Bug 201575, however if you look more closely you could find that some functions is broken with deep sleep. For example on HP Elite x2 1013 G3 ambient light sensor is not available when deep sleep is enabled.