Just a couple of quick questions, do you have any reason to implement
this besides just trying to match the specification?

On the Surface Laptop 3 touchpad and the keyboard are provided by a separate embedded controller that also handles various other components. I think setting the latency mode could improve battery efficiency during suspend. The same should be true for touchpads configured as wake-up sources.

Blaž

On Fri, 15 Jan 2021, at 01:55, Benjamin Tissoires wrote:

Hi Blaž,

On Sat, Dec 19, 2020 at 5:55 AM Blaž Hrastnik [off-list ref] wrote:

quoted

Per Windows Precision Touchpad guidelines:

quoted

The latency mode feature report is sent by the host to a Windows
Precision Touchpad to indicate when high latency is desirable for
power savings and, conversely, when normal latency is desired for
operation.

For USB-connected Windows Precision Touchpads, this enables the device
to disambiguate between being suspended for inactivity (runtime IDLE)
and being suspended because the system is entering S3 or Connected
Standby.

The current implementation would set the latency to normal on device initialization,
but we didn't set the device to high latency on suspend.

Just a couple of quick questions, do you have any reason to implement
this besides just trying to match the specification?

Have you checked if Windows is doing the same thing?

Cheers,
Benjamin

quoted

Signed-off-by: Blaž Hrastnik <redacted>
---
 drivers/hid/hid-multitouch.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index d670bcd57..28bac0f39 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -1746,6 +1746,13 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 }

 #ifdef CONFIG_PM
+static int mt_suspend(struct hid_device *hdev, pm_message_t state)
+{
+       /* High latency is desirable for power savings during S3/S0ix */
+       mt_set_modes(hdev, HID_LATENCY_HIGH, true, true);
+       return 0;
+}
+
 static int mt_reset_resume(struct hid_device *hdev)
 {
        mt_release_contacts(hdev);

@@ -1761,6 +1768,8 @@ static int mt_resume(struct hid_device *hdev)

        hid_hw_idle(hdev, 0, 0, HID_REQ_SET_IDLE);

+       mt_set_modes(hdev, HID_LATENCY_NORMAL, true, true);
+
        return 0;
 }
 #endif

@@ -2150,6 +2159,7 @@ static struct hid_driver mt_driver = {
        .event = mt_event,
        .report = mt_report,
 #ifdef CONFIG_PM
+       .suspend = mt_suspend,
        .reset_resume = mt_reset_resume,
        .resume = mt_resume,
 #endif
--

2.29.2