[PATCH] i2c: designware: size the RX FIFO threshold to the queued transfer

[PATCH] i2c: designware: size the RX FIFO threshold to the queued transfer

[Navon John Lukose]

i2c_dw_xfer_msg() leaves DW_IC_RX_TL at the 0 that i2c_dw_configure_mode()
writes, so the controller raises RX_FULL once per received byte. Program
RX_TL from the reads already queued, capped at half the FIFO until the last
message is queued, so there is room for the rest.

On an Arrow Lake-H LPSS core with rx_fifo_depth 32, a 22-byte HID report
costs 2.00 interrupts instead of 21.43, and a 452-byte descriptor 0.14
interrupts per byte instead of 1.64. Mean HIDIOCGINPUT latency rises from
2522 to 3268 us; holding /dev/cpu_dma_latency at 0 removes 94% of that.

Assisted-by: LLM
Signed-off-by: Navon John Lukose <navonjohnlukose@gmail.com>
---
Tested on my machine, a Lenovo Yoga Pro 7 14IAH10, and one controller, an
Intel LPSS core with rx_fifo_depth 32, where it has been the daily driver
for the past five days.

The half-FIFO cap matches DW_IC_TX_TL. Untested: a RECV_LEN continuation
longer than one byte, and cores with rx_fifo_depth below 4.

The two interrupts left on a short read are a TX_EMPTY that only runs the
state machine and one RX_FULL coalesced with STOP_DET.

psys power fell about 1.25 W in one run. It is out of the changelog because
the figure depends on how deep the platform idles.

 drivers/i2c/busses/i2c-designware-master.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index a1bcc37..f4ad207 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -378,7 +378,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 {
 	struct i2c_msg *msgs = dev->msgs;
 	u32 intr_mask;
-	int tx_limit, rx_limit;
+	int tx_limit, rx_limit, rx_tl;
 	u32 buf_len = dev->tx_buf_len;
 	u8 *buf = dev->tx_buf;
 	bool need_restart = false;
@@ -484,6 +484,19 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	if (dev->msg_err)
 		intr_mask = 0;
 
+	/*
+	 * Size the RX FIFO threshold to the reads already queued, so the
+	 * controller raises one RX_FULL for the whole burst instead of one per
+	 * received byte. While messages remain to be queued, cap it so RX_FULL
+	 * still arrives in time to drain the FIFO and let the next TX_EMPTY
+	 * queue the rest.
+	 */
+	rx_tl = dev->rx_outstanding;
+	if (dev->msg_write_idx < dev->msgs_num)
+		rx_tl = min_t(int, rx_tl, dev->rx_fifo_depth / 2);
+
+	regmap_write(dev->map, DW_IC_RX_TL, rx_tl ? rx_tl - 1 : 0);
+
 	__i2c_dw_write_intr_mask(dev, intr_mask);
 }
 
-- 
2.55.0

Re: [PATCH] i2c: designware: size the RX FIFO threshold to the queued transfer

[Mika Westerberg]

Hi,

On Sun, Sep 20, 2026 at 04:56:47AM +0530, Navon John Lukose wrote:
> i2c_dw_xfer_msg() leaves DW_IC_RX_TL at the 0 that i2c_dw_configure_mode()
> writes, so the controller raises RX_FULL once per received byte. Program
> RX_TL from the reads already queued, capped at half the FIFO until the last
> message is queued, so there is room for the rest.
> 
> On an Arrow Lake-H LPSS core with rx_fifo_depth 32, a 22-byte HID report
> costs 2.00 interrupts instead of 21.43, and a 452-byte descriptor 0.14
> interrupts per byte instead of 1.64. Mean HIDIOCGINPUT latency rises from
> 2522 to 3268 us; holding /dev/cpu_dma_latency at 0 removes 94% of that.

I'm slightly worried about the latency increase here and the fact that
HIDIOCGINPUT is pretty much same as HID_REQ_GET_REPORT so it's not only
hidraw that is affected but everything else using HID_REQ_GET_REPORT as
well. We cannot expect regular user knows or should be using
/dev/cpu_dma_latency (and it may be bad thing to keep the CPUs from using
certain C-states).

Re: [PATCH] i2c: designware: size the RX FIFO threshold to the queued transfer

[Navon John Lukose]

On Mon, Sep 21, 2026 at 02:05:26PM +0200, Mika Westerberg wrote:
> I'm slightly worried about the latency increase here and the fact that
> HIDIOCGINPUT is pretty much same as HID_REQ_GET_REPORT so it's not only
> hidraw that is affected but everything else using HID_REQ_GET_REPORT as
> well.

You are right, it is every HID_REQ_GET_REPORT user, not just hidraw.

The main motivation for this patch was cutting the wakeups from the
interrupts, and I should have made that clearer. The /dev/cpu_dma_latency
line was just to measure where the extra latency comes from.

What the wakeups cost, measured with a 21-byte HID GET_REPORT at 142 Hz
pinned to one CPU, display off, 32 reps per arm in randomized order:

                        psys W
    idle             1.87 +/- 0.28
    patched          2.53 +/- 0.17
    stock            4.11 +/- 0.44

Against the idle floor the polling costs 2.24 W without the patch and
0.66 W with it. Paired saving 1.58 W, 95% CI [1.42, 1.75].

So the added exit latency is the package idling deeper than it can when
the controller interrupts once per received byte.

Re: [PATCH] i2c: designware: size the RX FIFO threshold to the queued transfer

[Mika Westerberg]

Hi,

On Mon, Sep 21, 2026 at 10:34:56PM +0530, Navon John Lukose wrote:
> On Mon, Sep 21, 2026 at 02:05:26PM +0200, Mika Westerberg wrote:
> > I'm slightly worried about the latency increase here and the fact that
> > HIDIOCGINPUT is pretty much same as HID_REQ_GET_REPORT so it's not only
> > hidraw that is affected but everything else using HID_REQ_GET_REPORT as
> > well.
> 
> You are right, it is every HID_REQ_GET_REPORT user, not just hidraw.
> 
> The main motivation for this patch was cutting the wakeups from the
> interrupts, and I should have made that clearer. The /dev/cpu_dma_latency
> line was just to measure where the extra latency comes from.
> 
> What the wakeups cost, measured with a 21-byte HID GET_REPORT at 142 Hz
> pinned to one CPU, display off, 32 reps per arm in randomized order:
> 
>                         psys W
>     idle             1.87 +/- 0.28
>     patched          2.53 +/- 0.17
>     stock            4.11 +/- 0.44
> 
> Against the idle floor the polling costs 2.24 W without the patch and
> 0.66 W with it. Paired saving 1.58 W, 95% CI [1.42, 1.75].
> 
> So the added exit latency is the package idling deeper than it can when
> the controller interrupts once per received byte.

Yes power savings are always good but in this case it also affects the
latency visible to the user so we don't want devices like touchpads become
"sluggish" either. Do you see any such issues with the devices when you
have this enabled and not using /dev/cpu_dma_latency or any other interface
to limit the CPU low power states?