From 459e3576eb44cff1bf9fccb708d97af2946ef85d Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Mon, 20 Jul 2026 11:37:22 +0300 Subject: [PATCH] =?UTF-8?q?Jaguar=20USB=20RX:=2016=20KB=20URBs=20+=20match?= =?UTF-8?q?ing=20agg=20caps=20=E2=80=94=20restore=20the=20MediaTek=20host?= =?UTF-8?q?=20fix=20(#314)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some MediaTek Android xhci/usbfs stacks (Dimensity 810, Helio G99, MT6765) never complete a bulk-IN read larger than 16 KB — LIBUSB_ERROR_TIMEOUT forever, zero RX with a green init (OpenIPC/PixelPilot#6). floppyhammer's #19 fixed this with 16 KB host reads plus ≤16 KB device-side RX aggregation; the async-ring rewrites (#158/#164/#168) and the #213 transport split (40e3a2a) kept the aggregation half but regressed the host reads to 32 KB URBs on every 11ac generation. PixelPilot#104 shipped the first Android build of the async ring and the no-video reports came straight back. - Jaguar1/2/3 RX rings: 16 KB URBs (DeviceConfig::Rx::urb_bytes, env DEVOURER_RX_URB_BYTES, clamped >= 4096). Free on healthy hosts: with aggregates capped ≤16 KB a 32 KB URB never filled past 16 KB anyway — each aggregate ends its URB via short packet. - Jaguar2/3 halmac agg page-threshold 0x05 (20 KB) -> 0x03 (≤12 KB): an aggregate must never span two URBs or the next_offset walk breaks. Deliberate divergence from the kernel's 0x05, documented at both sites. - Kestrel untouched: the 8852C RXAGG LEN_TH (~20 KB) requires its 32 KB ring; the knob is documented as 11ac-only. - Docs: CLAUDE.md hardware gotcha + performance-tuning.md lever entry. ctest 36/36. Desktop bench (regress 2x2, goodput A/B) and on-device MTK validation via PixelPilot#6 volunteers tracked in #314. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 9 +++++++++ docs/performance-tuning.md | 15 +++++++++++++++ examples/common/env_config.cpp | 2 ++ src/DeviceConfig.h | 10 ++++++++++ src/jaguar1/RtlJaguarDevice.cpp | 12 +++++++++++- src/jaguar2/HalmacJaguar2MacInit.cpp | 7 ++++++- src/jaguar2/RtlJaguar2Device.cpp | 9 ++++++++- src/jaguar3/HalmacJaguar3MacInit.cpp | 11 ++++++++--- src/jaguar3/RtlJaguar3Device.cpp | 9 ++++++++- 9 files changed, 77 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 19e82f10..fd961282 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -487,6 +487,15 @@ generators, never the output files. only helps when firmware state is intact. - **The chip retains state across soft re-init** — cold-bisect hardware problems with a VBUS power-cycle, not a re-run. +- **MediaTek Android hosts cap bulk-IN reads at 16 KB**: some MTK xhci/usbfs + stacks (Dimensity 810, Helio G99, MT6765) never complete a larger bulk-IN + transfer — `LIBUSB_ERROR_TIMEOUT` forever, zero RX with a green init + (OpenIPC/PixelPilot#6). The 11ac RX rings therefore post 16 KB URBs paired + with ≤16 KB device-side aggregation caps (`DeviceConfig::Rx::urb_bytes`, + `DEVOURER_RX_URB_BYTES`); raise both together or MTK hosts go silent, and + never let an aggregate exceed the URB size or the parse walk breaks. + Kestrel is exempt (8852C RXAGG LEN_TH ~20 KB needs its 32 KB ring) and + unvalidated on MTK hosts. ## TX path diff --git a/docs/performance-tuning.md b/docs/performance-tuning.md index 5e1a6f1e..6d480311 100644 --- a/docs/performance-tuning.md +++ b/docs/performance-tuning.md @@ -143,6 +143,21 @@ does much — the structural win above is already the dominant effect: structurally-correct, low-risk path (and a base to develop further), not a measured CPU win at these rates. +- **RX URB size** (`rx.urb_bytes` / `DEVOURER_RX_URB_BYTES`, default 16 KB on + the 11ac generations) is a compatibility constraint, not a throughput lever. + Some MediaTek Android xhci/usbfs stacks never complete a bulk-IN read larger + than 16 KB — `LIBUSB_ERROR_TIMEOUT` forever, zero RX with a green init + (OpenIPC/PixelPilot#6; fixed by floppyhammer's 16 KB reads in #19, regressed + to 32 KB by the #213 transport split, restored since). Bigger URBs buy + nothing on healthy hosts either: the device-side RX aggregation thresholds + (Jaguar1 `rxagg_usb_size` 0x3/0x1, Jaguar2/3 halmac agg page-th 0x03) cap + every aggregate at ≤16 KB, so each aggregate ends its URB via short packet + and a 32 KB buffer never filled past 16 KB anyway. If you raise the URB + size, raise the aggregation thresholds with it — but never let an aggregate + exceed the URB size, or a block spans two URBs and the RND8/next_offset + parse walk breaks. Kestrel keeps its own 32 KB ring (8852C RXAGG LEN_TH is + ~20 KB) and does not consume this knob. + The honest summary: devourer's CPU advantage is the kernel stack it *doesn't* run, and that is already captured by the raw-injection design. There is no large additional CPU lever to pull on the 8812A on top of it. diff --git a/examples/common/env_config.cpp b/examples/common/env_config.cpp index c52f00c6..5d824261 100644 --- a/examples/common/env_config.cpp +++ b/examples/common/env_config.cpp @@ -55,6 +55,8 @@ devourer::DeviceConfig devourer_config_from_env() { cfg.rx.path_spec = e; if (env_long("DEVOURER_RX_URBS", &v)) cfg.rx.urbs = static_cast(v); + if (env_long("DEVOURER_RX_URB_BYTES", &v)) + cfg.rx.urb_bytes = static_cast(v); cfg.rx.phy_status_8821c = !env_flag("DEVOURER_8821C_NO_PHYST"); cfg.rx.abs_noise_floor = env_flag("DEVOURER_RX_NOISE_FLOOR"); if (env_long("DEVOURER_IGI", &v)) diff --git a/src/DeviceConfig.h b/src/DeviceConfig.h index 78e2e4d6..455f415e 100644 --- a/src/DeviceConfig.h +++ b/src/DeviceConfig.h @@ -92,6 +92,16 @@ struct DeviceConfig { /* env: DEVOURER_RX_URBS — Jaguar1 async bulk-IN URB queue depth * (default 8, clamped to >= 1). */ std::optional urbs; + /* env: DEVOURER_RX_URB_BYTES — per-URB bulk-IN buffer size on the 11ac + * generations (default 16384, clamped to >= 4096). Keep it at 16 KB unless + * you also raise the device-side RX aggregation thresholds: some MediaTek + * Android xhci hosts never complete a bulk-IN read larger than 16 KB + * (LIBUSB_ERROR_TIMEOUT forever, zero RX — OpenIPC/PixelPilot#6, fixed by + * #19, regressed by the #213 transport split), and the device-side + * aggregation caps (J1 0x3/4KB-pages, J2/J3 0x3) mean a bigger URB never + * fills past 16 KB anyway. Kestrel ignores this knob: the 8852C RXAGG + * LEN_TH is ~20 KB and its ring must hold a full aggregate (32 KB). */ + std::optional urb_bytes; /* env: DEVOURER_8821C_NO_PHYST (inverted) — 8821C: prepend the 32-byte * PHY-status to RX frames (per-frame RSSI/SNR/EVM). Disable only for the * leanest possible RX path. */ diff --git a/src/jaguar1/RtlJaguarDevice.cpp b/src/jaguar1/RtlJaguarDevice.cpp index d0c91faf..39e9ee67 100644 --- a/src/jaguar1/RtlJaguarDevice.cpp +++ b/src/jaguar1/RtlJaguarDevice.cpp @@ -1362,6 +1362,16 @@ void RtlJaguarDevice::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { int rx_urbs = _cfg.rx.urbs.value_or(8); if (rx_urbs < 1) rx_urbs = 1; + /* 16 KB per URB, NOT more: some MediaTek Android xhci hosts never complete + * a bulk-IN read larger than 16 KB — LIBUSB_ERROR_TIMEOUT forever, zero RX + * (OpenIPC/PixelPilot#6). floppyhammer's #19 fixed this with 16 KB reads + + * the ≤16 KB rxagg_usb_size caps (RtlAdapter.cpp); the #213 transport split + * kept the rxagg half but regressed the read size to 32 KB. Free on healthy + * hosts: aggregates are capped ≤16 KB, so a 32 KB URB never filled past + * 16 KB anyway — each aggregate ends its URB via short packet. */ + int rx_urb_bytes = _cfg.rx.urb_bytes.value_or(16 * 1024); + if (rx_urb_bytes < 4096) + rx_urb_bytes = 4096; /* Closed-loop CFO tracking on the receiver (#217): tick the controller on a * ~2 s cadence from the RX loop, mirroring Jaguar2/3. The crystal-cap field @@ -1437,7 +1447,7 @@ void RtlJaguarDevice::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { _packetProcessor(p); } }; - _device.bulk_read_async_loop(32 * 1024, rx_urbs, on_data, [this]() -> bool { + _device.bulk_read_async_loop(rx_urb_bytes, rx_urbs, on_data, [this]() -> bool { return should_stop || g_devourer_should_stop; }); diff --git a/src/jaguar2/HalmacJaguar2MacInit.cpp b/src/jaguar2/HalmacJaguar2MacInit.cpp index 8c45db33..138e22e1 100644 --- a/src/jaguar2/HalmacJaguar2MacInit.cpp +++ b/src/jaguar2/HalmacJaguar2MacInit.cpp @@ -684,8 +684,13 @@ void HalmacJaguar2MacInit::init_usb_cfg() { _device.rtw_write8(REG_TXDMA_PQ_MAP, agg_enable); _device.rtw_write8(kRxdmaAggPgTh + 3, dma_usb_agg); const uint8_t ss = (_device.rtw_read8(REG_SYS_CFG2 + 3) == 0x20); + /* Agg page threshold 0x03 (4 KB pages -> ≤12 KB aggregates), down from the + * halmac default 0x05 (20 KB): the RX ring posts 16 KB URBs (MediaTek + * Android hosts can't complete larger bulk-IN reads — OpenIPC/PixelPilot#6, + * DeviceConfig::Rx::urb_bytes) and an aggregate must never span two URBs or + * the next_offset walk breaks. Mirrors floppyhammer's Jaguar1 fix (#19). */ _device.rtw_write16(kRxdmaAggPgTh, - static_cast(0x05 | ((ss ? 0x0A : 0x20) << 8))); + static_cast(0x03 | ((ss ? 0x0A : 0x20) << 8))); _logger->info("Jaguar2: init_usb_cfg (RXDMA_MODE=0x{:02x}, RX-agg EN)", v); } diff --git a/src/jaguar2/RtlJaguar2Device.cpp b/src/jaguar2/RtlJaguar2Device.cpp index e958fc13..af913234 100644 --- a/src/jaguar2/RtlJaguar2Device.cpp +++ b/src/jaguar2/RtlJaguar2Device.cpp @@ -562,7 +562,14 @@ void RtlJaguar2Device::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { off += f.next_offset; } }; - _device.bulk_read_async_loop(32 * 1024, 8, on_data, [this]() -> bool { + /* ≤16 KB per URB — some MediaTek Android xhci hosts never complete larger + * bulk-IN reads (OpenIPC/PixelPilot#6; see DeviceConfig::Rx::urb_bytes). + * Paired with the ≤16 KB agg threshold in HalmacJaguar2MacInit::init_usb_cfg + * so an aggregate never spans two URBs and breaks the next_offset walk. */ + int rx_urb_bytes = _cfg.rx.urb_bytes.value_or(16 * 1024); + if (rx_urb_bytes < 4096) + rx_urb_bytes = 4096; + _device.bulk_read_async_loop(rx_urb_bytes, 8, on_data, [this]() -> bool { return _rx_stop || g_devourer_should_stop; }); stop_dig(); diff --git a/src/jaguar3/HalmacJaguar3MacInit.cpp b/src/jaguar3/HalmacJaguar3MacInit.cpp index 9db79d86..a1b14856 100644 --- a/src/jaguar3/HalmacJaguar3MacInit.cpp +++ b/src/jaguar3/HalmacJaguar3MacInit.cpp @@ -432,8 +432,13 @@ void HalmacJaguar3MacInit::init_usb_cfg() { * RX-DMA engine does not flush received frames into the bulk-IN endpoint unless * aggregation is enabled (the 8822C does by default), so without this the EU * delivers zero RX bytes (usbmon: bulk-IN URBs complete empty) even with the RF - * in RX mode. Matches the kernel's values: 0x280=size0x5|timeout0x20<<8, - * 0x10C[2] BIT_RXDMA_AGG_EN, 0x283 USB-mode (bit7 clear). */ + * in RX mode. Kernel values: 0x280=size0x5|timeout0x20<<8, 0x10C[2] + * BIT_RXDMA_AGG_EN, 0x283 USB-mode (bit7 clear). We deliberately diverge on + * the size: 0x03 (4 KB pages -> ≤12 KB aggregates) instead of the kernel's + * 0x05 (20 KB), because the RX ring posts 16 KB URBs (MediaTek Android + * hosts can't complete larger bulk-IN reads — OpenIPC/PixelPilot#6, + * DeviceConfig::Rx::urb_bytes) and an aggregate must never span two URBs or + * the next_offset walk breaks. Mirrors floppyhammer's Jaguar1 fix (#19). */ constexpr uint16_t kTxdmaPqMap = 0x010C; constexpr uint16_t kRxdmaAggPgTh = 0x0280; uint8_t dma_usb_agg = _device.rtw_read8(kRxdmaAggPgTh + 3); @@ -445,7 +450,7 @@ void HalmacJaguar3MacInit::init_usb_cfg() { _device.rtw_write8(kTxdmaPqMap, agg_enable); _device.rtw_write8(kRxdmaAggPgTh + 3, dma_usb_agg); _device.rtw_write16(kRxdmaAggPgTh, - static_cast(0x05 | (0x20 << 8))); /* size|timeout */ + static_cast(0x03 | (0x20 << 8))); /* size|timeout */ _logger->info("Jaguar3: init_usb_cfg (RXDMA_MODE=0x{:02x}, RX-agg EN) — RX DMA to bulk-IN", v); diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index b73c4568..6d50aca6 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -352,7 +352,14 @@ void RtlJaguar3Device::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { /* _rx_loop_active was set on entry (bulk-IN handover from the coex thread's * C2H drain — its one in-flight 200 ms bulk_read_raw may steal at most one * completion, harmless). Give the endpoint back on exit. */ - _device.bulk_read_async_loop(32 * 1024, 8, on_data, [this]() -> bool { + /* ≤16 KB per URB — some MediaTek Android xhci hosts never complete larger + * bulk-IN reads (OpenIPC/PixelPilot#6; see DeviceConfig::Rx::urb_bytes). + * Paired with the ≤16 KB agg threshold in HalmacJaguar3MacInit::init_usb_cfg + * so an aggregate never spans two URBs and breaks the next_offset walk. */ + int rx_urb_bytes = _cfg.rx.urb_bytes.value_or(16 * 1024); + if (rx_urb_bytes < 4096) + rx_urb_bytes = 4096; + _device.bulk_read_async_loop(rx_urb_bytes, 8, on_data, [this]() -> bool { return _rx_stop || g_devourer_should_stop; }); _rx_loop_active = false;