Re: [PATCH v2] HID: i2c-hid: Use block reads when possible to save power

From: Andi Shyti
Date: Tue Jun 16 2020 - 13:19:03 EST


Hi Andy,

> > 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.
>
> > + /* 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;
>
> AFAIR SMBus specification tells about 256. Why 255?
>
> Andi, am I correct?

Actually the SMBUS 3.0 protocol from 2015[*] says 255:

"
D.6 255 Bytes in Process Call

The maximum number of bytes allowed in the Block Write-Block Read
Process Call (Section 6.5.8) was increased from 32 to 255.
"

But why does it matter... I see the patch is detatching itself
from smbus.

And, actually, I wonder if this is the right way to fix it, isn't
it better to fix smbus instead?

I have a patch ready that fixes the smbus transfer size, perhaps
I should rebase, test and send it.

Andi

[*] http://smbus.org/specs/SMBus_3_0_20141220.pdf