linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* HID: multitouch: Set to high latency mode on suspend.
@ 2020-12-19  4:47 Blaž Hrastnik
  2021-01-14 16:55 ` Benjamin Tissoires
  2021-01-26 10:09 ` Jiri Kosina
  0 siblings, 2 replies; 4+ messages in thread
From: Blaž Hrastnik @ 2020-12-19  4:47 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input

Per Windows Precision Touchpad guidelines:

> 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.

Signed-off-by: Blaž Hrastnik <blaz@mxxn.io>
---
 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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: HID: multitouch: Set to high latency mode on suspend.
  2020-12-19  4:47 HID: multitouch: Set to high latency mode on suspend Blaž Hrastnik
@ 2021-01-14 16:55 ` Benjamin Tissoires
  2021-01-15  4:27   ` Blaž Hrastnik
  2021-01-26 10:09 ` Jiri Kosina
  1 sibling, 1 reply; 4+ messages in thread
From: Benjamin Tissoires @ 2021-01-14 16:55 UTC (permalink / raw)
  To: Blaž Hrastnik; +Cc: Jiri Kosina, open list:HID CORE LAYER

Hi Blaž,

On Sat, Dec 19, 2020 at 5:55 AM Blaž Hrastnik <blaz@mxxn.io> wrote:
>
> Per Windows Precision Touchpad guidelines:
>
> > 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



>
> Signed-off-by: Blaž Hrastnik <blaz@mxxn.io>
> ---
>  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
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: HID: multitouch: Set to high latency mode on suspend.
  2021-01-14 16:55 ` Benjamin Tissoires
@ 2021-01-15  4:27   ` Blaž Hrastnik
  0 siblings, 0 replies; 4+ messages in thread
From: Blaž Hrastnik @ 2021-01-15  4:27 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, open list:HID CORE LAYER

> 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 <blaz@mxxn.io> wrote:
> >
> > Per Windows Precision Touchpad guidelines:
> >
> > > 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
> 
> 
> 
> >
> > Signed-off-by: Blaž Hrastnik <blaz@mxxn.io>
> > ---
> >  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
> >
> 
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: HID: multitouch: Set to high latency mode on suspend.
  2020-12-19  4:47 HID: multitouch: Set to high latency mode on suspend Blaž Hrastnik
  2021-01-14 16:55 ` Benjamin Tissoires
@ 2021-01-26 10:09 ` Jiri Kosina
  1 sibling, 0 replies; 4+ messages in thread
From: Jiri Kosina @ 2021-01-26 10:09 UTC (permalink / raw)
  To: Blaž Hrastnik; +Cc: Benjamin Tissoires, linux-input

On Sat, 19 Dec 2020, Blaž Hrastnik wrote:

> Per Windows Precision Touchpad guidelines:
> 
> > 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.

Applied, thanks Blaž.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-01-26 10:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-19  4:47 HID: multitouch: Set to high latency mode on suspend Blaž Hrastnik
2021-01-14 16:55 ` Benjamin Tissoires
2021-01-15  4:27   ` Blaž Hrastnik
2021-01-26 10:09 ` Jiri Kosina

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).