Bug 212537 - intel-hid 5.11 Regression: Suspend is unreliable on Dell XPS 13 9310 2n1 (works reliably on 5.10.x)
Summary: intel-hid 5.11 Regression: Suspend is unreliable on Dell XPS 13 9310 2n1 (wor...
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Platform_x86 (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Rafael J. Wysocki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-04-03 10:11 UTC by Esokrarkose
Modified: 2021-04-08 14:28 UTC (History)
7 users (show)

See Also:
Kernel Version: 5.11.x
Tree: Mainline
Regression: Yes


Attachments
dmidecode output, I only removed the serial number entries for privacy reasons (34.25 KB, text/plain)
2021-04-03 10:11 UTC, Esokrarkose
Details
[PATCH] platform/x86: intel-hid: Fix spurious wakeups caused by tablet-mode events during suspend (2.09 KB, patch)
2021-04-04 13:19 UTC, Hans de Goede
Details | Diff

Description Esokrarkose 2021-04-03 10:11:19 UTC
Created attachment 296199 [details]
dmidecode output, I only removed the serial number entries for privacy reasons

When the Dell XPS 13 9310 2-in-1 is suspended on kernel 5.11, it stays asleep if and only if the laptop has not been moved 30 seconds prior to suspending and is not moved after the machine entered sleep.
As soon as I move the device it wakes up. This is not the case on kernel 5.10.x.
Also 5.11, if you move the laptop within a 30 second timeframe before entering sleep, the device wakes up from suspend alone within the next 30 seconds.

On 5.10.x the system remains in sleep reliably. I tested the Ubuntu mainline kernel builds and every 5.11 kernel starting from 5.11 rc2 is affected (could not test 5.11 rc1 since the build for it failed).

Regardless of the kernel version, the following behavior seems interesting (and is related to the machine waking up on the 5.11 kernel):

There seems to be some sort of movement tracking in the firmware: When moving the laptop the following acpi event is emitted:

 9DBB5994-A997- 000000d0 00000000
 A6FEA33E-DABF- 000000d0 00000000

When not moving the laptop for 30 seconds then the same event gets emitted again. When moving the laptop it does not get emitted and gets emitted as soon as the laptop is not moved anymore for 30 seconds.

On the 5.11 kernel as a workaround acpi.no_ec_wakeup=1 works, however it breaks waking up with the power button which is unfortunate when the machine is in tablet mode as the power button is the only way to wake up the machine.
Comment 1 Esokrarkose 2021-04-03 10:22:15 UTC
I have to make a correction: acpi.no_ec_wakeup=1 does not work anymore (this worked on firmware 2.0 and prior, now it does not make a difference anymore).

Anyway kernel 5.10.x works perfectly and something broke with 5.11. Would be great if you could give me a list of related commits that could be to blame in order to track this down. Doing an entire bisect without any input would be hard to do given my limited amount of time.
Comment 2 Kai-Heng Feng 2021-04-03 11:15:37 UTC
Is it possible to bisect the kernel?
Comment 3 Esokrarkose 2021-04-03 22:06:39 UTC
I bisected it to 61f914256c56a39a96dc14eae9f394d35b934812 which is a rather large merge commit from platform-drivers.
How should I go on from here?
Comment 4 Mario Limonciello 2021-04-04 02:01:54 UTC
Can you please try to blacklist dell-wmi-sysman and see if your problem persists?  I don't know why that would affect things, but it's a big thing that came in that commit.
Comment 5 Esokrarkose 2021-04-04 08:43:53 UTC
dell-wmi-sysman is not related, however blacklisting intel_hid resolves the suspend issue, which is unfortunate since I do not want to loose the new SW_TABLET_MODE detection.
Comment 6 Esokrarkose 2021-04-04 09:10:10 UTC
Added the people from https://bugzilla.kernel.org/show_bug.cgi?id=207433 to CC.
Comment 7 Elia Devito 2021-04-04 11:21:56 UTC
This is probably caused by this commit https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/platform/x86/intel-hid.c?h=v5.11&id=ac32bae0008340d87328a74d7598333bf48348c7

IMO if we don't are able to correctly handle intel-hid 0xcd and 0xcc event for XPS 13 9310 the best solutions is to blacklist it to prevent intel-hid from enabling switches support for this model.

However a possible alternative solutions can be to revert this commit and add a parameter to allow user to enable support for models that are not present in the allow list.
Comment 8 Esokrarkose 2021-04-04 11:36:03 UTC
@Elia I think the events are handled correctly. SW_TABLET_MODE switch works as expected (checked with acpi_listen and gnome-shell works correctly as well, only offers display rotation when switching to tablet mode), it is just that somehow the machine wakes up when moving the device.
Comment 9 Esokrarkose 2021-04-04 11:51:32 UTC
@Elia: I am just done bisecting the platform-drivers-x86 tree and indeed ac32bae0008340d87328a74d7598333bf48348c7 is to blame.

However the switch to tablet mode and back works correctly, using acpi_listen I see

video/tabletmode TBLT 0000008A 00000001
 9DBB5994-A997- 000000d0 00000000
 A6FEA33E-DABF- 000000d0 00000000
video/tabletmode TBLT 0000008A 00000000
 9DBB5994-A997- 000000d0 00000000
 A6FEA33E-DABF- 000000d0 00000000

when switching to tablet mode and back. Gnome-shell correctly disables osk and screen rotation in laptop mode which is a huge usability win.

The problem with the 9310 2-in-1 is simply, that it emits

 9DBB5994-A997- 000000d0 00000000
 A6FEA33E-DABF- 000000d0 00000000

also when just moving / lifting up device (without any changes to the hinge position).

Just to be a little bit more precise on how those events pop up:
- If the laptop is moved, the event is emitted
- If the laptop is moved further, no event is emitted until it rests again for a period of 30 seconds
- If the laptop rests for 30 seconds that same event is emitted again

I don't know why this could be useful, but maybe Mario could ask the firmware guys at Dell?

Anyway, it would be really bad to loose support for SW_TABLET_MODE on this device, it is really useful in practice.
Comment 10 Hans de Goede 2021-04-04 13:19:18 UTC
Esokrarkose, thank you for the bug report. Elia thank you for pointing out that this is likely caused by your recent changes.

I think I know what the problem is and I've prepared a fix which I will attach here.

Esokrarkose, if you can test this fix, then I'll try to get it on its way to Linus in time for it to get included in 5.12 final (from where it will get cherry-picked into the 5.11.y stable series).

Elia, if you can review the patch that would be great.
Comment 11 Hans de Goede 2021-04-04 13:19:48 UTC
Created attachment 296223 [details]
[PATCH] platform/x86: intel-hid: Fix spurious wakeups caused by tablet-mode events during suspend
Comment 12 Esokrarkose 2021-04-04 14:05:19 UTC
Hans, thanks very much for having a look!
Your patch works fine :-), looking forward to it getting merged!
Thanks everyone for their hints and quick responses!
Comment 13 Hans de Goede 2021-04-04 18:14:44 UTC
(In reply to Esokrarkose from comment #12)
> Hans, thanks very much for having a look!
> Your patch works fine :-), looking forward to it getting merged!
> Thanks everyone for their hints and quick responses!

Thank you for testing this so quickly. I'll make sure this get sends to Linus in time for 5.12-rc7.
Comment 14 Hans de Goede 2021-04-08 14:28:26 UTC
The fix for this has been merged by Linus, closing.

Note You need to log in before you can comment on or make changes to this bug.