| Message ID | 20220928144854.5580-6-michael.zaidman@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | HID: ft260: fixes and performance improvements | expand |
Hi Michael, On Wed, 2022-09-28 at 17:48 +0300, Michael Zaidman wrote: > After: > > $ sudo ./i2cperf -d 2 -o 2 -s 128 -r 0-0xff 13 0x51 -S > > Read block via i2ctransfer by chunks > ------------------------------------------------------------------- > data rate(bps) efficiency(%) data size(B) total IOs IO size(B) > ------------------------------------------------------------------- > 49316 85 256 2 128 > > Kernel log: > > [ +1.447360] ft260_i2c_write_read: off 0x0 rlen 128 wlen 2 > [ +0.000002] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 > [ +0.001633] ft260_xfer_status: bus_status 0x41, clock 100 > [ +0.000190] ft260_xfer_status: bus_status 0x40, clock 100 > [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 128 rlen 128 flag 0x7 > [ +0.008617] ft260_raw_event: i2c resp: rep 0xde len 60 > [ +0.008033] ft260_raw_event: i2c resp: rep 0xde len 60 > [ +0.000954] ft260_raw_event: i2c resp: rep 0xd1 len 8 As the ft260 can pack up to 60 bytes into one report, would it make sense to use a multiple-of-60 size (120 or 180)? Might reduce overhead by another tiny bit ... Cheers, Enrik
On Tue, Oct 04, 2022 at 08:15:56PM +0200, Enrik Berkhan wrote: > Hi Michael, > > On Wed, 2022-09-28 at 17:48 +0300, Michael Zaidman wrote: > > After: > > > > $ sudo ./i2cperf -d 2 -o 2 -s 128 -r 0-0xff 13 0x51 -S > > > > Read block via i2ctransfer by chunks > > ------------------------------------------------------------------- > > data rate(bps) efficiency(%) data size(B) total IOs IO size(B) > > ------------------------------------------------------------------- > > 49316 85 256 2 128 > > > > Kernel log: > > > > [ +1.447360] ft260_i2c_write_read: off 0x0 rlen 128 wlen 2 > > [ +0.000002] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 > > [ +0.001633] ft260_xfer_status: bus_status 0x41, clock 100 > > [ +0.000190] ft260_xfer_status: bus_status 0x40, clock 100 > > [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 128 rlen 128 flag 0x7 > > [ +0.008617] ft260_raw_event: i2c resp: rep 0xde len 60 > > [ +0.008033] ft260_raw_event: i2c resp: rep 0xde len 60 > > [ +0.000954] ft260_raw_event: i2c resp: rep 0xd1 len 8 > > As the ft260 can pack up to 60 bytes into one report, would it make > sense to use a multiple-of-60 size (120 or 180)? Might reduce overhead > by another tiny bit ... > > Cheers, > Enrik > The size of the Read IO to perform is given to the driver by the upper layer. So it's up to him how to align the IO request size. When we read from the EEPROM, we want to issue the read requests with EEPROM page size granularity. The I2C EEPROMs page sizes are usually a power of 2 aligned. Please see the examples of reading 4K bytes from the 24C512 EEPROM first by the read requests of EEPROM page size granularity of 128 bytes and the second time of the 120 bytes (a multiple of 60 bytes granularity). In the power of 2 aligned cases, we issued lesser Read IOs (I2C combined transactions - write address read data) than when we did it with the 60 bytes alignment. Hence the performance gain. $ sudo ./i2cperf -d 2 -o 2 -s 128 -r 0-0xfff 13 0x51 -S Read block via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 49581 85 4096 32 128 $ sudo ./i2cperf -d 2 -o 2 -s 120 -r 0-0xfff 13 0x51 -S Read block via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 48816 85 4096 35 120 Thanks, Michael >
Hi Michael, On Wed, 2022-10-05 at 17:34 +0300, Michael Zaidman wrote: > On Tue, Oct 04, 2022 at 08:15:56PM +0200, Enrik Berkhan wrote: > > As the ft260 can pack up to 60 bytes into one report, would it make > > sense to use a multiple-of-60 size (120 or 180)? Might reduce overhead > > by another tiny bit ... > > > > Cheers, > > Enrik > > > The size of the Read IO to perform is given to the driver by the upper > layer. So it's up to him how to align the IO request size. > > When we read from the EEPROM, we want to issue the read requests with > EEPROM page size granularity. The I2C EEPROMs page sizes are usually a > power of 2 aligned. Understood! I only thought about the HID report sizes. With EEPROMs etc. in mind, it makes perfect sense to prefer power of 2 sizes. Thanks for also providing the test results. Cheers, Enrik
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index cb8f1782d1f0..c7c3a9d395a2 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -30,12 +30,8 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages"); #define FT260_REPORT_MAX_LENGTH (64) #define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4) -/* - * The input report format assigns 62 bytes for the data payload, but ft260 - * returns 60 and 2 in two separate transactions. To minimize transfer time - * in reading chunks mode, set the maximum read payload length to 60 bytes. - */ -#define FT260_RD_DATA_MAX (60) + +#define FT260_RD_DATA_MAX (128) #define FT260_WR_DATA_MAX (60) /*