Save Power By Reducing I2c Xfers With Block Reads
* [PATCH 0/2] i2c-hid: Save power by reducing i2c xfers with block reads@ :02 Sultan Alsawaf :02 ` [PATCH 1/2] i2c: designware: Only check the first byte for SMBus block read length Sultan Alsawaf :02 ` [PATCH 2/2] HID: i2c-hid: Use block reads when possible to save power Sultan Alsawaf
0 siblings, 2 replies; 28+ messages in thread
From: Sultan Alsawaf @ :02 UTC (permalink / raw)
To: Aaron Ma, Andy Shevchenko, Benjamin Tissoires, Hans de Goede,
HungNien Chen, Jarkko Nikula, Jiri Kosina, Kai-Heng Feng,
linux-i2c, linux-input, linux-kernel, Mika Westerberg,
Pavel Balan, Tin Huynh, Wolfram Sang, You-Sheng Yang
Cc: Sultan Alsawaf
From: Sultan Alsawaf
Hi,
I noticed on my Dell Precision with an i9-9880H that simply putting my
finger on the touchpad would increase my system’s power consumption by 4W, which
is quite considerable. Resting my finger on the touchpad would generate roughly i2c irqs per second, or roughly 20 i2c irqs per touchpad irq.
Upon closer inspection, I noticed that the i2c-hid driver would always transfer
the maximum report size over i2c (which is 60 bytes for my touchpad), but all of
my touchpad’s normal touch events are only 32 bytes long according to the length
byte contained in the buffer sequence.
Therefore, I was able to save about 2W of power by passing the I2C_M_RECV_LEN
flag in i2c-hid, which says to look for the payload length in the first byte of
the transfer buffer and adjust the i2c transaction accordingly. The only problem
though is that my i2c controller’s driver allows bytes other than the first one
to be used to retrieve the payload length, which is incorrect according to the
SMBus spec, and would break my i2c-hid change since not *all* of the reports
from my touchpad are conforming SMBus block reads.
This patchset fixes the I2C_M_RECV_LEN behavior in the designware i2c driver and
modifies i2c-hid to use I2C_M_RECV_LEN to save quite a bit of power. Even if the
peripheral controlled by i2c-hid doesn’t support block reads, the i2c controller
drivers should cope with this and proceed with the i2c transfer using the
original requested length.
Sultan
Sultan Alsawaf (2):
i2c: designware: Only check the first byte for SMBus block read length
HID: i2c-hid: Use block reads when possible to save power
drivers/hid/i2c-hid/i2c-hid-core.c | 3 ++-
drivers/i2c/busses/i2c-designware-master.c | 10 +++++ files changed, 7 insertions(+), 6 deletions(-) .27.0
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/2] i2c: designware: Only check the first byte for SMBus block read length :02 [PATCH 0/2] i2c-hid: Save power by reducing i2c xfers with block reads Sultan Alsawaf
@ :02 ` Sultan Alsawaf :40 ` Andy Shevchenko :22 ` Jarkko Nikula :02 ` [PATCH 2/2] HID: i2c-hid: Use block reads when possible to save power Sultan Alsawaf
1 sibling, 2 replies; 28+ messages in thread
From: Sultan Alsawaf @ :02 UTC (permalink / raw)
To: Aaron Ma, Andy Shevchenko, Benjamin Tissoires, Hans de Goede,
HungNien Chen, Jarkko Nikula, Jiri Kosina, Kai-Heng Feng,
linux-i2c, linux-input, linux-kernel, Mika Westerberg,
Pavel Balan, Tin Huynh, Wolfram Sang, You-Sheng Yang
Cc: Sultan Alsawaf
From: Sultan Alsawaf
SMBus block reads can be broken because the read function will just skip
over bytes it doesn’t like until reaching a byte that conforms to the
length restrictions for block reads. This is problematic when it isn’t
known if the incoming payload is indeed a conforming block read.
According to the SMBus specification, block reads will only send the
payload length in the first byte, so we can fix this by only considering
the first byte in a sequence for block read length purposes.
Fixes: c3ae106050b9 (“i2c: designware: Implement support for SMBus block read and write”)
Signed-off-by: Sultan Alsawaf drivers/i2c/busses/i2c-designware-master.c | 10 +++++ file changed, 5 insertions(+), 5 deletions(-)
diff –git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index d6425ad6e6a3..16d38b8fc19a a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -398,7 +398,6 @@ i2c_dw_recv_len(struct dw_i2c_dev *dev, u8 len)
len += (flags & I2C_CLIENT_PEC) ? 2 : 1;
dev->tx_buf_len = len – min_t(u8, len, dev->rx_outstanding);
msgs[dev->msg_read_idx].len = len;
– msgs[dev->msg_read_idx].flags &= ~I2C_M_RECV_LEN;
return len;
}
@@ -430,10 +429,11 @@ i2c_dw_read(struct dw_i2c_dev *dev)
u32 flags = msgs[dev->msg_read_idx].flags; regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
– /* Ensure length byte is a valid value */
– if (flags & I2C_M_RECV_LEN && tmp 0) 3 ++-
1 file changed, 2 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..4b507de48d a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -476,7 +476,8 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
if (size > ihid->bufsize)
size = ihid->bufsize;
– ret = i2c_master_recv(ihid->client, ihid->inbuf, size);
+ ret = i2c_transfer_buffer_flags(ihid->client, ihid->inbuf, size,
+ I2C_M_RD