lkml.org 
[lkml]   [2020]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] HID: i2c-hid: Use block reads when possible to save power
From
Date
On 6/16/20 6:49 PM, Sultan Alsawaf wrote:
> From: Sultan Alsawaf <sultan@kerneltoast.com>
>
> We have no way of knowing how large an incoming payload is going to be,
> so the only strategy available up until now has been to always retrieve
> the maximum possible report length over i2c, which can be quite
> inefficient. For devices that send reports in block read format, the i2c
> controller driver can read the payload length on the fly and terminate
> the i2c transaction early, resulting in considerable power savings.
>
> On a Dell Precision 15 5540 with an i9-9880H, resting my finger on the
> touchpad causes psys power readings to go up by about 4W and hover there
> until I remove my finger. With this patch, my psys readings go from 4.7W
> down to 3.1W, yielding about 1.6W in savings. This is because my
> touchpad's max report length is 60 bytes, but all of the regular reports
> it sends for touch events are only 32 bytes, so the i2c transfer is
> roughly halved for the common case.
>
> Signed-off-by: Sultan Alsawaf <sultan@kerneltoast.com>
> ---
> Jarkko, could you try this?
> drivers/hid/i2c-hid/i2c-hid-core.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 294c84e136d7..739dccfc57e1 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -472,11 +472,14 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
> int ret;
> u32 ret_size;
> int size = le16_to_cpu(ihid->hdesc.wMaxInputLength);
> + u16 flags;
>
> if (size > ihid->bufsize)
> size = ihid->bufsize;
>
> - ret = i2c_master_recv(ihid->client, ihid->inbuf, size);
> + /* Try to do a block read if the size fits in one byte */
> + flags = size > 255 ? I2C_M_RD : I2C_M_RD | I2C_M_RECV_LEN;
> + ret = i2c_transfer_buffer_flags(ihid->client, ihid->inbuf, size, flags);
> if (ret != size) {
> if (ret < 0)
> return;

This still causes a regression for me.

[ 9.457656] i2c_hid i2c-ELAN221D:00: Fetching the HID descriptor
[ 9.457663] i2c_hid i2c-ELAN221D:00: __i2c_hid_command: cmd=01 00
[ 9.458591] i2c_hid i2c-ELAN221D:00: HID Descriptor: 1e 00 00 01 31
02 02 00 03 00 43 00 04 00 ff 00 05 00 06 00 f3 04 1d 22 10 56 00 00 00 00
[ 9.459519] i2c_hid i2c-ELAN221D:00: entering i2c_hid_parse
[ 9.459526] i2c_hid i2c-ELAN221D:00: i2c_hid_hwreset
[ 9.459576] i2c_hid i2c-ELAN221D:00: i2c_hid_set_power
[ 9.459591] i2c_hid i2c-ELAN221D:00: __i2c_hid_command: cmd=05 00 00 08
[ 9.464070] i2c_hid i2c-ELAN221D:00: resetting...
[ 9.464078] i2c_hid i2c-ELAN221D:00: __i2c_hid_command: cmd=05 00 00 01
[ 9.464346] i2c_hid i2c-ELAN221D:00: __i2c_hid_command: waiting...
[ 10.497169] i2c_designware i2c_designware.3: controller timed out
[ 10.533940] i2c_designware i2c_designware.3: timeout in disabling adapter
[ 14.528677] i2c_hid i2c-ELAN221D:00: __i2c_hid_command: finished.
[ 14.528695] i2c_hid i2c-ELAN221D:00: failed to reset device.
[ 14.536125] i2c_hid i2c-ELAN221D:00: i2c_hid_set_power
[ 14.536141] i2c_hid i2c-ELAN221D:00: __i2c_hid_command: cmd=05 00 01 08
[ 14.556335] i2c_designware i2c_designware.3: timeout waiting for bus
ready
[ 14.565086] i2c_hid i2c-ELAN221D:00: failed to change power setting.
[ 15.584374] i2c_hid i2c-ELAN221D:00: i2c_hid_hwreset
[ 15.584395] i2c_hid i2c-ELAN221D:00: i2c_hid_set_power
[ 15.584410] i2c_hid i2c-ELAN221D:00: __i2c_hid_command: cmd=05 00 00 08
[ 15.605683] i2c_designware i2c_designware.3: timeout waiting for bus
ready
[ 15.614304] i2c_hid i2c-ELAN221D:00: failed to change power setting.
...

--
Jarkko

\
 
 \ /
  Last update: 2020-06-17 13:28    [W:1.332 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site