Bug 196115

(In reply to Chen Yu from comment #21 in Bug 195897)
> (In reply to Chen Yu from comment #20in Bug 195897)
> > After suspended to idle(with Rafael's patch modified for the Latitude 7275)
> > on top of BIOS 1.1.31, have to hold the power button for 6 second to
> > wakeup.Well, I think there is still problem that it takes so much time to
> > resume. Another test might be, how about waking up the system from s2idle
> by
> > rtcwake?

Waking up from s2idle with rtcwake is instantaneous.

> AFAIK, BIOS is not involved during s2idle, is it possible that the system
> has already resumed but the graphic did not show up? maybe this can be
> verified by ping the 7275 across s2idle test cycle.

The graphic stack is not the issue here. In this previous comment, BIOS was mentioned since the behavior of that system has changed between version 1.1.20 and the current latest 1.1.31.

Quoting Rafael from http://marc.info/?l=linux-pm&m=149731067302800&w=2:

"However, BIOS 1.1.31 contains additional Notify () invocations in the NEVT () method used for processing EC events (invoked by _Q66) with event values (0xCE and 0xCF) corresponding to the 5-button array power button scancodes."


and then, in http://marc.info/?l=linux-acpi&m=149731336104641&w=2 :

"intel-hid could check the 0xCE event unconditionally in the "wakeup" mode, but that will be a separate patch on top of my series."

I'm continuing the discussion about how to handle the suspend case here:

http://marc.info/?l=linux-acpi&m=149961874514664&w=2

before being able to propose a proper patch covering both suspend and wakeup scenarios.

Created attachment 257721 [details]
intel-hid: Check power button press event code unconditionally in wakeup mode

Please check if this patch makes the power button wakeup from suspend-to-idle work on your system.

I've just tested this patch on linux-pm master branch, being currently 4.13-rc2. It is working fine and makes the power button wakeup from suspend-to-idle on my Latitude 7275. Thanks Rafael.

Feel free to add:
Tested-by: Jérôme de Bretagne <jerome.debretagne@gmail.com>

Let me know if you need any other inputs for the suspend case.

Created attachment 258281 [details]
intel-hid: Check power button press event code unconditionally for the suspend case

By taking inspiration from the code from Yu for the Surface Pro 3 found in [1], I've finally been able to create a small patch that makes suspend work with the power button on the Dell Latitude 7275.

I've taken the same approach as for the wakeup case, which is to check the OxCE event unconditionally, but outside the "wakeup" mode this time. KEY_POWER is then reported using input_report_key, as done for the Surface Pro 3.

Rafael, Yu, please let me know if such a patch would be an acceptable implementation to fix this suspend issue, especially the "unconditional" part of it? Thank you.


[1] https://bugzilla.kernel.org/show_bug.cgi?id=84651

Adding you Mario to see if you have any comments on this suspend fix proposal,  since you reviewed the previous patch from Rafael for the wakeup scenario.

Also by email you previously proposed to catch and ignore the "unknown event 0xCF" that are still appearing in the logs. That would be a small (and nice) fine-tuning to add indeed.

(In reply to Jérôme de Bretagne from comment #5)
> Created attachment 258281 [details]
> intel-hid: Check power button press event code unconditionally for the
> suspend case 
> I've taken the same approach as for the wakeup case, which is to check the
> OxCE event unconditionally, but outside the "wakeup" mode this time.
> KEY_POWER is then reported using input_report_key, as done for the Surface
> Pro 3.
> 
> Rafael, Yu, please let me know if such a patch would be an acceptable
> implementation to fix this suspend issue, especially the "unconditional"
> part of it? Thank you.
> 
Looks ok for me.

@Jérôme,

Were you aware this landed?
https://github.com/torvalds/linux/commit/635173a17b0323c28a39fb06fa36d876035cd2b9#diff-8a4b68d9537ee44bd31caf61049c1c8b

I think it accomplishes your fix too doesn't it?

@Mario,

Fully aware indeed. However, the commit you mention from Rafael added support only for the wakeup case.

This follow-up proposal in comment #5 is now adding support for the suspend case, that has never been working with the hardware Power button (until this patch).

It would be great if you could have look at it, as I wonder if there could be any side-effects for other systems that expose the 5-array button. Thanks.


P.S. To make it more explicit, I've always had to trigger suspend from the command line until now on this system.

@Jérôme,

Ah OK thanks for clarifying.  I think this approach looks fine to me too.

Thanks to you both, Yu and Mario!

Created attachment 258337 [details]
intel-hid: Check power button press event code for the suspend case v2

v2:
 - Move from an unconditional to a conditional check, based on
   priv->array being NULL, to only target the affected platforms

 - On those platforms, catch also and ignore the corresponding 
   power release notification (0xCF) to stop it from being
   reported as an "unknown event" message in the logs

The first of the 2 discussed issues is now fixed upstream.

Indeed, the patch proposed in comment #3 for the wakeup case is now part of Linux 4.14, commit here for reference:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=635173a17b0323c28a39fb06fa36d876035cd2b9

Waiting for the second patch to land upstream, hopefully in Linux 4.15, before marking this bug as closed overall.

The second of the 2 discussed issues is now also fixed upstream, the second patch is part of Linux 4.15, commit here for reference:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/platform/x86/intel-hid.c?h=v4.15&id=821b85366284542e00dd834062144c637e818ee0

this can now be closed.