Bug 203297

Summary:	Synaptics touchpad TM-3127 functionality broken by PCI runtime power management patch on 4.20.2
Product:	Power Management	Reporter:	Keijo Vaara (ferdasyn)
Component:	Run-Time-PM	Assignee:	Jarkko Nikula (jarkko.nikula)
Status:	CLOSED CODE_FIX
Severity:	normal	CC:	bjorn, jdelvare, rjw, rui.zhang, spam, teika
Priority:	P1
Hardware:	All
OS:	Linux
Kernel Version:	4.20.2 and later	Subsystem:
Regression:	Yes	Bisected commit-id:
Attachments:	"ls -l /sys/class/input/" "lspci -vvv" "acpidump -b" Force SMBus adapter on

Description Keijo Vaara 2019-04-14 10:29:13 UTC

After upgrading from 4.20.1 to 4.20.2 Synaptics touchpad TM-3127 on HP 250 G5 is recognized in all logs like normal, but PCI runtime power management seems to keep it from working shortly after module initialization. Issuing "modprobe -r psmouse && modprobe psmouse" fixes the issue but the device stops working again if inactive for a second or so. This is likely caused by https://lkml.org/lkml/2019/1/11/615.

Possible workarounds known to work:
1) Forcing psmouse module to use imps protocol works, but disables most of the touchpad functionality such as two finger scrolling. 
2) Disabling PCI runtime power management for the device (f.e. using tlp RUNTIME_PM_ON_BAT=on fixes the issue)
3) Downgrade kernel to 4.20.1

The bug has been reported by multiple users with different software setups, all with HP laptops (see https://bbs.archlinux.org/viewtopic.php?id=243677 for more info).

The suspect commit from kernel log:

commit 39e1be324c2f9048b013aaa190acf91b3f23b1a8
Author: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Date:   Tue Oct 23 14:45:52 2018 +0300

    PCI / PM: Allow runtime PM without callback functions
    
    commit c5eb1190074cfb14c5d9cac692f1912eecf1a5e4 upstream.
    
    a9c8088c7988 ("i2c: i801: Don't restore config registers on runtime PM")
    nullified the runtime PM suspend/resume callback pointers while keeping the
    runtime PM enabled.
    
    This caused the SMBus PCI device to stay in D0 with
    /sys/devices/.../power/runtime_status showing "error" when the runtime PM
    framework attempted to autosuspend the device.  This is due to PCI bus
    runtime PM, which checks for driver runtime PM callbacks and returns
    -ENOSYS if they are not set.
    
    Since i2c-i801.c doesn't need to do anything device-specific for runtime
    PM, Jean Delvare proposed this be fixed in the PCI core rather than adding
    dummy runtime PM callback functions in the PCI drivers.
    
    Change pci_pm_runtime_suspend()/pci_pm_runtime_resume() so they allow
    changing the PCI device power state during runtime PM transitions even if
    the driver supplies no runtime PM callbacks.
    
    This fixes the runtime PM regression on i2c-i801.c.
    
    It is not obvious why the code previously required the runtime PM
    callbacks.  The test has been there since the code was introduced by
    6cbf82148ff2 ("PCI PM: Run-time callbacks for PCI bus type").
    
    On the other hand, a similar change was done to generic runtime PM
    callbacks in 05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm
    callbacks").
    
    Fixes: a9c8088c7988 ("i2c: i801: Don't restore config registers on runtime PM")
    Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com>
    Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Reviewed-by: Jean Delvare <jdelvare@suse.de>
    Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
    Cc: stable@vger.kernel.org      # v4.18+
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Comment 1 Bjorn Helgaas 2019-04-18 12:59:43 UTC

I'm sorry for the inconvenience, and thank you very much for all the detective work so far.

If someone could test 4.20.2 with just 39e1be324c2f reverted, that would confirm whether that is the culprit.  It certainly seems reasonable, but it would be good to be certain.

If it really is a runtime PM issue (and it sounds like it is), booting with "pcie_port_pm=off" should be a workaround that doesn't give up the trackpad functionality.  Of course it will reduce battery life.

Comment 2 Jarkko Nikula 2019-04-18 14:03:20 UTC

I have to admit input drivers a bit unknown territory to me, so guesswork here. I didn't figure out the direct connection between psmouse and PCI PM but since psmouse with support for Synaptics and Synaptics  has support for SMBus transport I'm thinking can it be due the SMBus i2c_i801 runtime suspend if the those touchpads use SMBus transport?

Could you add here mapping between input devices and HW:
ls -l /sys/class/input/

If touchpad is connected to SMBus above might show that device being connected to PCI device pci0000:00:1f.3 or similar. Should match with lspci output below:

lspci -vvv |grep -A 10 SMBus

Or even better if you could attach the "lspci -vvv" showing all devices.

Continuing my speculation if SMBus runtime suspend is to blame which has 1 second autosuspend timeout if can be forced on to see does that make psmouse working by following command (where 00:1f.3 is the SMBus device found above):

echo on >/sys/bus/pci/devices/0000\:00\:1f.3/power/control

But as said just guesswork from my side.

Comment 3 Keijo Vaara 2019-04-20 14:58:46 UTC

Created attachment 282433 [details]
"ls -l /sys/class/input/"

Comment 4 Keijo Vaara 2019-04-20 14:59:29 UTC

Created attachment 282435 [details]
"lspci -vvv"

Comment 5 Keijo Vaara 2019-04-20 15:03:13 UTC

Thanks for replies. You're right, "echo on >/sys/bus/pci/devices/0000\:00\:1f.3/power/control" indeed fixes it, as does blacklisting the device at /etc/udev/rules.d/pci_pm.rules.

Logs from "ls -l /sys/class/input/", "lspci -vvv" attached.

Comment 6 Keijo Vaara 2019-04-21 16:26:36 UTC

Not sure if relevant but Synaptics rmi4 driver seems to have had some problems with PM since the touchpad is apparently connected to both PS/2 and SMBus: 

https://patchwork.kernel.org/patch/9167249/#19338855
https://lkml.org/lkml/2015/11/4/61

Comment 7 Jarkko Nikula 2019-04-23 10:36:45 UTC

Thanks. This confirms the SMBus controller runtime suspend breaks it. In that HW SMBus has the PCI PM capabilities and thus the controller is put to D3 power state when it goes to runtime suspend.

I'm hoping there is a way to prevent SMBus runtime suspend whenever rmi4 is active. Could you attach here ACPI tables to see is there any dependency defined between rmi4 and SMBus. "acpidump -b", pack resulting files and attach here.

Comment 8 Keijo Vaara 2019-04-23 15:57:21 UTC

Created attachment 282475 [details]
"acpidump -b"

Comment 9 Jarkko Nikula 2019-04-25 14:44:41 UTC

Created attachment 282529 [details]
Force SMBus adapter on

Hackish patch forcing the SMBus adapter on while there is an RMI4 device connected to an SMBus

Comment 10 Jarkko Nikula 2019-04-25 14:55:56 UTC

Could you test does attached hackish patch to rmi_smbus.c make the touchpad working? I don't consider it the final one but just an input to discussion with input driver folks.

I didn't figure out yet from ACPI tables does it describe somehow dependency between SMBus power state and touchpad but got an idea just to force SMBus controller on while there is an RMI4 device probed. I think this forcing should happen only when there is userspace listening for coordinates but would like to hear does it have any proof of concept value.

Comment 11 Keijo Vaara 2019-04-26 05:31:55 UTC

Thank you for the patch! I can confirm it fixes the issue.

Comment 12 teika kazura 2019-04-29 11:28:14 UTC

(For searchability)
Follow-up posts in kernel MLs can be found, under the title:
  "[Bug 203297] Synaptics touchpad TM-3127 functionality broken by PCI runtime power management patch on 4.20.2"
in linux-pm, linux-pci and linux-kernel.

They can be read e.g. at https://www.spinics.net/lists/linux-pci/msg82225.html

Comment 13 Jarkko Nikula 2019-05-08 07:40:32 UTC

Can this be closed now? Fix to this is found from upstream kernel tag v5.1 commit
72bfcee11cf8 ("i2c: Prevent runtime suspend of adapter when Host Notify is required") and from following linux-stable kernel tags and commits:
v5.0.14: e4f53dc963f9
v4.19.41: b19c230648b5