Messages in this thread | | | Subject | Re: [PATCH v2] HID: i2c-hid: Use block reads when possible to save power | From | Jarkko Nikula <> | Date | Wed, 17 Jun 2020 14:17:19 +0300 |
| |
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
| |