From 396e526e8ec0c152907fdba5ea591bcecef08399 Mon Sep 17 00:00:00 2001 From: snokvist Date: Sun, 16 Aug 2026 07:16:20 +0200 Subject: [PATCH 1/8] rtl8733b: runtime TX-power offset on the TSSI target (#1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rtl8733bDevice overrode none of the IRtlDevice runtime-power family, so every SetTxPowerOffsetQdb landed on the base class's `(void)qdb; return 0`. Measured on a CV610 bench 2026-08-14: 18 dB of commanded offset moved the air by nothing, EVM bit-identical across every rung, while a matched 8822EU control on the same receiver moved 5.6 dB for a 6 dB command and railed. That the knob was unported is documented; the defect is that `return 0` is indistinguishable from a successful zero-offset apply, and the consumer's state came back {"applied_qdb":0,"saturated_low":false} — a healthy actuator with travel remaining. On a TSSI-offset PG unit the closed loop IS the TX-power control, so the actuator is its per-rate target table: the five packed dwords at 0x3a00..0x3a10, rewritten IN PLACE with tracking left enabled. That is the shape #389 validated and fast_retune already uses for its per-channel rewrite, not the ~165 ms disable/re-enable dance. The offset caps first and shifts second — clamp(min(target, ceiling) + qdb). A lowered ceiling would move only the rates sitting above it and silently flatten the calibrated spread that src/TxPower.h promises to preserve; the selftest's shape cell fails on exactly that mistake. Capabilities are the dBm-target model Kestrel already reports (index_max 0, one qdB per step), range [-64, 0]: offset 0 is kSafeTssiTargetQdbm8733b = 16 dBm, the highest level this backend has characterised, so the knob can only back off and no un-measured power increase is reachable through the API. step_measured stays false — the quarter-dB step is what the hardware target table is denominated in, not a slope anyone has measured on air for this part. Caps stay static and EFUSE-free per the GetAdapterCaps contract; a flat-PG unit's lack of an actuator surfaces as a loud refusal from SetTxPowerOffsetQdb and in GetTxPowerState, not by mutating capabilities. The offset is sticky by construction: configure_tx_power folds it back in on every channel set, and FastRetune passes it to the in-place hop rewrite — without that a hop would recompute targets from the bare ceiling and walk the caller's backoff back up. GetTxPowerState reports hw_readback from an actual 0x3a00 read, so the offset is confirmed against the chip rather than echoed from a shadow that always agrees with itself. Still not ported, deliberately: SetTxPowerIndexOverride, SetTxPowerRateDiffs and ReApplyTxPower. kSafeTxAgcIndex8733b was witnessed unable to carry HT at all (MCS7, 300/300 submitted, 0 captured, twice) and no dB-per-step slope has been measured for the flat index. ctest 54/54. On-air slope measurement pending — step_measured flips on that evidence, not before. Co-Authored-By: Claude Opus 5 (1M context) --- CMakeLists.txt | 6 ++ src/rtl8733b/Phy8733b.cpp | 129 +++++++++++++++++++++++--- src/rtl8733b/Phy8733b.h | 50 +++++++++- src/rtl8733b/Rtl8733bDevice.cpp | 146 ++++++++++++++++++++++++++++- src/rtl8733b/Rtl8733bDevice.h | 15 +++ tests/rtl8733b_txpwr_selftest.cpp | 148 ++++++++++++++++++++++++++++++ 6 files changed, 472 insertions(+), 22 deletions(-) create mode 100644 tests/rtl8733b_txpwr_selftest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index db93f4f..fbf4798 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1059,6 +1059,12 @@ if(DEVOURER_8733B) ) target_link_libraries(Rtl8733bTxDescSelftest PRIVATE devourer) add_test(NAME rtl8733b_tx_desc COMMAND Rtl8733bTxDescSelftest) + + add_executable(Rtl8733bTxPwrSelftest + tests/rtl8733b_txpwr_selftest.cpp + ) + target_link_libraries(Rtl8733bTxPwrSelftest PRIVATE devourer) + add_test(NAME rtl8733b_txpwr COMMAND Rtl8733bTxPwrSelftest) endif() # Headless guard for the per-UE RX attribution seed (src/cell/UeRxAttribution.h) diff --git a/src/rtl8733b/Phy8733b.cpp b/src/rtl8733b/Phy8733b.cpp index 7b2f9c9..58b6e95 100644 --- a/src/rtl8733b/Phy8733b.cpp +++ b/src/rtl8733b/Phy8733b.cpp @@ -428,7 +428,8 @@ bool Phy8733b::parse_tx_power_targets(const uint32_t *table, size_t len, std::optional> Phy8733b::tssi_rate_offsets(const TxPowerTargets8733b &targets, uint8_t band, - uint8_t path, uint8_t max_target_qdbm) { + uint8_t path, uint8_t max_target_qdbm, + int offset_qdb, TssiOffsetSat8733b *sat) { if (band > 1 || path > 1 || !targets.present[band][path]) return std::nullopt; std::array offsets{}; @@ -439,9 +440,30 @@ Phy8733b::tssi_rate_offsets(const TxPowerTargets8733b &targets, uint8_t band, continue; // CCK is not defined outside 2.4 GHz. return std::nullopt; } - offsets[rate] = static_cast(std::clamp( - static_cast((std::min)(target, max_target_qdbm)) - 64, -128, - 127)); + /* Cap first, then shift: the ceiling is the safety limit on absolute + * power, the offset is the caller's relative move below it. A rate the + * ceiling already pulled down keeps its calibrated distance from the + * others through the shift. */ + int shifted = + static_cast((std::min)(target, max_target_qdbm)) + offset_qdb; + if (shifted < 0) { + /* Below 0 qdBm the target has no meaning left — that is the low rail, and + * a closed-loop controller needs to be told rather than handed a wrapped + * value. */ + shifted = 0; + if (sat) + sat->low = true; + } + /* Floored at 0 above, so the delta cannot go below -64: only the int8 + * field's positive end needs clamping, and it is reachable only with a + * ceiling above the 64 qdBm anchor. */ + int delta = shifted - 64; + if (delta > 127) { + delta = 127; + if (sat) + sat->high = true; + } + offsets[rate] = static_cast(delta); } return offsets; } @@ -449,12 +471,13 @@ Phy8733b::tssi_rate_offsets(const TxPowerTargets8733b &targets, uint8_t band, std::optional Phy8733b::tssi_bb_plan(const TxPowerTargets8733b &targets, uint8_t channel, uint8_t rfe_type, uint8_t path, - uint8_t max_target_qdbm) { + uint8_t max_target_qdbm, int offset_qdb, + TssiOffsetSat8733b *sat) { if (path > 1 || !legal_20mhz(channel)) return std::nullopt; const uint8_t band = channel <= 14 ? 0 : 1; const auto offsets = - tssi_rate_offsets(targets, band, path, max_target_qdbm); + tssi_rate_offsets(targets, band, path, max_target_qdbm, offset_qdb, sat); if (!offsets) return std::nullopt; @@ -1277,11 +1300,12 @@ bool Phy8733b::audit_tssi_enable(SelectedChannel channel, bool Phy8733b::enable_tssi_tracking(SelectedChannel channel, const EfuseInfo &efuse, - uint8_t max_target_qdbm) { + uint8_t max_target_qdbm, int offset_qdb) { const auto channel_cfg = channel_plan(channel); const uint8_t path = static_cast(get_bb(0x1884, 1u << 20)); const auto capped = tssi_bb_plan(_tx_power_targets, channel.Channel, - _rfe_type, path, max_target_qdbm); + _rfe_type, path, max_target_qdbm, + offset_qdb); if (!_initialized || !channel_cfg || !capped || max_target_qdbm > 64 || efuse.tx_power_mode != TxPowerPgMode8733b::TssiOffset || _tssi_digital_snapshot || _tssi_analog_snapshot || @@ -1339,16 +1363,18 @@ bool Phy8733b::enable_tssi_tracking(SelectedChannel channel, _fr_tssi_offsets = capped->rate_offsets; _fr_tssi_path = path; _logger->info( - "RTL8733B TSSI tracking enabled: ch={} path={} ceiling={} " + "RTL8733B TSSI tracking enabled: ch={} path={} ceiling={} offset={} " "rates={:08x}/{:08x}/{:08x}/{:08x}/{:08x}", - channel.Channel, path, max_target_qdbm, capped->rate_offsets[0], - capped->rate_offsets[1], capped->rate_offsets[2], - capped->rate_offsets[3], capped->rate_offsets[4]); + channel.Channel, path, max_target_qdbm, offset_qdb, + capped->rate_offsets[0], capped->rate_offsets[1], + capped->rate_offsets[2], capped->rate_offsets[3], + capped->rate_offsets[4]); devourer::Ev(_logger->events(), "rtl8733b.tssi_tracking") .f("enabled", true) .f("channel", channel.Channel) .f("path", path) - .f("ceiling_qdbm", max_target_qdbm); + .f("ceiling_qdbm", max_target_qdbm) + .f("offset_qdb", offset_qdb); return true; } catch (...) { set_bb(0x4318, 0x70000000u, 0); @@ -1362,6 +1388,75 @@ bool Phy8733b::enable_tssi_tracking(SelectedChannel channel, } } +bool Phy8733b::set_tssi_offset(SelectedChannel channel, + uint8_t max_target_qdbm, int offset_qdb, + TssiOffsetSat8733b *sat) { + /* Refuse before the first write, like fast_retune: tracking not live means + * there is no loop to retarget (the flat-PG path has no actuator at all), + * and a channel the radio is not on would install the wrong band's targets. */ + if (!_initialized || !_tssi_digital_snapshot || !_fr_tssi_offsets || + max_target_qdbm > kSafeTssiTargetQdbm8733b || + (_fr_plan && _fr_plan->primary != channel.Channel)) { + _logger->error("RTL8733B TSSI offset: refused (tracking not live or " + "channel/ceiling mismatch)"); + return false; + } + const auto plan = + tssi_bb_plan(_tx_power_targets, channel.Channel, _rfe_type, + _fr_tssi_path, max_target_qdbm, offset_qdb, sat); + if (!plan) { + _logger->error("RTL8733B TSSI offset: no target plan for ch{}", + channel.Channel); + return false; + } + if (plan->rate_offsets == *_fr_tssi_offsets) + return true; + + /* In place, tracking left enabled — the #389 shape. Only the five packed + * per-rate target dwords carry the power level; every other register in the + * plan is a constant the enable path already wrote. */ + for (size_t i = 0; i < plan->rate_offsets.size(); ++i) + set_bb(static_cast(0x3a00 + i * 4), kDwordMask, + plan->rate_offsets[i]); + const TssiBbState8733b now = read_tssi_bb_state(); + const bool ok = now.enabled && now.rate_offsets == plan->rate_offsets; + if (!ok) { + /* Roll back to what the chip was carrying rather than leaving the loop on + * a half-written target. _fr_tssi_offsets stays as it was — it describes + * the state being restored. */ + for (size_t i = 0; i < _fr_tssi_offsets->size(); ++i) + set_bb(static_cast(0x3a00 + i * 4), kDwordMask, + (*_fr_tssi_offsets)[i]); + _logger->error("RTL8733B TSSI offset: readback failed, rolled back"); + } else { + _fr_tssi_offsets = plan->rate_offsets; + } + _logger->info("RTL8733B TSSI offset: ok={} ch={} ceiling={} offset={} " + "sat={}/{} rates={:08x}/{:08x}/{:08x}/{:08x}/{:08x}", + ok, channel.Channel, max_target_qdbm, offset_qdb, + sat && sat->low, sat && sat->high, plan->rate_offsets[0], + plan->rate_offsets[1], plan->rate_offsets[2], + plan->rate_offsets[3], plan->rate_offsets[4]); + devourer::Ev(_logger->events(), "rtl8733b.tssi_offset") + .f("ok", ok) + .f("channel", channel.Channel) + .f("ceiling_qdbm", max_target_qdbm) + .f("offset_qdb", offset_qdb) + .f("saturated_low", sat && sat->low) + .f("saturated_high", sat && sat->high) + .hexf("rate_0_3", plan->rate_offsets[0], 8) + .hexf("rate_16_19", plan->rate_offsets[4], 8); + return ok; +} + +bool Phy8733b::tssi_offsets_confirmed() { + if (!_initialized || !_tssi_digital_snapshot || !_fr_tssi_offsets) + return false; + /* read_txagc_state's rate_diffs array IS 0x3a00..0x3a10 — the same five + * dwords that carry the loop's per-rate targets while tracking is on. */ + return read_txagc_state().rate_diffs == *_fr_tssi_offsets; +} + bool Phy8733b::disable_tssi_tracking() { if (!_tssi_digital_snapshot || !_tssi_analog_snapshot) return true; @@ -1556,7 +1651,8 @@ bool Phy8733b::set_channel(SelectedChannel channel) { } bool Phy8733b::fast_retune(SelectedChannel channel, bool tssi_live, - uint8_t max_target_qdbm, bool cache_rf) { + uint8_t max_target_qdbm, bool cache_rf, + int offset_qdb) { const auto plan = channel_plan(channel); if (!_initialized || !plan || !_fr_plan) return false; @@ -1573,8 +1669,11 @@ bool Phy8733b::fast_retune(SelectedChannel channel, bool tssi_live, std::optional tssi; std::optional de; if (tssi_live && _fr_tssi_offsets) { + /* The live runtime offset rides along: without it a hop would recompute + * the targets from the bare ceiling and silently walk the caller's TX-power + * backoff back up. */ tssi = tssi_bb_plan(_tx_power_targets, channel.Channel, _rfe_type, - _fr_tssi_path, max_target_qdbm); + _fr_tssi_path, max_target_qdbm, offset_qdb); if (!tssi) return false; } diff --git a/src/rtl8733b/Phy8733b.h b/src/rtl8733b/Phy8733b.h index 5069bc6..5dd9cfe 100644 --- a/src/rtl8733b/Phy8733b.h +++ b/src/rtl8733b/Phy8733b.h @@ -78,6 +78,18 @@ inline constexpr uint8_t kSafeTxAgcIndex8733b = 0x10; * setup. */ inline constexpr uint8_t kSafeTssiTargetQdbm8733b = 64; +/* Which rail the runtime TX-power offset clamped at, if any — the signal a + * closed-loop controller uses to know the knob has run out of travel + * (IRtlDevice::GetTxPowerState). `low` is set when a rate's shifted target hit + * 0 qdBm or the int8 delta field's floor; `high` when it hit that field's + * ceiling. Both are per-rate facts: a shift can rail one rate while the rest + * still move, which is exactly what a shape-preserving offset does at the end + * of its range. */ +struct TssiOffsetSat8733b { + bool low = false; + bool high = false; +}; + struct TxAgcState8733b { uint8_t cck_ref_a = 0; uint8_t cck_ref_b = 0; @@ -194,8 +206,29 @@ class Phy8733b { bool audit_tssi_enable(SelectedChannel channel, const EfuseInfo &efuse, uint8_t max_target_qdbm); bool enable_tssi_tracking(SelectedChannel channel, const EfuseInfo &efuse, - uint8_t max_target_qdbm); + uint8_t max_target_qdbm, int offset_qdb = 0); bool disable_tssi_tracking(); + /* Runtime TX-power actuator (IRtlDevice::SetTxPowerOffsetQdb). On a + * TSSI-offset PG unit the closed loop IS the TX-power control, so moving + * power means moving the loop's per-rate target table: the five packed + * dwords at 0x3a00..0x3a10, rewritten IN PLACE with tracking left enabled — + * the #389 shape fast_retune already uses for its per-channel rewrite, not + * the ~165 ms disable/re-enable dance. Everything that can refuse is + * computed before the first chip write, so a declined call leaves the + * target untouched; a failed readback rolls back to the offsets the chip + * was carrying. Returns false when tracking is not live (nothing to + * retarget) or the plan does not resolve for this channel. + * + * The loop needs settling time — see docs/rtl8733b.md — so a caller + * sweeping offsets must pace, or it measures the tracking loop rather than + * the knob. */ + bool set_tssi_offset(SelectedChannel channel, uint8_t max_target_qdbm, + int offset_qdb, TssiOffsetSat8733b *sat = nullptr); + /* Does the chip's live per-rate target table still match what this session + * believes it wrote? Six register reads (the 0x3a00 dwords via + * read_txagc_state), so a caller can poll it at its own cadence — the + * chip-truth half of GetTxPowerState on the TSSI path. */ + bool tssi_offsets_confirmed(); /* Lean intra-band, same-bandwidth hop — the FastRetune core (see * docs/frequency-hopping.md; profile that sized it: full set_channel on * this USB-HS part is ~330 ms, of which ~165 ms is the TSSI @@ -217,7 +250,8 @@ class Phy8733b { * on a band or width change or when the radio was never tuned; the caller * falls back to the full set_channel. */ bool fast_retune(SelectedChannel channel, bool tssi_live, - uint8_t max_target_qdbm, bool cache_rf); + uint8_t max_target_qdbm, bool cache_rf, + int offset_qdb = 0); bool prepare_tssi_offsets(SelectedChannel channel, const EfuseInfo &efuse); TssiDeState8733b read_tssi_de_state(); uint8_t read_thermal(); @@ -233,13 +267,21 @@ class Phy8733b { tssi_de_plan(const TssiPowerInfo8733b &power, uint8_t channel); static bool parse_tx_power_targets(const uint32_t *table, size_t len, TxPowerTargets8733b &out); + /* Per-rate closed-loop targets as int8 deltas from the 64 qdBm anchor: + * clamp(min(factory_target, max_target_qdbm) + offset_qdb) - 64. The ceiling + * caps; the offset SHIFTS what survives the cap, which is what preserves the + * calibrated per-rate shape the src/TxPower.h contract promises (a lowered + * ceiling alone would move only the rates sitting above it). offset_qdb = 0 + * reproduces the pre-runtime-knob table byte for byte. */ static std::optional> tssi_rate_offsets(const TxPowerTargets8733b &targets, uint8_t band, - uint8_t path, uint8_t max_target_qdbm = 0xff); + uint8_t path, uint8_t max_target_qdbm = 0xff, + int offset_qdb = 0, TssiOffsetSat8733b *sat = nullptr); static std::optional tssi_bb_plan(const TxPowerTargets8733b &targets, uint8_t channel, uint8_t rfe_type, uint8_t path, - uint8_t max_target_qdbm = 0xff); + uint8_t max_target_qdbm = 0xff, int offset_qdb = 0, + TssiOffsetSat8733b *sat = nullptr); static TssiThermalPlan8733b tssi_thermal_plan(uint8_t efuse_thermal, bool cck); diff --git a/src/rtl8733b/Rtl8733bDevice.cpp b/src/rtl8733b/Rtl8733bDevice.cpp index 32b1cc2..48da769 100644 --- a/src/rtl8733b/Rtl8733bDevice.cpp +++ b/src/rtl8733b/Rtl8733bDevice.cpp @@ -188,10 +188,21 @@ bool Rtl8733bDevice::configure_tx_power(SelectedChannel channel) { if (!_phy.prepare_tssi_bb(channel, _efuse) || !_phy.prepare_tssi_thermal(_efuse, cck_table) || !_phy.prepare_tssi_offsets(channel, _efuse) || - !_phy.enable_tssi_tracking( - channel, _efuse, rtl8733b::kSafeTssiTargetQdbm8733b)) + !_phy.enable_tssi_tracking(channel, _efuse, + rtl8733b::kSafeTssiTargetQdbm8733b, + _tx_offset_qdb)) return false; _tssi_tracking = true; + /* The enable above already installed the offset, so this recomputes the same + * plan, writes nothing, and returns the rails it hit — which is how the + * saturation flags stay chip-derived on the bring-up and channel-change + * paths without a second code path for the arithmetic. */ + rtl8733b::TssiOffsetSat8733b sat; + if (!_phy.set_tssi_offset(channel, rtl8733b::kSafeTssiTargetQdbm8733b, + _tx_offset_qdb, &sat)) + return false; + _tx_sat_low = sat.low; + _tx_sat_high = sat.high; return true; } @@ -358,7 +369,8 @@ void Rtl8733bDevice::FastRetune(uint8_t channel, bool cache_rf) { target.Channel = channel; if (_phy_ready && _phy.fast_retune(target, _tssi_tracking, - rtl8733b::kSafeTssiTargetQdbm8733b, cache_rf)) { + rtl8733b::kSafeTssiTargetQdbm8733b, cache_rf, + _tx_offset_qdb)) { _channel = target; return; } @@ -664,9 +676,137 @@ devourer::AdapterCaps Rtl8733bDevice::GetAdapterCaps() { * validation unit: ~55 ms call / ~10 ms p50 radio-live, vs the * ~330-440 ms full path (USB HS). */ caps.fastretune_ok = true; + caps.txpwr = GetTxPowerCaps(); return caps; } +/* A dBm-TARGET model, not an index model — the TSSI loop's per-rate target + * table is quarter-dBm, so index_max stays 0 (the value TxPowerCaps reserves + * for exactly this shape) and one step is one qdB, the same answer Kestrel's + * fixed-dBm BB target gives. + * + * The range is deliberately one-sided. Offset 0 is kSafeTssiTargetQdbm8733b = + * 16 dBm, the highest level this backend has characterised, so the knob can + * only back off from it and no un-measured power increase is reachable through + * this API; raising the ceiling is a separate, conducted-measurement decision. + * -64 qdB puts the target at 0 qdBm, where the per-rate delta field bottoms out + * too. + * + * step_measured is false: the quarter-dB step is what the hardware target table + * is denominated in, not a slope anyone has measured on air for this part. It + * flips when a paced offset sweep against a witness receiver says so — no SDR + * has been on this silicon, exactly as every other RF-domain claim in this + * backend records. + * + * Static and state-free, per the GetAdapterCaps contract (resolved at + * construction, callable before Init, safe from any thread). In particular it + * does NOT consult the EFUSE PG mode, which is unknown until bring-up: a + * flat-PG unit's lack of an actuator surfaces on SetTxPowerOffsetQdb (refused, + * loudly) and GetTxPowerState, not by mutating the family's capabilities. */ +devourer::TxPowerCaps Rtl8733bDevice::GetTxPowerCaps() { + devourer::TxPowerCaps c; + c.supported = true; + c.index_max = 0; + c.step_qdb = 1; + c.step_measured = false; + c.offset_min_qdb = + -static_cast(rtl8733b::kSafeTssiTargetQdbm8733b); + c.offset_max_qdb = 0; + c.rate_diffs = false; + c.rate_diffs_hw_table = false; + c.rate_diffs_measured = false; + return c; +} + +int Rtl8733bDevice::SetTxPowerOffsetQdb(int qdb) { + std::lock_guard lock(_reg_mu); + const devourer::TxPowerCaps caps = GetTxPowerCaps(); + int steps = 0; + const int applied = devourer::quantize_offset_qdb(qdb, caps, &steps); + const bool req_low = qdb < caps.offset_min_qdb; + const bool req_high = qdb > caps.offset_max_qdb; + + if (_tx_ready && !_tssi_tracking) { + /* This unit's EFUSE carries no TSSI calibration, so TX runs the flat + * kSafeTxAgcIndex8733b path, which has no runtime actuator here. Say so + * rather than return a number that reads like a successful apply — that + * indistinguishability is the whole reason this knob exists. */ + _logger->error( + "RTL8733B: SetTxPowerOffsetQdb({}) refused — no TSSI calibration on " + "this unit, so TX power is the fixed flat index and has no runtime " + "actuator", + qdb); + return 0; + } + + if (!_tx_ready) { + /* Recorded now, applied by configure_tx_power at InitWrite — the family + * contract for a knob moved before the chip is up. */ + _tx_offset_qdb = static_cast(applied); + _tx_sat_low = req_low; + _tx_sat_high = req_high; + _logger->info("RTL8733B: TX-power offset {} qdB recorded (requested {}), " + "applied at InitWrite", + applied, qdb); + return applied; + } + + rtl8733b::TssiOffsetSat8733b sat; + if (!_phy.set_tssi_offset(_channel, rtl8733b::kSafeTssiTargetQdbm8733b, + applied, &sat)) + return 0; + _tx_offset_qdb = static_cast(applied); + _tx_sat_low = sat.low || req_low; + _tx_sat_high = sat.high || req_high; + _logger->info("RTL8733B: SetTxPowerOffsetQdb({}) -> applied {} qdB " + "(target {} qdBm) sat_low={} sat_high={}", + qdb, applied, + rtl8733b::kSafeTssiTargetQdbm8733b + applied, _tx_sat_low, + _tx_sat_high); + return applied; +} + +devourer::TxPowerState Rtl8733bDevice::GetTxPowerState() { + std::lock_guard lock(_reg_mu); + devourer::TxPowerState s; + if (!_phy_ready || !_tx_ready) + return s; /* valid=false — no TX-power state has been programmed yet. */ + s.valid = true; + + if (!_tssi_tracking) { + /* Flat-PG unit: no actuator, but the TXAGC registers ARE the level and + * they read back, so report chip truth. set_flat_tx_power writes one index + * to both references and zeroes the per-rate diffs, so every + * representative rate sits at that index. */ + const rtl8733b::TxAgcState8733b agc = _phy.read_txagc_state(); + s.flat_index = agc.ofdm_ref_a; + s.cck_index = agc.cck_ref_a; + s.ofdm_index = agc.ofdm_ref_a; + s.mcs7_index = agc.ofdm_ref_a; + s.hw_readback = true; + return s; + } + + /* TSSI path: a dBm-target model, so there is no TXAGC index to report and + * flat_index / the three representative fields stay -1 rather than carrying + * quarter-dBm targets in fields declared as indices (Kestrel reports the + * same shape). hw_readback says the offset below was confirmed against the + * chip's live target table, not just read out of this shadow — the + * distinction a consumer needs when the whole failure mode being fixed was a + * shadow that always agreed with itself. */ + s.offset_qdb = _tx_offset_qdb; + s.offset_steps = _tx_offset_qdb; /* 1 step == 1 qdB on the dBm model */ + s.saturated_low = _tx_sat_low; + s.saturated_high = _tx_sat_high; + s.hw_readback = _phy.tssi_offsets_confirmed(); + if (!s.hw_readback) + _logger->warn("RTL8733B: TX-power state unconfirmed — the chip's TSSI " + "target table does not match the {} qdB offset this session " + "believes it applied", + _tx_offset_qdb); + return s; +} + devourer::ThermalStatus Rtl8733bDevice::GetThermalStatus() { std::lock_guard lock(_reg_mu); devourer::ThermalStatus status; diff --git a/src/rtl8733b/Rtl8733bDevice.h b/src/rtl8733b/Rtl8733bDevice.h index 558e754..8415f40 100644 --- a/src/rtl8733b/Rtl8733bDevice.h +++ b/src/rtl8733b/Rtl8733bDevice.h @@ -47,6 +47,15 @@ class Rtl8733bDevice : public IRtlDevice { devourer::TxCaps GetTxCaps() override; devourer::AdapterCaps GetAdapterCaps() override; + /* Runtime TX power. Only the relative offset is ported: on a TSSI-offset PG + * unit the closed loop is the power control, and moving its target is the + * one lever this part has that was measured to work. The flat-index and + * per-rate-diff knobs stay on IRtlDevice's not-ported defaults — + * kSafeTxAgcIndex8733b was witnessed unable to carry HT at all, and no + * dB-per-step slope has been measured for the index. */ + devourer::TxPowerCaps GetTxPowerCaps() override; + int SetTxPowerOffsetQdb(int qdb) override; + devourer::TxPowerState GetTxPowerState() override; devourer::TxStats GetTxStats() override { return _device.GetTxStats(); } devourer::ThermalStatus GetThermalStatus() override; bool GetPermanentMacAddress(uint8_t out[6]) override; @@ -75,6 +84,12 @@ class Rtl8733bDevice : public IRtlDevice { bool _phy_ready = false; bool _tx_ready = false; bool _tssi_tracking = false; + /* Session TX-power offset (qdB, <= 0) and the rails the last apply hit. + * Sticky by construction: configure_tx_power folds it back in on every + * channel set, and FastRetune passes it to the in-place hop rewrite. */ + int16_t _tx_offset_qdb = 0; + bool _tx_sat_low = false; + bool _tx_sat_high = false; std::atomic _rx_stop{false}; std::atomic _rx_active{false}; std::atomic _rx_configured_bw{0}; diff --git a/tests/rtl8733b_txpwr_selftest.cpp b/tests/rtl8733b_txpwr_selftest.cpp new file mode 100644 index 0000000..e075a38 --- /dev/null +++ b/tests/rtl8733b_txpwr_selftest.cpp @@ -0,0 +1,148 @@ +/* Runtime TX-power offset math for the RTL8733B (devourer#1). + * + * On a TSSI-offset PG unit the closed loop is the TX-power control, so the + * runtime offset moves the loop's per-rate target table rather than a TXAGC + * index. Phy8733b::tssi_rate_offsets is pure and static, which makes the whole + * contract testable headless: cap first, then shift what survives the cap, and + * report the rails. + * + * The load-bearing cell is "shape survives the shift" — an implementation that + * lowered the ceiling instead of shifting would pass every uniform-target case + * here and still silently flatten the calibrated per-rate spread. + */ +#include +#include + +#include "hal8733b_tables.h" +#include "rtl8733b/Phy8733b.h" + +namespace { +int failures = 0; + +void expect(const char *what, bool condition) { + if (condition) + return; + ++failures; + std::printf("FAIL: %s\n", what); +} + +/* Synthetic 2.4 GHz targets: a flat ladder with one rate deliberately + * calibrated below the safe ceiling, so the shape assertions have something to + * preserve. Quarter-dBm, like the generated table. */ +rtl8733b::TxPowerTargets8733b synthetic_targets(uint8_t low_rate_qdbm) { + rtl8733b::TxPowerTargets8733b t; + for (uint8_t path = 0; path < 2; ++path) { + for (size_t rate = 0; rate < 20; ++rate) + t.qdbm[0][path][rate] = 80; /* 20 dBm — above the 16 dBm ceiling */ + t.qdbm[0][path][7] = low_rate_qdbm; + t.present[0][path] = true; + } + return t; +} +} // namespace + +int main() { + constexpr uint8_t kCeiling = rtl8733b::kSafeTssiTargetQdbm8733b; /* 64 */ + const rtl8733b::TxPowerTargets8733b t = synthetic_targets(60); + + /* 1. Offset 0 is the pre-knob table, byte for byte — the no-change control. + * Compared against the default-argument call, which is what every + * existing caller compiles to. */ + const auto base = rtl8733b::Phy8733b::tssi_rate_offsets(t, 0, 0, kCeiling); + const auto base_explicit_zero = + rtl8733b::Phy8733b::tssi_rate_offsets(t, 0, 0, kCeiling, 0); + expect("offset 0 reproduces the pre-knob table", + base && base_explicit_zero && *base == *base_explicit_zero); + expect("ceiling caps a hot rate at the anchor", base && (*base)[0] == 0); + expect("a rate calibrated below the ceiling keeps its own level", + base && (*base)[7] == -4); + + /* 2. A uniform shift moves every capped rate by exactly the offset. */ + const auto down24 = + rtl8733b::Phy8733b::tssi_rate_offsets(t, 0, 0, kCeiling, -24); + expect("-24 qdB shifts the capped rates by -24", + down24 && (*down24)[0] == -24 && (*down24)[19] == -24); + + /* 3. THE cell: the calibrated spread survives the shift. A lowered-ceiling + * implementation would leave rate 7 at -4 while the rest moved to -24. */ + expect("per-rate shape survives the shift", + down24 && (*down24)[7] == -28 && + ((*down24)[0] - (*down24)[7]) == ((*base)[0] - (*base)[7])); + + /* 4. Rails. -64 qdB puts the anchor rates at a 0 qdBm target; the rate + * already 4 qdB colder runs out first and says so. */ + rtl8733b::TssiOffsetSat8733b sat_clean; + const auto floor_ok = + rtl8733b::Phy8733b::tssi_rate_offsets(t, 0, 0, kCeiling, -64, + &sat_clean); + expect("-64 qdB reaches the 0 qdBm target without railing the ladder", + floor_ok && (*floor_ok)[0] == -64 && !sat_clean.high); + expect("the colder rate rails first and is reported", + floor_ok && (*floor_ok)[7] == -64 && sat_clean.low); + + rtl8733b::TssiOffsetSat8733b sat_zero; + const auto no_shift = rtl8733b::Phy8733b::tssi_rate_offsets( + t, 0, 0, kCeiling, 0, &sat_zero); + expect("offset 0 rails nothing", + no_shift && !sat_zero.low && !sat_zero.high); + + /* 5. The int8 field's positive end is reachable only above the anchor, which + * is what the raised-ceiling default argument does. */ + rtl8733b::TxPowerTargets8733b hot; + for (uint8_t path = 0; path < 2; ++path) { + for (size_t rate = 0; rate < 20; ++rate) + hot.qdbm[0][path][rate] = 254; + hot.present[0][path] = true; + } + rtl8733b::TssiOffsetSat8733b sat_hot; + const auto clipped = + rtl8733b::Phy8733b::tssi_rate_offsets(hot, 0, 0, 0xff, 0, &sat_hot); + expect("a target past the int8 delta field clamps and reports", + clipped && (*clipped)[0] == 127 && sat_hot.high && !sat_hot.low); + + /* 6. The generated table, capped and shifted: parity at 0 against the values + * the phy-table selftest pins, then the same uniform shift. */ + rtl8733b::TxPowerTargets8733b real; + expect("generated target-power table parses", + rtl8733b::Phy8733b::parse_tx_power_targets( + array_mp_8733b_phy_reg_pg, array_mp_8733b_phy_reg_pg_len, real)); + const auto real_base = + rtl8733b::Phy8733b::tssi_rate_offsets(real, 0, 0, kCeiling); + const auto real_down = + rtl8733b::Phy8733b::tssi_rate_offsets(real, 0, 0, kCeiling, -24); + expect("generated 2G table is flat at the ceiling with no offset", + real_base && (*real_base)[0] == 0 && (*real_base)[19] == 0); + expect("generated 2G table shifts uniformly", + real_down && (*real_down)[0] == -24 && (*real_down)[19] == -24); + + /* 5 GHz has no CCK targets; those four entries stay inert at the anchor + * whatever the offset, exactly as they did before the knob existed. */ + const auto real_5g_base = + rtl8733b::Phy8733b::tssi_rate_offsets(real, 1, 0, kCeiling); + const auto real_5g_down = + rtl8733b::Phy8733b::tssi_rate_offsets(real, 1, 0, kCeiling, -24); + expect("5G CCK entries stay inert across the shift", + real_5g_base && real_5g_down && (*real_5g_base)[0] == 0 && + (*real_5g_down)[0] == 0); + expect("5G OFDM shifts and the colder MCS7 keeps its distance", + real_5g_down && (*real_5g_down)[4] == -24 && + ((*real_5g_base)[4] - (*real_5g_base)[19]) == + ((*real_5g_down)[4] - (*real_5g_down)[19])); + + /* 7. The packed BB plan carries the shift through to the five dwords the + * actuator writes, and nothing else in the plan moves with power. */ + const auto plan_base = rtl8733b::Phy8733b::tssi_bb_plan(real, 1, 0, 0, + kCeiling); + const auto plan_down = + rtl8733b::Phy8733b::tssi_bb_plan(real, 1, 0, 0, kCeiling, -24); + expect("BB plan packs the shifted targets", + plan_base && plan_down && + plan_base->rate_offsets != plan_down->rate_offsets && + plan_down->rate_offsets[0] == 0xe8e8e8e8u); + expect("only the rate-offset dwords depend on power", + plan_base && plan_down && plan_base->reg_4308 == plan_down->reg_4308 && + plan_base->reg_439c == plan_down->reg_439c && + plan_base->reg_43a8 == plan_down->reg_43a8); + + return failures == 0 ? 0 : 1; +} From 695ada87a8604b599ad8942c7861ab27ec97cfab Mon Sep 17 00:00:00 2001 From: snokvist Date: Sun, 16 Aug 2026 07:50:45 +0200 Subject: [PATCH 2/8] rtl8733b: open the TX-power offset both ways, and measure where it stops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the same issue, from two review questions. 1. The range was one-sided, [-64, 0]. That made this the only backend where headroom above the generated table is unreachable, against src/TxPower.h's explicit statement that such headroom is deliberate and compliance is the caller's, and against Jaguar1 (+126) / Jaguar3 (+127) / Kestrel (+12). It also blocked the actual consumer: a per-unit EFUSE trimmed too cold is exactly what a bench calibration exists to correct, and clamping at the PG table would leave a measured operating point uncommandable. The range is now [-64, +127], the int8 per-rate delta field, and the shift is symmetric. 2. "Operator owns compliance" is only a fair answer if the operator is told what they are choosing between, so the overdrive half was swept with the witness reporting EVM beside RSSI (MCS0, ch36): offset RSSI EVM SNR 0 75.4 -62.0 62.0 +16 78.2 -50.2 63.7 <- top of the PG table +32 84.1 -18.0 57.9 +48 83.7 -18.0 58.1 +64 83.8 -18.0 58.2 +16 is real gain (+2.8 dB) that already cost 12 dB of EVM. +32 is not gain at all: 8.7 dB more energy with the constellation collapsed, and +48/+64 move nothing — RSSI and EVM both pinned. The PA is in hard compression, and SNR held 58..64 throughout and never saw it, which is the exact failure docs/bench-testing-near-field.md warns about. So the vendor's PG table lands about where this part stops being linear: +16 qdB is the edge of USABLE overdrive even though the field allows +127, and a caller sweeping for its own operating point must watch EVM, not RSSI. The backoff half stays clean by comparison — EVM flat at -58..-61 across all 16 dB. kMaxPgTargetQdbm8733b records where the vendor's calibration ends (80 qdBm = 20 dBm at 2.4 GHz), pinned against the generated table by the selftest so the figure the docs quote cannot drift off the data. The on-air harness row stays at the backoff half: the overdrive half is not monotone in received power and cannot be scored by an RSSI slope. It is characterised with the EVM column in docs/rtl8733b.md instead. Also adds tests/rtl8733b_txpwr_regcheck.sh — the register-level cells this family needs, separate from txpwr_offset_regcheck.sh because every cell there reads TXAGC indices this dBm-model backend reports as -1 (Kestrel is absent from that script for the same reason). 7/7 on the device: caps, offset-0 parity vs master, the null control (master ignores a -48 request), the -24 shift, both rails, stickiness across SetMonitorChannel + FastRetune, and hw_readback after the hop. ctest 54/54. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 13 ++- docs/rtl8733b.md | 90 ++++++++++++++++- src/rtl8733b/CLAUDE.md | 60 +++++++++++- src/rtl8733b/Phy8733b.cpp | 14 ++- src/rtl8733b/Phy8733b.h | 25 +++-- src/rtl8733b/Rtl8733bDevice.cpp | 58 ++++++++--- tests/rtl8733b_txpwr_regcheck.sh | 158 ++++++++++++++++++++++++++++++ tests/rtl8733b_txpwr_selftest.cpp | 49 +++++++++ tests/txpwr_offset_onair.sh | 59 +++++++---- 9 files changed, 477 insertions(+), 49 deletions(-) create mode 100755 tests/rtl8733b_txpwr_regcheck.sh diff --git a/CLAUDE.md b/CLAUDE.md index f5df669..5c060fd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,7 +59,7 @@ construction from the `SYS_CFG2` chip-id (Kestrel: PID-first): only what an independent witness decoded — legacy OFDM + HT MCS0-7, BCC, 20/40 MHz on 2.4/5 GHz, plus long-preamble CCK on 2.4 GHz at 20 MHz. Everything the backend has not ported (TSF/beacons, hardware ACK, A-MPDU, - the runtime TX-power levers) falls through to `IRtlDevice`'s + the flat-index and per-rate TX-power knobs) falls through to `IRtlDevice`'s not-ported defaults rather than being faked, so read the base class before assuming a cross-generation feature below applies here. `FastRetune` IS ported (intra-band, TSSI kept live — `src/rtl8733b/CLAUDE.md`). SGI, LDPC, STBC, VHT @@ -341,11 +341,16 @@ Behavioural traps the per-field docs can't carry: separate null (`tests/dis_cca_onair.sh`). **Runtime TX power** — the adaptive-link power lever, three knobs on the four -Jaguar/Kestrel generations (the RTL8733B ports none of them: it runs a fixed -safe closed-loop TSSI target): `SetTxPowerOffsetQdb` (relative, +Jaguar/Kestrel generations: `SetTxPowerOffsetQdb` (relative, shape-preserving), `SetTxPowerIndexOverride` (flat absolute), `SetTxPowerRateDiffs` (replace the -calibrated per-rate shape). The contract — how they compose, the MCS7-anchor +calibrated per-rate shape). The RTL8733B ports **only the first**, and on a +different mechanism: its closed-loop TSSI target table, not a TXAGC index, so +its caps report the dBm model (`index_max = 0`) and a one-sided +`[-64, 0] qdB` range below the safe 16 dBm target — the knob can only back off +from the level that backend characterised. The flat index is refused there +because it was measured unable to carry HT at all. The contract — how they +compose, the MCS7-anchor semantics, family step sizes, the write-only-family `hw_readback=false` shadow, and Kestrel's software send-time fold — is documented at the declarations in `src/TxPower.h`; the per-chip mechanics are in each diff --git a/docs/rtl8733b.md b/docs/rtl8733b.md index c1ab80e..61df3e0 100644 --- a/docs/rtl8733b.md +++ b/docs/rtl8733b.md @@ -137,6 +137,87 @@ not survive the link. Measured with the witness, 300 frames submitted at MCS7 with the flat index: zero captured, twice. A unit whose EFUSE carries no TSSI calibration has nothing to drive the loop and takes the flat path. +### The runtime lever + +Because the loop is the power control, the runtime knob is the loop's target. +`IRtlDevice::SetTxPowerOffsetQdb` shifts every per-rate target that survives +the safe ceiling — cap first, then shift, so the calibrated per-rate spread +comes through the move intact — and writes it as the five packed dwords at +`0x3a00..0x3a10`, **in place, with tracking left enabled**. That is the same +shape `fast_retune` uses for its per-channel rewrite (#389); the alternative, +a TSSI disable/re-enable pair, costs ~165 ms and buys nothing here. + +The capability report is the dBm model — `index_max = 0`, one qdB per step — +over `[-64, +127]`, the int8 per-rate delta field's range and the same +clamped-only-at-the-hardware-rail answer Jaguar1 (±126) and Jaguar3 (±127) +give. Offset 0 is `kSafeTssiTargetQdbm8733b` (16 dBm), a first-light clip +sitting at or below this part's factory targets (18–20 dBm at 2.4 GHz, +16–19 at 5 GHz); −64 qdB puts the target at 0 qdBm, where the delta field +bottoms out at the same moment. + +The positive half is deliberately **not** re-clamped at the factory table. +`src/TxPower.h` states for every family that headroom above the generated +table is the operator's call, and a per-unit EFUSE trimmed too cold is exactly +what a bench calibration exists to correct — clamping here would make this the +one backend where a measured operating point cannot be commanded. What that +costs is measured below. + +The offset is sticky: a full `SetMonitorChannel` re-folds it against the new +channel's targets, and `FastRetune` carries it into the hop's in-place rewrite. + +The flat-index and per-rate-diff knobs are **not** ported. The flat index has +no measured dB-per-step slope on this part and, as above, cannot carry HT. + +**Measured on air.** Two independent 6-point passes at ch36 against an +RTL8812AU witness (chip-RSSI ground station — the SDR saturates at this range, +see `tests/txpwr_offset_onair.sh`): the lever is monotone and worth +**14.2 / 14.8 dB** of received power for the full 16 dB of command, overall +slope **0.222 / 0.231 dB per qdB** against the 0.25 nominal. Its adversarial +counterpart, in the same breath: the step is not constant. The bottom 12 qdB +delivered **0.125 and 0.126** dB/qdB — the only structure that reproduced +exactly across both passes — while everything above −52 qdB ran 0.233..0.242 +and the mid-range scattered 0.219..0.252 between passes. Near the floor the +loop returns about half the dB commanded, and `saturated_low` does not warn +about it (that clamp fires only at −64). `TxPowerCaps::step_measured` +therefore stays **false**, the same call the 8822E gets for the same reason: a +controller should calibrate its own dB-per-qdB, or close the loop on the +ground's RSSI. + +The control that makes those numbers readable: the **pre-change binary, in the +same session and geometry, measured flat** — 0.3 dB across the same 64 qdB of +command, because it had no actuator to move. One unit, one witness, near-field +geometry, integer-quantised RSSI, no SDR. Harnesses: +`tests/rtl8733b_txpwr_regcheck.sh` (registers, including that null control), +`tests/txpwr_offset_onair.sh` (slope), `tests/rtl8733b_txpwr_selftest.cpp` +(the offset math, in `ctest`). + +### Overdrive: about 3 dB, and then the PA compresses + +Sweeping the other way — up from the clip, MCS0 at ch36, with the witness +reporting EVM beside RSSI: + +| offset | witness RSSI | witness EVM | witness SNR | +|---|---|---|---| +| 0 | 75.4 | −62.0 | 62.0 | +| +16 (top of the PG table) | 78.2 | −50.2 | 63.7 | +| +32 | 84.1 | **−18.0** | 57.9 | +| +48 | 83.7 | −18.0 | 58.1 | +| +64 | 83.8 | −18.0 | 58.2 | + +The +16 rung is real gain, +2.8 dB, though the constellation has already given +up 12 dB of EVM to get it. The +32 rung is **not** gain: the witness hears +8.7 dB more energy while EVM collapses to −18, and +48 and +64 change nothing +at all — RSSI and EVM both pinned. That is the PA in hard compression, and +`SNR never saw it` (58..64 throughout), which is the failure mode +`docs/bench-testing-near-field.md` exists to warn about: strong RSSI plus poor +EVM means back power off, the opposite of the weak-link response. + +Two things follow. The vendor's PG table lands about where this part stops +being linear, so **+16 qdB is the edge of usable overdrive** even though the +API allows +127 — a caller sweeping for its own operating point should watch +EVM, not RSSI, and stop where EVM turns. And the negative half is clean by +comparison: EVM sits flat at −58..−61 across all 16 dB of backoff. + The chip keeps two thermal-compensation curves, one for CCK and one for OFDM/HT. The table is chosen once per channel set from the configured TX mode and then left alone, which is what the vendor driver does — @@ -189,7 +270,14 @@ a request a caller may be making only through an inherited environment. These results have **not** been claimed: - No SDR was available, so occupied bandwidth, spectral mask, EVM and absolute - output power were not measured. + output power were not measured. The runtime power lever's slope is a + *relative* witness-RSSI measurement for the same reason: it says the lever + moves ~14 dB, not what any rung radiates in dBm. +- The TX-power offset's dB-per-qdB is not constant across its advertised range + (0.125 in the bottom 12 qdB vs 0.233..0.242 above −52), so + `step_measured` stays false and a controller must calibrate its own slope. + Raising `kSafeTssiTargetQdbm8733b` above 16 dBm is deferred — that wants a + conducted measurement, not a witness receiver. - The experimental 5/10 MHz sequence has register-readback and normal-path RX evidence only. Narrowband TX and decode by an independent narrowband peer are deferred; `AdapterCaps::narrowband_ok` remains false. diff --git a/src/rtl8733b/CLAUDE.md b/src/rtl8733b/CLAUDE.md index fd2387f..201779f 100644 --- a/src/rtl8733b/CLAUDE.md +++ b/src/rtl8733b/CLAUDE.md @@ -46,13 +46,55 @@ unrelated register map. tail and its body in separate completions. The RX loop floors its URB size at the same constant and a `static_assert` ties the two together — raising either alone reintroduces the straddle, from opposite sides. -- **TSSI closed loop.** Power runs from a fixed safe target - (`kSafeTssiTargetQdbm8733b`); none of the runtime TX-power levers are ported. - On a TSSI-offset PG unit the loop **is** the TX-power control, so it is not +- **TSSI closed loop.** Power runs from a safe ceiling + (`kSafeTssiTargetQdbm8733b` = 16 dBm). On a TSSI-offset PG unit the loop + **is** the TX-power control, so it is not optional there — an attempt to make it opt-in with a fall back to the flat `kSafeTxAgcIndex8733b` could not carry HT at all (witnessed: MCS7, 300/300 submitted, 0 captured, twice). A unit whose EFUSE carries no TSSI calibration has nothing to drive the loop and takes the flat path. +- **The runtime TX-power lever is that loop's target, and only the relative + knob is ported.** `SetTxPowerOffsetQdb` shifts every per-rate target below the + ceiling (`tssi_rate_offsets`: cap first, then shift — a lowered ceiling would + move only the rates above it and flatten the calibrated spread). The write is + the five packed dwords at `0x3a00..0x3a10`, rewritten **in place with tracking + live**, the same #389 shape `fast_retune` uses — not the ~165 ms + disable/re-enable pair. Caps therefore report the dBm model (`index_max = 0`, + one qdB per step) over `[-64, +127]` — the int8 delta field's range, the same + clamped-only-at-the-hardware-rail answer Jaguar1/3 give. Offset 0 is the safe + clip, −64 qdB puts the target at 0 qdBm where the field bottoms out, and the + positive half is deliberately NOT re-clamped at the PG table: an EFUSE + trimmed too cold is what an operator calibrates their way out of. + `SetTxPowerIndexOverride`, `SetTxPowerRateDiffs` and `ReApplyTxPower` stay + unported. +- **Overdrive above the clip buys ~3 dB and then the PA compresses, and EVM is + the only tell.** Sweeping UP from the clip (MCS0, ch36, witness reporting EVM + beside RSSI): +16 qdB — the top of the PG table — gave +2.8 dB with EVM + already down from −62 to −50; **+32 qdB read 8.7 dB louder with EVM collapsed + to −18**, and +48/+64 changed nothing at all (RSSI and EVM both pinned). More + energy, unusable constellation. **SNR held 58..64 throughout and never saw + it** — precisely the case `docs/bench-testing-near-field.md` is about. So the + counterpart to leaving the range open: the vendor's PG table lands about + where this part stops being linear, and +16 qdB is the edge of *usable* + overdrive even though the field allows +127. Below the clip EVM stays flat at + −58..−61 across all 16 dB of backoff — the negative half is clean. +- **The lever is worth ~14 dB, and it compresses at the bottom.** On-air + against an RTL8812AU witness (chip-RSSI ground station, the + `tests/txpwr_offset_onair.sh` method), two independent 6-point passes at + ch36: monotone, 14.2 / 14.8 dB of received power for the full 16 dB of + command, overall slope 0.222 / 0.231 dB per qdB against the 0.25 nominal. + The counterpart in the same breath: the step is **not** constant. The bottom + 12 qdB delivered 0.125 and 0.126 dB/qdB — the one structure that reproduced + exactly across both passes — while everything above −52 qdB ran 0.233..0.242, + and the mid-range scattered 0.219..0.252 between passes. So the loop gives + about half the commanded dB as its target nears 0 qdBm, while + `saturated_low` still reads false (that clamp only fires at −64). + `step_measured` stays false for exactly that reason, the same call the 8822E + gets: calibrate your own dB-per-qdB or lean on the ground's RSSI. One unit, + one witness, near-field, integer-quantised RSSI, no SDR. + The pre-change binary measured **flat in the same session and geometry** + (0.3 dB across the same 64 qdB) — the do-nothing control that makes the + 14 dB readable. - **The thermal table is chosen once per channel set, not per frame.** The CCK and OFDM/HT variants of the thermal-compensation table are different tables. `configure_tx_power` picks one from the configured TX mode and leaves it, @@ -162,7 +204,9 @@ untouched) and fall back to the full path. ## Not ported `ReadTsf`/beacons, hardware ACK/BlockAck, A-MPDU, -`FastSetBandwidth`, the runtime TX-power knobs, `rx.path` per-chain telemetry, +`FastSetBandwidth`, the flat-index / per-rate-diff TX-power knobs +(`SetTxPowerIndexOverride`, `SetTxPowerRateDiffs`, `ReApplyTxPower` — only the +relative `SetTxPowerOffsetQdb` is ported), `rx.path` per-chain telemetry, and CCA disable. These inherit `IRtlDevice`'s not-ported defaults (`false`, `0`, or a full-path fallback) rather than being faked. `SetCcaMode` is the one exception to the silent-default rule: it is pure virtual, so `true` throws @@ -170,6 +214,14 @@ loudly — without tearing the session down, since an unported optional knob is not a hardware-safety event — while `false` succeeds as a no-op because that is the state MAC bring-up already leaves programmed. +`SetTxPowerOffsetQdb` is the second exception, in the same spirit: on a unit +whose EFUSE carries no TSSI calibration there is no actuator at all, and the +call **refuses loudly and returns 0** rather than reporting a successful +zero-offset apply. That indistinguishability is what this knob exists to end — +a consumer measured 18 dB of commanded offset moving nothing while its state +read `{"applied_qdb":0,"saturated_low":false}`, which is exactly what a healthy +actuator with travel remaining looks like. + `DeviceConfig::tuning::disable_cca` cannot be honoured either, and bring-up warns rather than dropping it — a config knob must not be the one door where a request the setter refuses loudly instead vanishes without a word. The warning diff --git a/src/rtl8733b/Phy8733b.cpp b/src/rtl8733b/Phy8733b.cpp index 58b6e95..ada0832 100644 --- a/src/rtl8733b/Phy8733b.cpp +++ b/src/rtl8733b/Phy8733b.cpp @@ -441,9 +441,17 @@ Phy8733b::tssi_rate_offsets(const TxPowerTargets8733b &targets, uint8_t band, return std::nullopt; } /* Cap first, then shift: the ceiling is the safety limit on absolute - * power, the offset is the caller's relative move below it. A rate the - * ceiling already pulled down keeps its calibrated distance from the - * others through the shift. */ + * power, the offset is the caller's relative move around it, in either + * direction. A rate the ceiling already pulled down keeps its calibrated + * distance from the others through the shift. + * + * The shift is deliberately NOT re-clamped at the factory target on the + * way up. The PG table is the vendor's calibration, not a hardware rail, + * and a per-unit EFUSE trimmed too cold is exactly the case an operator + * calibrates their own way out of — src/TxPower.h states for every family + * that headroom above the generated table is deliberate and compliance is + * the caller's. Clamping here would make this the one backend where a + * measured operating point cannot be commanded. */ int shifted = static_cast((std::min)(target, max_target_qdbm)) + offset_qdb; if (shifted < 0) { diff --git a/src/rtl8733b/Phy8733b.h b/src/rtl8733b/Phy8733b.h index 5dd9cfe..e48de4b 100644 --- a/src/rtl8733b/Phy8733b.h +++ b/src/rtl8733b/Phy8733b.h @@ -78,6 +78,16 @@ inline constexpr uint8_t kSafeTxAgcIndex8733b = 0x10; * setup. */ inline constexpr uint8_t kSafeTssiTargetQdbm8733b = 64; +/* Highest per-rate target in the compiled PG table: 80 qdBm = 20 dBm at + * 2.4 GHz (5 GHz tops out at 76 = 19 dBm), against the 64 qdBm safe clip + * above. Not a limit on anything — the runtime TX-power offset can be + * commanded past it, deliberately (see Rtl8733bDevice::GetTxPowerCaps) — but + * it is the reference point that says where the vendor's calibration ends and + * the operator's own begins. Pinned against the generated table by + * tests/rtl8733b_txpwr_selftest.cpp so the figure quoted in the docs cannot + * drift away from the data. */ +inline constexpr uint8_t kMaxPgTargetQdbm8733b = 80; + /* Which rail the runtime TX-power offset clamped at, if any — the signal a * closed-loop controller uses to know the knob has run out of travel * (IRtlDevice::GetTxPowerState). `low` is set when a rate's shifted target hit @@ -267,12 +277,15 @@ class Phy8733b { tssi_de_plan(const TssiPowerInfo8733b &power, uint8_t channel); static bool parse_tx_power_targets(const uint32_t *table, size_t len, TxPowerTargets8733b &out); - /* Per-rate closed-loop targets as int8 deltas from the 64 qdBm anchor: - * clamp(min(factory_target, max_target_qdbm) + offset_qdb) - 64. The ceiling - * caps; the offset SHIFTS what survives the cap, which is what preserves the - * calibrated per-rate shape the src/TxPower.h contract promises (a lowered - * ceiling alone would move only the rates sitting above it). offset_qdb = 0 - * reproduces the pre-runtime-knob table byte for byte. */ + /* Per-rate closed-loop targets as int8 deltas from the 64 qdBm anchor. + * `max_target_qdbm` is the safety clip; `offset_qdb` is the runtime knob, + * and its two halves do different things because offset 0 sits on that clip + * rather than on a hardware rail (the asymmetry is argued at the + * implementation): a NEGATIVE offset shifts what survives the clip, which + * preserves the calibrated per-rate shape src/TxPower.h promises, while a + * POSITIVE offset raises the clip and each rate rises only as far as its own + * factory target. offset_qdb = 0 reproduces the pre-runtime-knob table byte + * for byte. */ static std::optional> tssi_rate_offsets(const TxPowerTargets8733b &targets, uint8_t band, uint8_t path, uint8_t max_target_qdbm = 0xff, diff --git a/src/rtl8733b/Rtl8733bDevice.cpp b/src/rtl8733b/Rtl8733bDevice.cpp index 48da769..4849ada 100644 --- a/src/rtl8733b/Rtl8733bDevice.cpp +++ b/src/rtl8733b/Rtl8733bDevice.cpp @@ -685,18 +685,52 @@ devourer::AdapterCaps Rtl8733bDevice::GetAdapterCaps() { * for exactly this shape) and one step is one qdB, the same answer Kestrel's * fixed-dBm BB target gives. * - * The range is deliberately one-sided. Offset 0 is kSafeTssiTargetQdbm8733b = - * 16 dBm, the highest level this backend has characterised, so the knob can - * only back off from it and no un-measured power increase is reachable through - * this API; raising the ceiling is a separate, conducted-measurement decision. - * -64 qdB puts the target at 0 qdBm, where the per-rate delta field bottoms out - * too. + * Offset 0 is kSafeTssiTargetQdbm8733b (16 dBm), a first-light clip the backend + * imposes at or below this part's factory targets — which run 18..20 dBm at + * 2.4 GHz and 16..19 dBm at 5 GHz (kMaxPgTargetQdbm8733b). The range around it + * is the delta field's, not a characterised PA window: * - * step_measured is false: the quarter-dB step is what the hardware target table - * is denominated in, not a slope anyone has measured on air for this part. It - * flips when a paced offset sweep against a witness receiver says so — no SDR - * has been on this silicon, exactly as every other RF-domain claim in this - * backend records. + * - Down to -64 qdB, where the target reaches 0 qdBm and the per-rate delta + * field bottoms out at the same moment. + * - Up to +127 qdB, the delta field's positive limit — the same + * clamped-only-at-the-hardware-rail answer Jaguar1 (+126) and Jaguar3 + * (+127) give. src/TxPower.h is deliberate that headroom above the + * generated table belongs to the operator, and a per-unit EFUSE trimmed + * too cold is precisely the case a bench calibration exists to correct: + * clamping at the PG table would make this the one backend where a + * measured operating point cannot be commanded. + * + * Measured where that goes, because "the operator's call" is only a fair + * answer if the operator is told what they are choosing between. Sweeping UP + * from the clip (MCS0, ch36, witness EVM alongside RSSI): +16 qdB — the top of + * the PG table — bought 2.8 dB but EVM had already fallen from -62 to -50; by + * +32 the witness read 8.7 dB more RSSI with EVM COLLAPSED to -18, and +48 and + * +64 changed nothing at all (RSSI pinned, EVM pinned at -18). That is the PA + * in hard compression: more energy, unusable constellation, and SNR never + * moved (58..64) so it cannot be the tell — see docs/bench-testing-near-field.md. + * Below the clip EVM stays flat at -58..-61 across the whole 16 dB of backoff. + * So the vendor's PG table lands about where this part stops being linear: + * treat +16 qdB as the edge of usable overdrive, not the edge of the range. + * + * step_measured stays false, and now for a MEASURED reason rather than an + * unexamined one. On-air against an RTL8812AU witness (chip-RSSI ground + * station, tests/txpwr_offset_onair.sh's method; the B210 saturates at this + * range), two independent 6-point passes: the lever is monotone and worth + * 14.2 / 14.8 dB of received power for the full 16 dB of command, overall + * slope 0.222 / 0.231 dB per qdB against the 0.25 nominal. But the step is NOT + * constant across the advertised range — the bottom 12 qdB delivered 0.125 and + * 0.126 dB/qdB, the one structure that reproduced exactly, while everything + * above -52 qdB ran 0.233..0.242. The closed loop compresses as its target + * approaches 0 qdBm, so a controller near the floor gets about half the dB it + * asked for while saturated_low still reads false (the clamp only fires at + * -64). This is the same call the 8822E gets for the same reason: calibrate + * your own dB-per-qdB, or lean on GetTxPowerState plus the ground's RSSI. + * + * The counterparts: one physical unit, one witness, near-field geometry, and + * an integer-quantised RSSI scale. The mid-range slope scattered 0.219..0.252 + * between the two passes, so the ~0.24 figure is a bench average, not a + * constant. No SDR has been on this silicon, as with every other RF-domain + * claim in this backend. * * Static and state-free, per the GetAdapterCaps contract (resolved at * construction, callable before Init, safe from any thread). In particular it @@ -711,7 +745,7 @@ devourer::TxPowerCaps Rtl8733bDevice::GetTxPowerCaps() { c.step_measured = false; c.offset_min_qdb = -static_cast(rtl8733b::kSafeTssiTargetQdbm8733b); - c.offset_max_qdb = 0; + c.offset_max_qdb = 127; /* the int8 per-rate delta field's positive limit */ c.rate_diffs = false; c.rate_diffs_hw_table = false; c.rate_diffs_measured = false; diff --git a/tests/rtl8733b_txpwr_regcheck.sh b/tests/rtl8733b_txpwr_regcheck.sh new file mode 100755 index 0000000..7b91574 --- /dev/null +++ b/tests/rtl8733b_txpwr_regcheck.sh @@ -0,0 +1,158 @@ +#!/usr/bin/env bash +# Register-level validation of the RTL8733B runtime TX-power offset +# (IRtlDevice::SetTxPowerOffsetQdb / GetTxPowerState), the on-device +# counterpart to tests/rtl8733b_txpwr_selftest.cpp's pure math and to +# tests/txpwr_offset_onair.sh's slope measurement. +# +# Separate from txpwr_offset_regcheck.sh on purpose: every cell there reads the +# representative TXAGC INDICES out of GetTxPowerState, and this family reports +# -1 for all three because it is a dBm-target model (caps.index_max == 0). Its +# power lives in the five packed TSSI target dwords at 0x3a00..0x3a10, which +# the bring-up and actuator log verbatim, so that log line is what these cells +# read. Kestrel — the other dBm-model family — is absent from that script for +# the same reason and has its own (kestrel_txpwr_sweep.sh). +# +# Cells: +# parity offset 0 leaves the target table byte-identical to the master +# build's (the key no-regression invariant, and the control that +# says the offset path costs nothing when unused). Needs a master +# worktree build, cached at $MASTER_BUILD; skip with SKIP_PARITY=1. +# nullctl the MASTER build with an offset REQUESTED writes the same table — +# the do-nothing control proving the pre-change binary has no +# actuator, so an on-air null in this geometry is the code and not +# the bench. +# move -24 qdB shifts every capped rate byte by exactly -24 (0xe8). +# rails -200 clamps to -64 with saturated_low, +200 to +127 with +# saturated_high - the int8 per-rate delta field's range. +# sticky -24 qdB then a full SetMonitorChannel to another 5 GHz group +# re-folds the offset against the new channel; a following +# FastRetune leaves it in place (the hop rewrites the target table, +# so a hop that forgot the offset would silently restore power). +# confirm GetTxPowerState reports hw_readback=1 — the offset was verified +# against the chip, not echoed from the session shadow. +# +# Usage: sudo -v && tests/rtl8733b_txpwr_regcheck.sh +set -u +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +OUT="${RTL8733B_TXPWR_OUT:-/tmp/devourer-8733b-txpwr}" +MASTER_BUILD="${MASTER_BUILD:-/tmp/devourer-master-build}" +PID=0xf72b; VID=0x0bda; CH_A=36; CH_B=149; CH_FAST=153 +mkdir -p "$OUT" + +PASS=0; FAIL=0; SKIP=0 +pass() { echo " PASS: $*"; PASS=$((PASS+1)); } +fail() { echo " FAIL: $*"; FAIL=$((FAIL+1)); } +skip() { echo " SKIP: $*"; SKIP=$((SKIP+1)); } +cleanup() { sudo -n pkill -x txpower 2>/dev/null; sudo -n pkill -x txdemo 2>/dev/null; true; } +trap cleanup EXIT INT TERM + +lsusb -d "$(printf '%04x:%04x' "$VID" "$PID")" >/dev/null 2>&1 || { + echo "SKIP: $PID@$VID not plugged"; exit 0; } + +echo "== building ==" +cmake --build "$ROOT/build" -j --target txpower txdemo >/dev/null || exit 1 + +# The five target dwords, as the bring-up / actuator log them. +targets_at_bringup() { grep -o "TSSI tracking enabled:.*rates=[0-9a-f/]*" "$1" | tail -1 | grep -o "rates=.*"; } +targets_last() { grep -oE "TSSI (tracking enabled|offset):.*rates=[0-9a-f/]*" "$1" | tail -1 | grep -o "rates=.*"; } +state_field() { grep -F '"ev":"txpwr.state"' "$1" | sed -n "$2p" | grep -o "\"$3\":-\?[0-9]*" | cut -d: -f2; } +offset_field() { grep -F '"ev":"txpwr.offset"' "$1" | sed -n "$2p" | grep -o "\"$3\":-\?[0-9]*" | cut -d: -f2; } + +run_txpower() { local out="$1"; shift + sudo -n timeout 90 "$ROOT/build/txpower" --vid "$VID" --pid "$PID" "$@" >"$out" 2>&1 || true; } +run_txdemo() { local out="$1" bin="$2"; shift 2 + sudo -n env DEVOURER_PID="$PID" DEVOURER_VID="$VID" DEVOURER_CHANNEL="$CH_A" \ + DEVOURER_TX_RATE=MCS0 DEVOURER_TX_FRAMES=20 "$@" \ + timeout 60 "$bin" >"$out" 2>&1 || true; } + +ensure_master_build() { + [ -x "$MASTER_BUILD/txdemo" ] && return 0 + local wt="/tmp/devourer-master-worktree" + git -C "$ROOT" worktree add --force "$wt" origin/master >/dev/null 2>&1 || return 1 + cmake -S "$wt" -B "$MASTER_BUILD" >/dev/null 2>&1 || return 1 + cmake --build "$MASTER_BUILD" -j --target txdemo >/dev/null 2>&1 || return 1 +} + +echo "== DUT $PID@$VID (RTL8733B) ==" + +# -- caps -------------------------------------------------------------------- +run_txpower "$OUT/base.log" --channel "$CH_A" --offset-start 0 --offset-stop 0 --step-ms 200 +caps="$(grep -F '"ev":"txpwr.caps"' "$OUT/base.log" | head -1)" +case "$caps" in + *'"supported":1'*'"max":0'*'"step_qdb":1'*'"min_qdb":-64'*'"max_qdb":127'*) + pass "caps: dBm model, delta-field range [-64, +127]" ;; + "") fail "caps: no txpwr.caps event (bring-up failed? see $OUT/base.log)" ;; + *) fail "caps: unexpected $caps" ;; +esac +base_targets="$(targets_at_bringup "$OUT/base.log")" +echo " bring-up targets ch$CH_A: $base_targets" + +# -- parity + null control --------------------------------------------------- +if [ "${SKIP_PARITY:-0}" = "1" ]; then + skip "parity/nullctl (SKIP_PARITY=1)" +elif ensure_master_build; then + run_txdemo "$OUT/master-0.log" "$MASTER_BUILD/txdemo" DEVOURER_TX_PWR_OFFSET_QDB=0 + run_txdemo "$OUT/master-48.log" "$MASTER_BUILD/txdemo" DEVOURER_TX_PWR_OFFSET_QDB=-48 + run_txdemo "$OUT/new-0.log" "$ROOT/build/txdemo" DEVOURER_TX_PWR_OFFSET_QDB=0 + m0="$(targets_at_bringup "$OUT/master-0.log")" + m48="$(targets_at_bringup "$OUT/master-48.log")" + n0="$(targets_at_bringup "$OUT/new-0.log")" + if [ -z "$m0" ]; then + skip "parity (master build produced no TSSI line)" + elif [ "$m0" = "$n0" ]; then + pass "parity: offset 0 byte-identical to master ($n0)" + else + fail "parity: master=$m0 new=$n0" + fi + if [ "$m48" = "$m0" ]; then + pass "nullctl: master ignores a -48 qdB request (no actuator), $m48" + else + fail "nullctl: master moved the targets — the pre-change baseline is wrong" + fi +else + skip "parity/nullctl (master build unavailable)" +fi + +# -- move -------------------------------------------------------------------- +run_txpower "$OUT/move.log" --channel "$CH_A" --offset-start -24 --offset-stop -24 --step-ms 200 +mv_t="$(targets_last "$OUT/move.log")" +mv_applied="$(offset_field "$OUT/move.log" 1 applied)" +if [ "$mv_t" = "rates=00000000/e8e8e8e8/e8e8e8e8/e8e8e8e8/e8e8e8e8" ] && [ "$mv_applied" = "-24" ]; then + pass "move: -24 qdB shifts every capped rate byte to 0xe8, applied=-24" +else + fail "move: applied=$mv_applied $mv_t" +fi + +# -- rails ------------------------------------------------------------------- +run_txpower "$OUT/rails.log" --channel "$CH_A" --offset-start -200 --offset-stop 200 --step-qdb 400 --step-ms 200 +lo_applied="$(offset_field "$OUT/rails.log" 1 applied)"; lo_sat="$(state_field "$OUT/rails.log" 2 satlo)" +hi_applied="$(offset_field "$OUT/rails.log" 2 applied)"; hi_sat="$(state_field "$OUT/rails.log" 3 sathi)" +if [ "$lo_applied" = "-64" ] && [ "$lo_sat" = "1" ] && [ "$hi_applied" = "127" ] && [ "$hi_sat" = "1" ]; then + pass "rails: -200 -> -64 satlo=1, +200 -> +127 sathi=1" +else + fail "rails: low(applied=$lo_applied satlo=$lo_sat) high(applied=$hi_applied sathi=$hi_sat)" +fi + +# -- sticky ------------------------------------------------------------------ +run_txpower "$OUT/sticky.log" --channel "$CH_A" --offset-start -24 --offset-stop -24 --step-ms 200 \ + --switch-channel "$CH_B" --retune "$CH_FAST" +sw_line="$(grep -c "TSSI tracking enabled: ch=$CH_B .*offset=-24" "$OUT/sticky.log")" +post_switch="$(state_field "$OUT/sticky.log" 3 offset_qdb)" +post_retune="$(state_field "$OUT/sticky.log" 4 offset_qdb)" +post_rb="$(state_field "$OUT/sticky.log" 4 rb)" +if [ "$sw_line" -ge 1 ] && [ "$post_switch" = "-24" ] && [ "$post_retune" = "-24" ]; then + pass "sticky: offset survives SetMonitorChannel($CH_B) and FastRetune($CH_FAST)" +else + fail "sticky: refold=$sw_line post_switch=$post_switch post_retune=$post_retune" +fi + +# -- confirm ----------------------------------------------------------------- +if [ "$post_rb" = "1" ]; then + pass "confirm: hw_readback=1 after the hop (chip truth, not the shadow)" +else + fail "confirm: hw_readback=$post_rb after the hop" +fi + +echo +echo "== rtl8733b txpwr regcheck: PASS=$PASS FAIL=$FAIL SKIP=$SKIP ==" +[ "$FAIL" -eq 0 ] diff --git a/tests/rtl8733b_txpwr_selftest.cpp b/tests/rtl8733b_txpwr_selftest.cpp index e075a38..e7486c4 100644 --- a/tests/rtl8733b_txpwr_selftest.cpp +++ b/tests/rtl8733b_txpwr_selftest.cpp @@ -86,6 +86,27 @@ int main() { expect("offset 0 rails nothing", no_shift && !sat_zero.low && !sat_zero.high); + /* 4b. The knob is symmetric: a positive offset shifts the same ladder up, + * shape intact, and is NOT re-clamped at the vendor's factory target. + * That last part is the point — a per-unit EFUSE trimmed too cold is the + * case an operator calibrates their own operating point for, and this is + * the only lever this backend gives them to command it. */ + rtl8733b::TssiOffsetSat8733b sat_up; + const auto up16 = rtl8733b::Phy8733b::tssi_rate_offsets(t, 0, 0, kCeiling, + 16, &sat_up); + expect("+16 qdB shifts the ladder up, shape intact", + up16 && (*up16)[0] == 16 && (*up16)[7] == 12 && + ((*up16)[0] - (*up16)[7]) == ((*base)[0] - (*base)[7])); + expect("a positive offset within the field rails nothing", + !sat_up.high && !sat_up.low); + /* Past the factory target too: the synthetic ladder's rates are calibrated + * at 80 qdBm, and +32 commands 96 — uncalibrated by construction, reachable + * by design, compliance the caller's (src/TxPower.h). */ + const auto up32 = + rtl8733b::Phy8733b::tssi_rate_offsets(t, 0, 0, kCeiling, 32); + expect("the offset can be commanded past the factory target", + up32 && (*up32)[0] == 32); + /* 5. The int8 field's positive end is reachable only above the anchor, which * is what the raised-ceiling default argument does. */ rtl8733b::TxPowerTargets8733b hot; @@ -115,6 +136,34 @@ int main() { expect("generated 2G table shifts uniformly", real_down && (*real_down)[0] == -24 && (*real_down)[19] == -24); + /* kMaxPgTargetQdbm8733b is the figure the caps comment and docs quote as + * where the vendor's calibration ends, so it must stay pinned to the + * generated table: a regeneration that moves the highest factory target has + * to fail here rather than let the documented number drift off the data. */ + int table_max = -1; + for (uint8_t band = 0; band < 2; ++band) + for (uint8_t path = 0; path < 2; ++path) { + if (!real.present[band][path]) + continue; + for (size_t rate = 0; rate < 20; ++rate) { + const uint8_t v = real.qdbm[band][path][rate]; + if (v != 0xff && static_cast(v) > table_max) + table_max = v; + } + } + expect("kMaxPgTargetQdbm8733b matches the generated table's highest target", + table_max == static_cast(rtl8733b::kMaxPgTargetQdbm8733b)); + + /* The generated 2G table is flat against the 16 dBm clip, so a +16 qdB + * command lands every rate at 20 dBm — at the top of the vendor's range for + * the hottest rate and ABOVE it for the rest. That is the shape a + * calibrating operator asks for, and it is why the positive half is not + * re-clamped per rate. */ + const auto real_up = + rtl8733b::Phy8733b::tssi_rate_offsets(real, 0, 0, kCeiling, 16); + expect("+16 qdB commands the 2G ladder to the top of the PG range", + real_up && (*real_up)[0] == 16 && (*real_up)[19] == 16); + /* 5 GHz has no CCK targets; those four entries stay inert at the anchor * whatever the offset, exactly as they did before the knob existed. */ const auto real_5g_base = diff --git a/tests/txpwr_offset_onair.sh b/tests/txpwr_offset_onair.sh index b03220f..ba9d47e 100755 --- a/tests/txpwr_offset_onair.sh +++ b/tests/txpwr_offset_onair.sh @@ -14,11 +14,16 @@ # both read -56.9 dBFS at gain 0), so wideband SDR power cannot see the TXAGC # slope at all; a WiFi chip's AGC is built for exactly this input range. # -# Method: one FIXED-INDEX TxDemo process per point (DEVOURER_TX_PWR — the flat -# override riding the runtime API), 6 points spanning the family's range, the -# ground's median per-frame RSSI per cell, least-squares slope. Default cell -# channel is 5 GHz ch36, per-DUT overrides in the table (canonical-SA -# filtering keeps the ground blind to ambient frames either way). +# Method: one fixed-power TxDemo process per point, 6 points spanning the +# family's range, the ground's median per-frame RSSI per cell, least-squares +# slope. The power is held by the family's own lever (the DUT table's `knob` +# column): the flat override DEVOURER_TX_PWR on the index families, the +# relative DEVOURER_TX_PWR_OFFSET_QDB on the RTL8733B, which has no flat lever. +# Each cell holds one level for 14 s and the fit window opens 6 s in, which is +# also what gives a closed-loop family time to settle (the RTL8733B's TSSI loop +# wants tens of ms; 6 s is not close to marginal). Default cell channel is +# 5 GHz ch36, per-DUT overrides in the table (canonical-SA filtering keeps the +# ground blind to ambient frames either way). # # Usage: sudo -v && tests/txpwr_offset_onair.sh [PID ...] (default: plugged set) set -u @@ -42,7 +47,7 @@ trap cleanup EXIT INT TERM echo "== building ==" cmake --build "$ROOT/build" -j --target txdemo rxdemo >/dev/null || exit 1 -# DUT table: pid vid ramp_start ramp_stop nominal_db_per_step channel +# DUT table: pid vid ramp_start ramp_stop nominal_db_per_step channel knob # - 8821AU runs at ch6: its 5 GHz chain IGNORES BB TXAGC (measured flat at # ch36 across two grounds while registers move; 0.50 dB/idx exactly at # 2.4 GHz) — the power lever is 2.4 GHz-only on that part. @@ -50,14 +55,28 @@ cmake --build "$ROOT/build" -j --target txdemo rxdemo >/dev/null || exit 1 # transfer (measured 0.3->0.9 dB/idx across the range, ~0.55 avg), so the # cell asserts a working monotone lever (slope 0.15..1.0) instead of a # fixed step classification. +# - The `knob` column is the env var each family's lever rides. Every index +# family sweeps the flat override DEVOURER_TX_PWR; the RTL8733B has no flat +# lever at all (kSafeTxAgcIndex8733b cannot carry HT), so it sweeps the +# RELATIVE offset instead — the x axis is qdB below the safe TSSI target, +# not an index, which is also why its range is negative. Its transfer is +# TSSI-reshaped like the 8822E's, so it takes the same monotone-lever +# assertion: measured 0.222/0.231 dB per qdB overall across two passes, but +# only 0.125 in the bottom 12 qdB where the loop nears its 0 qdBm floor. +# Its cells stop at 0 — the BACKOFF half. The API allows +127 qdB, but that +# half is not monotone in received power and cannot be asserted this way: +# +32 qdB reads 8.7 dB louder with EVM collapsed from -62 to -18, and +48/+64 +# move nothing at all. Characterised with the EVM column in +# docs/rtl8733b.md; RSSI alone cannot score it, and SNR never sees it. DUTS=( - "0x8812 0x0bda 8 56 0.5 36" # RTL8812AU (Jaguar1) - "0x0120 0x2357 8 56 0.5 6" # RTL8821AU (Jaguar1, 2.4G-only lever) - "0x8813 0x0bda 8 56 0.5 36" # RTL8814AU (Jaguar1) - "0x012d 0x2357 8 56 0.5 36" # RTL8822BU (Jaguar2) - "0xc811 0x0bda 8 56 0.5 36" # RTL8821CU (Jaguar2) - "0xc812 0x0bda 24 104 0.25 36" # RTL8822CU (Jaguar3) - "0xa81a 0x0bda 24 104 tssi 36" # RTL8822EU (Jaguar3, TSSI-reshaped) + "0x8812 0x0bda 8 56 0.5 36 DEVOURER_TX_PWR" # RTL8812AU (Jaguar1) + "0x0120 0x2357 8 56 0.5 6 DEVOURER_TX_PWR" # RTL8821AU (Jaguar1, 2.4G-only lever) + "0x8813 0x0bda 8 56 0.5 36 DEVOURER_TX_PWR" # RTL8814AU (Jaguar1) + "0x012d 0x2357 8 56 0.5 36 DEVOURER_TX_PWR" # RTL8822BU (Jaguar2) + "0xc811 0x0bda 8 56 0.5 36 DEVOURER_TX_PWR" # RTL8821CU (Jaguar2) + "0xc812 0x0bda 24 104 0.25 36 DEVOURER_TX_PWR" # RTL8822CU (Jaguar3) + "0xa81a 0x0bda 24 104 tssi 36 DEVOURER_TX_PWR" # RTL8822EU (Jaguar3, TSSI-reshaped) + "0xf72b 0x0bda -64 0 tssi 36 DEVOURER_TX_PWR_OFFSET_QDB" # RTL8733BU (offset lever) ) plugged() { lsusb -d "$(printf '%04x:%04x' "$2" "$1")" >/dev/null 2>&1; } @@ -72,7 +91,7 @@ pick_ground() { # $1=dut_pid -> "pid vid" or "" } for dut in "${DUTS[@]}"; do - read -r PID VID START STOP NOMINAL DCH <<<"$dut" + read -r PID VID START STOP NOMINAL DCH KNOB <<<"$dut" if [ "$#" -gt 0 ]; then want=0; for p in "$@"; do [ "$p" = "$PID" ] && want=1; done [ "$want" = "1" ] || continue @@ -89,7 +108,8 @@ for dut in "${DUTS[@]}"; do read -r GPID GVID <<<"$ground" tag="${PID#0x}" CH="$DCH" - echo "== DUT $PID@$VID (ground $GPID): ch$CH cells $START..$STOP (nominal $NOMINAL dB/idx) ==" + unit="idx"; [ "$KNOB" = "DEVOURER_TX_PWR_OFFSET_QDB" ] && unit="qdB" + echo "== DUT $PID@$VID (ground $GPID): ch$CH cells $START..$STOP ($KNOB, nominal $NOMINAL dB/$unit) ==" # Ground RX for the whole DUT session, stream lines epoch-stamped. : >"$OUT/$tag-ground.log" @@ -109,7 +129,7 @@ for dut in "${DUTS[@]}"; do for idx in $idxs; do t0="$(date +%s.%N)" sudo -n env DEVOURER_PID="$PID" DEVOURER_VID="$VID" \ - DEVOURER_CHANNEL="$CH" DEVOURER_TX_PWR="$idx" \ + DEVOURER_CHANNEL="$CH" "$KNOB=$idx" \ DEVOURER_TX_GAP_US=2000 \ timeout 14 "$ROOT/build/txdemo" >"$OUT/$tag-cell$idx.log" 2>&1 || true t1="$(date +%s.%N)" @@ -121,10 +141,11 @@ for dut in "${DUTS[@]}"; do # Slope fit: per cell, median ground RSSI (chain A) of the canonical-SA # rx.frame events in [t0+6, t1-1] (bring-up transmits nothing at first). - python3 - "$OUT/$tag-ground.log" "$OUT/$tag-cells.txt" "$NOMINAL" >"$OUT/$tag-fit.txt" 2>&1 <<'PYEOF' + python3 - "$OUT/$tag-ground.log" "$OUT/$tag-cells.txt" "$NOMINAL" "$unit" >"$OUT/$tag-fit.txt" 2>&1 <<'PYEOF' import re, statistics, sys ground_log, cells_txt = sys.argv[1], sys.argv[2] tssi = sys.argv[3] == "tssi" +unit = sys.argv[4] if len(sys.argv) > 4 else "idx" nominal = None if tssi else float(sys.argv[3]) frames = [] rx = re.compile(r'^([0-9.]+) .*"ev":"rx\.frame".*"rssi":\[(-?\d+),(-?\d+)\]') @@ -152,11 +173,11 @@ span_db = slope * (pts[-1][0] - pts[0][0]) detail = " ".join(f"{x}:{y:.1f}" for x, y in pts) if tssi: ok = 0.15 <= slope <= 1.0 - print(f"RESULT slope={slope:.3f} dB/idx (TSSI-reshaped lever; monotone " + print(f"RESULT slope={slope:.3f} dB/{unit} (TSSI-reshaped lever; monotone " f"0.15..1.0) rms_resid={rms:.2f} dB span={span_db:.1f} dB pts={n} [{detail}]") sys.exit(0 if ok else 1) klass = 0.5 if abs(slope - 0.5) < abs(slope - 0.25) else 0.25 -print(f"RESULT slope={slope:.3f} dB/idx (nominal {nominal}, classified {klass}) " +print(f"RESULT slope={slope:.3f} dB/{unit} (nominal {nominal}, classified {klass}) " f"rms_resid={rms:.2f} dB span={span_db:.1f} dB pts={n} [{detail}]") sys.exit(0 if klass == nominal and rms < 2.5 else 1) PYEOF From 7bf9e56fec7b3e5f4d80ee1310eac7692d32b42a Mon Sep 17 00:00:00 2001 From: snokvist Date: Sun, 16 Aug 2026 08:00:51 +0200 Subject: [PATCH 3/8] rtl8733b: close the gaps an adversarial pass found in the TX-power knob MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review before upstreaming. Four findings, one of which reintroduced the very defect this work exists to remove. 1. A latched offset died silently on a flat-PG unit. SetTxPowerOffsetQdb accepts an offset before InitWrite and returns it as applied — the demos' ordering, and the documented family contract. configure_tx_power's non-TSSI branch returned right after set_flat_tx_power and never read _tx_offset_qdb, so on a unit with no TSSI calibration the caller got a non-zero "applied" for a value no register would ever carry. That is the same indistinguishable-from-success shape the issue is about, on the one path that has no actuator. The branch now drops the latch loudly and zeroes it, so the reported state agrees with the chip. Not fatal — an unported optional knob is not a hardware-safety event. No hardware coverage: every unit seen is TSSI-offset PG, and the path writes no registers. 2. SetTxPowerIndexOverride was silently ignored while the docs claimed the flat index was "refused". IRtlDevice's default returns void and drops the value, so on this backend — where the index genuinely is unported — the caller's only answer was silence, which is exactly what this header's own rule forbids ("unsupported optional controls refuse loudly rather than silently no-opping"). Overridden now to log the refusal and name the knob that does work. Verified on the device: --flat 32 logs the refusal. SetTxPowerRateDiffs keeps the default; its `false` return already says it. 3. hw_readback compared one shadow against the chip. tssi_offsets_confirmed() checked the PHY's own _fr_tssi_offsets bookkeeping, which cannot catch a disagreement between the device's session state and the PHY's. It now recomputes the expected plan from (channel, ceiling, offset) — the offset the CALLER believes is applied — so the check is chip-versus-claim rather than shadow-versus-itself. 4. GetTxPowerState warned on every call. It is a getter a control loop polls (waybeam-link serves GET /api/v1/tx/power from it), so an unconfirmed chip would emit one warning per poll forever. Latched to once; the state field is the machine-readable signal. Also documents, at the configure_tx_power call site, that a positive session offset deliberately lifts bring-up targets above the safe clip — it is a widening of what bring-up may program, not a hole in enable_tssi_tracking's `max_target_qdbm > 64` guard, which still refuses a raised ceiling argument. And drops an unused `steps` out-param. Verified: ctest 54/54, tests/rtl8733b_txpwr_regcheck.sh 7/7 on the device with hw_readback still confirming after a hop under the stricter check. Co-Authored-By: Claude Opus 5 (1M context) --- src/rtl8733b/CLAUDE.md | 25 ++++++++++----- src/rtl8733b/Phy8733b.cpp | 13 ++++++-- src/rtl8733b/Phy8733b.h | 15 ++++++--- src/rtl8733b/Rtl8733bDevice.cpp | 54 +++++++++++++++++++++++++++++---- src/rtl8733b/Rtl8733bDevice.h | 8 +++++ 5 files changed, 94 insertions(+), 21 deletions(-) diff --git a/src/rtl8733b/CLAUDE.md b/src/rtl8733b/CLAUDE.md index 201779f..7db6fbc 100644 --- a/src/rtl8733b/CLAUDE.md +++ b/src/rtl8733b/CLAUDE.md @@ -214,13 +214,24 @@ loudly — without tearing the session down, since an unported optional knob is not a hardware-safety event — while `false` succeeds as a no-op because that is the state MAC bring-up already leaves programmed. -`SetTxPowerOffsetQdb` is the second exception, in the same spirit: on a unit -whose EFUSE carries no TSSI calibration there is no actuator at all, and the -call **refuses loudly and returns 0** rather than reporting a successful -zero-offset apply. That indistinguishability is what this knob exists to end — -a consumer measured 18 dB of commanded offset moving nothing while its state -read `{"applied_qdb":0,"saturated_low":false}`, which is exactly what a healthy -actuator with travel remaining looks like. +The TX-power knobs are the other exceptions, in the same spirit: + +- `SetTxPowerOffsetQdb` **refuses loudly and returns 0** on a unit whose EFUSE + carries no TSSI calibration, rather than reporting a successful zero-offset + apply. That indistinguishability is what this knob exists to end — a consumer + measured 18 dB of commanded offset moving nothing while its state read + `{"applied_qdb":0,"saturated_low":false}`, which is exactly what a healthy + actuator with travel remaining looks like. An offset latched *before* + bring-up on such a unit is **dropped loudly and zeroed** by + `configure_tx_power`, so the reported state never claims an offset no + register carries. (That path has no hardware coverage — every unit seen so + far is TSSI-offset PG — but it writes no registers, only a log and a reset.) +- `SetTxPowerIndexOverride` is overridden **solely to log a refusal**. The + `IRtlDevice` default returns `void` and ignores the value, so silence would + be the caller's only answer on the one backend where the flat index really is + unported — a knob that looks granted, in the PR that exists to abolish them. + `SetTxPowerRateDiffs` needs no such override: its `false` return already says + it. `DeviceConfig::tuning::disable_cca` cannot be honoured either, and bring-up warns rather than dropping it — a config knob must not be the one door where a diff --git a/src/rtl8733b/Phy8733b.cpp b/src/rtl8733b/Phy8733b.cpp index ada0832..e2deac5 100644 --- a/src/rtl8733b/Phy8733b.cpp +++ b/src/rtl8733b/Phy8733b.cpp @@ -1457,12 +1457,19 @@ bool Phy8733b::set_tssi_offset(SelectedChannel channel, return ok; } -bool Phy8733b::tssi_offsets_confirmed() { - if (!_initialized || !_tssi_digital_snapshot || !_fr_tssi_offsets) +bool Phy8733b::tssi_offsets_confirmed(SelectedChannel channel, + uint8_t max_target_qdbm, + int offset_qdb) { + if (!_initialized || !_tssi_digital_snapshot) + return false; + const auto plan = + tssi_bb_plan(_tx_power_targets, channel.Channel, _rfe_type, + _fr_tssi_path, max_target_qdbm, offset_qdb); + if (!plan) return false; /* read_txagc_state's rate_diffs array IS 0x3a00..0x3a10 — the same five * dwords that carry the loop's per-rate targets while tracking is on. */ - return read_txagc_state().rate_diffs == *_fr_tssi_offsets; + return read_txagc_state().rate_diffs == plan->rate_offsets; } bool Phy8733b::disable_tssi_tracking() { diff --git a/src/rtl8733b/Phy8733b.h b/src/rtl8733b/Phy8733b.h index e48de4b..2ab4935 100644 --- a/src/rtl8733b/Phy8733b.h +++ b/src/rtl8733b/Phy8733b.h @@ -234,11 +234,16 @@ class Phy8733b { * the knob. */ bool set_tssi_offset(SelectedChannel channel, uint8_t max_target_qdbm, int offset_qdb, TssiOffsetSat8733b *sat = nullptr); - /* Does the chip's live per-rate target table still match what this session - * believes it wrote? Six register reads (the 0x3a00 dwords via - * read_txagc_state), so a caller can poll it at its own cadence — the - * chip-truth half of GetTxPowerState on the TSSI path. */ - bool tssi_offsets_confirmed(); + /* Does the chip's live per-rate target table match the plan for the offset + * the CALLER believes is applied? Recomputed from (channel, ceiling, offset) + * rather than compared against this class's own `_fr_tssi_offsets` shadow, + * so a disagreement between the device's session state and the PHY's + * bookkeeping is caught too — a shadow checked against itself always agrees, + * which is the whole failure this API is fixing. Six register reads (the + * 0x3a00 dwords via read_txagc_state); the chip-truth half of + * GetTxPowerState on the TSSI path. */ + bool tssi_offsets_confirmed(SelectedChannel channel, uint8_t max_target_qdbm, + int offset_qdb); /* Lean intra-band, same-bandwidth hop — the FastRetune core (see * docs/frequency-hopping.md; profile that sized it: full set_channel on * this USB-HS part is ~330 ms, of which ~165 ms is the TSSI diff --git a/src/rtl8733b/Rtl8733bDevice.cpp b/src/rtl8733b/Rtl8733bDevice.cpp index 4849ada..437e419 100644 --- a/src/rtl8733b/Rtl8733bDevice.cpp +++ b/src/rtl8733b/Rtl8733bDevice.cpp @@ -156,8 +156,27 @@ bool Rtl8733bDevice::configure_tx_power(SelectedChannel channel) { * survive the link (measured on the DUT: MCS7 undecodable by an RTL8812AU * witness, 300/300 submitted, 0 captured). A unit whose EFUSE carries no * TSSI calibration has nothing to drive the loop and takes the flat path. */ - if (_efuse.tx_power_mode != rtl8733b::TxPowerPgMode8733b::TssiOffset) + if (_efuse.tx_power_mode != rtl8733b::TxPowerPgMode8733b::TssiOffset) { + /* No loop, so no actuator. A runtime offset latched BEFORE bring-up (the + * demos' ordering: DEVOURER_TX_PWR_OFFSET_QDB is applied before InitWrite) + * was accepted on the promise of being applied here, and there is nothing + * to apply it to. Drop it loudly and zero the latch so the reported state + * agrees with the chip — leaving it set would make GetTxPowerState claim an + * offset no register carries, which is the exact failure this knob exists + * to end. Not fatal: an unported optional knob is not a hardware-safety + * event. */ + if (_tx_offset_qdb != 0) { + _logger->error( + "RTL8733B: dropping the {} qdB TX-power offset — this unit's EFUSE " + "carries no TSSI calibration, so TX power is the fixed flat index " + "and has no runtime actuator", + _tx_offset_qdb); + _tx_offset_qdb = 0; + _tx_sat_low = false; + _tx_sat_high = false; + } return _phy.set_flat_tx_power(rtl8733b::kSafeTxAgcIndex8733b); + } /* Pick the thermal-compensation curve once, from the TX mode configured at * this point, and leave it alone — the vendor's own setup keys the table on @@ -188,6 +207,12 @@ bool Rtl8733bDevice::configure_tx_power(SelectedChannel channel) { if (!_phy.prepare_tssi_bb(channel, _efuse) || !_phy.prepare_tssi_thermal(_efuse, cck_table) || !_phy.prepare_tssi_offsets(channel, _efuse) || + /* The ceiling argument stays the safe first-light value; a POSITIVE + * session offset therefore lifts the resulting targets above it here, by + * design (GetTxPowerCaps argues why the positive half exists). That is a + * deliberate widening of what bring-up can program, not a hole in + * enable_tssi_tracking's `max_target_qdbm > 64` guard, which still refuses + * a caller that tries to raise the ceiling itself. */ !_phy.enable_tssi_tracking(channel, _efuse, rtl8733b::kSafeTssiTargetQdbm8733b, _tx_offset_qdb)) @@ -752,11 +777,21 @@ devourer::TxPowerCaps Rtl8733bDevice::GetTxPowerCaps() { return c; } +void Rtl8733bDevice::SetTxPowerIndexOverride(int idx) { + /* kSafeTxAgcIndex8733b is the only flat index this backend programs, and it + * was witnessed unable to carry HT at all (MCS7, 300/300 submitted, 0 + * captured, twice), so no dB-per-step slope has ever been measured for the + * index on this part. SetTxPowerOffsetQdb is the ported lever. */ + _logger->error("RTL8733B: SetTxPowerIndexOverride({}) ignored — the flat " + "TXAGC index is not ported on this backend; use " + "SetTxPowerOffsetQdb (GetTxPowerCaps reports the dBm model)", + idx); +} + int Rtl8733bDevice::SetTxPowerOffsetQdb(int qdb) { std::lock_guard lock(_reg_mu); const devourer::TxPowerCaps caps = GetTxPowerCaps(); - int steps = 0; - const int applied = devourer::quantize_offset_qdb(qdb, caps, &steps); + const int applied = devourer::quantize_offset_qdb(qdb, caps, nullptr); const bool req_low = qdb < caps.offset_min_qdb; const bool req_high = qdb > caps.offset_max_qdb; @@ -832,12 +867,19 @@ devourer::TxPowerState Rtl8733bDevice::GetTxPowerState() { s.offset_steps = _tx_offset_qdb; /* 1 step == 1 qdB on the dBm model */ s.saturated_low = _tx_sat_low; s.saturated_high = _tx_sat_high; - s.hw_readback = _phy.tssi_offsets_confirmed(); - if (!s.hw_readback) + s.hw_readback = _phy.tssi_offsets_confirmed( + _channel, rtl8733b::kSafeTssiTargetQdbm8733b, _tx_offset_qdb); + /* Latched, because this is a getter a control loop polls: an unconfirmed + * chip would otherwise emit one warning per poll forever. The state field is + * the machine-readable signal; the log line only has to fire the first + * time. */ + if (!s.hw_readback && !_tx_readback_warned) { + _tx_readback_warned = true; _logger->warn("RTL8733B: TX-power state unconfirmed — the chip's TSSI " "target table does not match the {} qdB offset this session " - "believes it applied", + "believes it applied (warned once)", _tx_offset_qdb); + } return s; } diff --git a/src/rtl8733b/Rtl8733bDevice.h b/src/rtl8733b/Rtl8733bDevice.h index 8415f40..11b423f 100644 --- a/src/rtl8733b/Rtl8733bDevice.h +++ b/src/rtl8733b/Rtl8733bDevice.h @@ -56,6 +56,13 @@ class Rtl8733bDevice : public IRtlDevice { devourer::TxPowerCaps GetTxPowerCaps() override; int SetTxPowerOffsetQdb(int qdb) override; devourer::TxPowerState GetTxPowerState() override; + /* Overridden only to refuse out loud. IRtlDevice's default returns void and + * ignores the value, so on this backend — where the flat index is genuinely + * unported — silence would be the caller's only answer, and a knob that + * looks granted is precisely the defect this family's offset knob was added + * to fix. SetTxPowerRateDiffs needs no such override: its `false` return + * already says it. */ + void SetTxPowerIndexOverride(int idx) override; devourer::TxStats GetTxStats() override { return _device.GetTxStats(); } devourer::ThermalStatus GetThermalStatus() override; bool GetPermanentMacAddress(uint8_t out[6]) override; @@ -90,6 +97,7 @@ class Rtl8733bDevice : public IRtlDevice { int16_t _tx_offset_qdb = 0; bool _tx_sat_low = false; bool _tx_sat_high = false; + bool _tx_readback_warned = false; std::atomic _rx_stop{false}; std::atomic _rx_active{false}; std::atomic _rx_configured_bw{0}; From 677a0ec63953a5c211762e9cc864fee1fb3837c3 Mon Sep 17 00:00:00 2001 From: snokvist Date: Sun, 16 Aug 2026 08:01:54 +0200 Subject: [PATCH 4/8] rtl8733b: publish the offset rail flags only on success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tssi_rate_offsets wrote into the caller's TssiOffsetSat8733b as it walked the rate table, so a table that failed partway (a missing non-CCK target) returned nullopt with rail flags already set from the rates computed before it. No caller reads sat on the nullopt path today, so nothing was wrong on the wire — but the contract read badly and the fix is a local accumulator published at the end. ctest 54/54. Co-Authored-By: Claude Opus 5 (1M context) --- src/rtl8733b/Phy8733b.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/rtl8733b/Phy8733b.cpp b/src/rtl8733b/Phy8733b.cpp index e2deac5..8905df6 100644 --- a/src/rtl8733b/Phy8733b.cpp +++ b/src/rtl8733b/Phy8733b.cpp @@ -433,6 +433,10 @@ Phy8733b::tssi_rate_offsets(const TxPowerTargets8733b &targets, uint8_t band, if (band > 1 || path > 1 || !targets.present[band][path]) return std::nullopt; std::array offsets{}; + /* Accumulated locally and published only on success: a caller that gets + * nullopt must not find rail flags set by the rates that were computed + * before the one that failed. */ + TssiOffsetSat8733b rails; for (size_t rate = 0; rate < offsets.size(); ++rate) { const uint8_t target = targets.qdbm[band][path][rate]; if (target == 0xff) { @@ -459,8 +463,7 @@ Phy8733b::tssi_rate_offsets(const TxPowerTargets8733b &targets, uint8_t band, * a closed-loop controller needs to be told rather than handed a wrapped * value. */ shifted = 0; - if (sat) - sat->low = true; + rails.low = true; } /* Floored at 0 above, so the delta cannot go below -64: only the int8 * field's positive end needs clamping, and it is reachable only with a @@ -468,11 +471,12 @@ Phy8733b::tssi_rate_offsets(const TxPowerTargets8733b &targets, uint8_t band, int delta = shifted - 64; if (delta > 127) { delta = 127; - if (sat) - sat->high = true; + rails.high = true; } offsets[rate] = static_cast(delta); } + if (sat) + *sat = rails; return offsets; } From ec7c8d3332ae8d1c796658196be97bf6cd6c39c4 Mon Sep 17 00:00:00 2001 From: snokvist Date: Sun, 16 Aug 2026 08:16:56 +0200 Subject: [PATCH 5/8] rtl8733b: say correctly why the offset range is asymmetric MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docs claimed -64 qdB was where "the int8 delta field bottoms out". It is not. The 0x3a00 bytes hold a signed int8 offset from the 64 qdBm anchor, so the field spans [-128, +127] — targets from -16 dBm to +47.75 dBm. Only the +127 end is a hardware limit. -64 is a judgement: it is where the ABSOLUTE target reaches 0 dBm, and stopping there is backed by the sweep rather than by the register width. The bottom 12 qdB of travel already delivers only 0.125 dB/qdB against 0.25 nominal — the one structure that reproduced exactly across both passes — so the loop is visibly running out of authority as its target approaches zero, and nothing below 0 dBm has been characterised. Extending into that region would hand a controller commands the loop cannot act on, which is the defect this lever exists to remove; saturated_low marks the boundary instead. Corrected at all four sites that had it wrong: the GetTxPowerCaps comment, src/rtl8733b/CLAUDE.md, docs/rtl8733b.md, and the regcheck script header, plus the TssiOffsetSat8733b declaration (its `low` flag is the absolute floor, not the field's -128). Comment and documentation only — no behaviour change, and the code always computed it this way (tssi_rate_offsets floors `shifted` at 0 and notes the delta therefore cannot go below -64). ctest 54/54. Co-Authored-By: Claude Opus 5 (1M context) --- docs/rtl8733b.md | 22 ++++++++++++++++------ src/rtl8733b/CLAUDE.md | 15 ++++++++++----- src/rtl8733b/Phy8733b.h | 8 ++++---- src/rtl8733b/Rtl8733bDevice.cpp | 17 +++++++++++++---- tests/rtl8733b_txpwr_regcheck.sh | 5 +++-- 5 files changed, 46 insertions(+), 21 deletions(-) diff --git a/docs/rtl8733b.md b/docs/rtl8733b.md index 61df3e0..3217fcc 100644 --- a/docs/rtl8733b.md +++ b/docs/rtl8733b.md @@ -148,12 +148,22 @@ shape `fast_retune` uses for its per-channel rewrite (#389); the alternative, a TSSI disable/re-enable pair, costs ~165 ms and buys nothing here. The capability report is the dBm model — `index_max = 0`, one qdB per step — -over `[-64, +127]`, the int8 per-rate delta field's range and the same -clamped-only-at-the-hardware-rail answer Jaguar1 (±126) and Jaguar3 (±127) -give. Offset 0 is `kSafeTssiTargetQdbm8733b` (16 dBm), a first-light clip -sitting at or below this part's factory targets (18–20 dBm at 2.4 GHz, -16–19 at 5 GHz); −64 qdB puts the target at 0 qdBm, where the delta field -bottoms out at the same moment. +over `[-64, +127]`. Offset 0 is `kSafeTssiTargetQdbm8733b` (16 dBm), a +first-light clip sitting at or below this part's factory targets (18–20 dBm at +2.4 GHz, 16–19 at 5 GHz). + +That range is asymmetric, and only one end is a hardware limit. The +`0x3a00..0x3a10` bytes hold a **signed int8 offset from the 64 qdBm anchor**, so +the field itself spans [−128, +127] — targets from −16 dBm to +47.75 dBm. + +- **+127 is the field**, the same clamped-only-at-the-hardware-rail answer + Jaguar1 (±126) and Jaguar3 (±127) give. +- **−64 is a judgement, not the field's floor** (−128 is). It is where the + absolute target reaches 0 dBm. Stopping there is backed by the sweep: the + bottom 12 qdB of travel delivers only 0.125 dB/qdB against 0.25 nominal — + reproduced exactly across two passes — so the loop is already running out of + authority as its target approaches zero, and nothing below 0 dBm has been + characterised. `saturated_low` marks that boundary. The positive half is deliberately **not** re-clamped at the factory table. `src/TxPower.h` states for every family that headroom above the generated diff --git a/src/rtl8733b/CLAUDE.md b/src/rtl8733b/CLAUDE.md index 7db6fbc..d51e487 100644 --- a/src/rtl8733b/CLAUDE.md +++ b/src/rtl8733b/CLAUDE.md @@ -60,11 +60,16 @@ unrelated register map. the five packed dwords at `0x3a00..0x3a10`, rewritten **in place with tracking live**, the same #389 shape `fast_retune` uses — not the ~165 ms disable/re-enable pair. Caps therefore report the dBm model (`index_max = 0`, - one qdB per step) over `[-64, +127]` — the int8 delta field's range, the same - clamped-only-at-the-hardware-rail answer Jaguar1/3 give. Offset 0 is the safe - clip, −64 qdB puts the target at 0 qdBm where the field bottoms out, and the - positive half is deliberately NOT re-clamped at the PG table: an EFUSE - trimmed too cold is what an operator calibrates their way out of. + one qdB per step) over `[-64, +127]`, and only the top of that is a hardware + limit. The 0x3a00 bytes are a signed int8 offset from the 64 qdBm anchor, so + the field spans [-128, +127] — targets from −16 to +47.75 dBm. **+127 is the + field**, the same clamped-only-at-the-hardware-rail answer Jaguar1/3 give; + **−64 is a judgement**, the point where the absolute target reaches 0 dBm. + Stopping there is backed by the sweep: the bottom 12 qdB already delivers + 0.125 dB/qdB against 0.25 nominal, so the loop is running out of authority + as the target nears zero, and below it nothing is characterised. The positive + half is deliberately NOT re-clamped at the PG table either: an EFUSE trimmed + too cold is what an operator calibrates their way out of. `SetTxPowerIndexOverride`, `SetTxPowerRateDiffs` and `ReApplyTxPower` stay unported. - **Overdrive above the clip buys ~3 dB and then the PA compresses, and EVM is diff --git a/src/rtl8733b/Phy8733b.h b/src/rtl8733b/Phy8733b.h index 2ab4935..0ea1269 100644 --- a/src/rtl8733b/Phy8733b.h +++ b/src/rtl8733b/Phy8733b.h @@ -91,10 +91,10 @@ inline constexpr uint8_t kMaxPgTargetQdbm8733b = 80; /* Which rail the runtime TX-power offset clamped at, if any — the signal a * closed-loop controller uses to know the knob has run out of travel * (IRtlDevice::GetTxPowerState). `low` is set when a rate's shifted target hit - * 0 qdBm or the int8 delta field's floor; `high` when it hit that field's - * ceiling. Both are per-rate facts: a shift can rail one rate while the rest - * still move, which is exactly what a shape-preserving offset does at the end - * of its range. */ + * the 0 qdBm floor (the low bound is that absolute floor, not the int8 field's + * -128); `high` when a rate hit the int8 field's +127 ceiling. Both are + * per-rate facts: a shift can rail one rate while the rest still move, which + * is exactly what a shape-preserving offset does at the end of its range. */ struct TssiOffsetSat8733b { bool low = false; bool high = false; diff --git a/src/rtl8733b/Rtl8733bDevice.cpp b/src/rtl8733b/Rtl8733bDevice.cpp index 437e419..7ca7184 100644 --- a/src/rtl8733b/Rtl8733bDevice.cpp +++ b/src/rtl8733b/Rtl8733bDevice.cpp @@ -712,11 +712,20 @@ devourer::AdapterCaps Rtl8733bDevice::GetAdapterCaps() { * * Offset 0 is kSafeTssiTargetQdbm8733b (16 dBm), a first-light clip the backend * imposes at or below this part's factory targets — which run 18..20 dBm at - * 2.4 GHz and 16..19 dBm at 5 GHz (kMaxPgTargetQdbm8733b). The range around it - * is the delta field's, not a characterised PA window: + * 2.4 GHz and 16..19 dBm at 5 GHz (kMaxPgTargetQdbm8733b). * - * - Down to -64 qdB, where the target reaches 0 qdBm and the per-rate delta - * field bottoms out at the same moment. + * The range around it is ASYMMETRIC, and only one end is a hardware limit. The + * 0x3a00 bytes are a signed int8 offset from the 64 qdBm anchor, so the field + * itself spans [-128, +127] — targets from -16 dBm to +47.75 dBm: + * + * - Down to -64 qdB, which is NOT the field's floor (-128 is). It is where + * the absolute target reaches 0 dBm, and stopping there is a judgement + * backed by the sweep: the bottom 12 qdB of travel already delivers only + * 0.125 dB/qdB against 0.25 nominal — reproduced exactly across two passes + * — so the loop is visibly running out of authority as its target + * approaches zero. Below that nothing is characterised, and a knob that + * keeps accepting commands it cannot act on is the defect this whole + * lever exists to remove. saturated_low marks that boundary. * - Up to +127 qdB, the delta field's positive limit — the same * clamped-only-at-the-hardware-rail answer Jaguar1 (+126) and Jaguar3 * (+127) give. src/TxPower.h is deliberate that headroom above the diff --git a/tests/rtl8733b_txpwr_regcheck.sh b/tests/rtl8733b_txpwr_regcheck.sh index 7b91574..0d75a50 100755 --- a/tests/rtl8733b_txpwr_regcheck.sh +++ b/tests/rtl8733b_txpwr_regcheck.sh @@ -22,8 +22,9 @@ # actuator, so an on-air null in this geometry is the code and not # the bench. # move -24 qdB shifts every capped rate byte by exactly -24 (0xe8). -# rails -200 clamps to -64 with saturated_low, +200 to +127 with -# saturated_high - the int8 per-rate delta field's range. +# rails -200 clamps to -64 with saturated_low (the 0 dBm absolute target +# floor, NOT the int8 field's -128), +200 to +127 with +# saturated_high (that one IS the field's ceiling). # sticky -24 qdB then a full SetMonitorChannel to another 5 GHz group # re-folds the offset against the new channel; a following # FastRetune leaves it in place (the hop rewrites the target table, From 03daab56763c94aa3a5d1702f875b6c1ed787d96 Mon Sep 17 00:00:00 2001 From: snokvist Date: Sun, 16 Aug 2026 08:27:16 +0200 Subject: [PATCH 6/8] rtl8733b: the backoff floor is the delta field too, worth 9 dB more MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review question: if "the field is the limit and the operator judges" holds at the top of the offset range, why does the bottom stop at -64? It did not have a good answer. -64 was a 0 dBm absolute target, clamped there on the assumption that a negative target was meaningless — an assumption, not a measurement, in a change whose entire subject is knobs that assume instead of checking. Measured it instead (MCS0, ch36, witness RSSI + EVM): offset target RSSI EVM -32 8.0 dBm 69.40 -60.8 -64 0.0 dBm 62.07 -59.3 <- the old clamp -80 -4.0 dBm 54.80 -54.0 -96 -8.0 dBm 52.96 -53.1 -112 -12.0 dBm 52.97 -52.8 -128 -16.0 dBm 52.99 -52.8 Power keeps falling straight past the 0 dBm target — 7.3 dB more between it and -4 dBm — with no sign wrap, and pins from about -96 qdB where three successive rungs agree within 0.03 dB. EVM softens from -59 to -53 across the extension and the link stays decodable. The old clamp was throwing away ~9 dB of working backoff: real range for near-field bench work, where an over-strong witness inverts evidence, and for a link that wants to sit quiet. So the floor becomes the int8 delta field (-128, a -16 dBm target), symmetric with the +127 ceiling, and ~-96 qdB is documented as the measured end of usable travel rather than enforced — exactly how the +32 qdB PA-compression knee is handled at the other end. saturated_low now means the field rail rather than the 0 dBm target, and the selftest says so: -64 rails nothing and the ladder keeps its calibrated spread through it, -100 still preserves that spread, -128 rails the colder rate while the hot one sits exactly on the field floor. Verified on the device: regcheck 7/7 (rails cell now -200 -> -128), on-air 0.223 dB/qdB over the shipped -64..0 range, ctest 54/54. Co-Authored-By: Claude Opus 5 (1M context) --- docs/rtl8733b.md | 35 ++++++++++++++++++++++++------- src/rtl8733b/CLAUDE.md | 28 ++++++++++++++++--------- src/rtl8733b/Phy8733b.cpp | 19 ++++++++++------- src/rtl8733b/Rtl8733bDevice.cpp | 28 +++++++++++++------------ tests/rtl8733b_txpwr_regcheck.sh | 15 ++++++------- tests/rtl8733b_txpwr_selftest.cpp | 32 ++++++++++++++++++++++------ 6 files changed, 105 insertions(+), 52 deletions(-) diff --git a/docs/rtl8733b.md b/docs/rtl8733b.md index 3217fcc..380d784 100644 --- a/docs/rtl8733b.md +++ b/docs/rtl8733b.md @@ -156,14 +156,11 @@ That range is asymmetric, and only one end is a hardware limit. The `0x3a00..0x3a10` bytes hold a **signed int8 offset from the 64 qdBm anchor**, so the field itself spans [−128, +127] — targets from −16 dBm to +47.75 dBm. -- **+127 is the field**, the same clamped-only-at-the-hardware-rail answer - Jaguar1 (±126) and Jaguar3 (±127) give. -- **−64 is a judgement, not the field's floor** (−128 is). It is where the - absolute target reaches 0 dBm. Stopping there is backed by the sweep: the - bottom 12 qdB of travel delivers only 0.125 dB/qdB against 0.25 nominal — - reproduced exactly across two passes — so the loop is already running out of - authority as its target approaches zero, and nothing below 0 dBm has been - characterised. `saturated_low` marks that boundary. +Both ends are that field, and neither is re-clamped at something softer — the +same clamped-only-at-the-hardware-rail answer Jaguar1 (±126) and Jaguar3 (±127) +give. Where the chip stops responding is measured and written down below rather +than enforced in the setter, because on this part both ends stop responding well +inside the field and at points only a bench can find. The positive half is deliberately **not** re-clamped at the factory table. `src/TxPower.h` states for every family that headroom above the generated @@ -201,6 +198,28 @@ geometry, integer-quantised RSSI, no SDR. Harnesses: `tests/txpwr_offset_onair.sh` (slope), `tests/rtl8733b_txpwr_selftest.cpp` (the offset math, in `ctest`). +### The backoff floor is about −96 qdB, not the 0 dBm target + +An earlier cut of this work clamped the offset at −64 qdB, reasoning that a +target below 0 dBm was meaningless. Sweeping past it says otherwise (MCS0, +ch36): + +| offset | target | witness RSSI | witness EVM | +|---|---|---|---| +| −32 | 8.0 dBm | 69.40 | −60.8 | +| −64 | 0.0 dBm | 62.07 | −59.3 | +| −80 | −4.0 dBm | **54.80** | −54.0 | +| −96 | −8.0 dBm | 52.96 | −53.1 | +| −112 | −12.0 dBm | 52.97 | −52.8 | +| −128 | −16.0 dBm | 52.99 | −52.8 | + +Power keeps falling past the 0 dBm target — another **7.3 dB** between it and +−4 dBm — with no sign wrap, and pins from about **−96 qdB**, where three +successive rungs agree within 0.03 dB. EVM softens from −59 to −53 across the +whole extension and the link stays decodable throughout. So usable travel is +~23 dB below the clip rather than the ~16 dB the first cut allowed, which +matters for near-field bench work and for a link that wants to sit quiet. + ### Overdrive: about 3 dB, and then the PA compresses Sweeping the other way — up from the clip, MCS0 at ch36, with the witness diff --git a/src/rtl8733b/CLAUDE.md b/src/rtl8733b/CLAUDE.md index d51e487..37b1f93 100644 --- a/src/rtl8733b/CLAUDE.md +++ b/src/rtl8733b/CLAUDE.md @@ -60,18 +60,26 @@ unrelated register map. the five packed dwords at `0x3a00..0x3a10`, rewritten **in place with tracking live**, the same #389 shape `fast_retune` uses — not the ~165 ms disable/re-enable pair. Caps therefore report the dBm model (`index_max = 0`, - one qdB per step) over `[-64, +127]`, and only the top of that is a hardware - limit. The 0x3a00 bytes are a signed int8 offset from the 64 qdBm anchor, so - the field spans [-128, +127] — targets from −16 to +47.75 dBm. **+127 is the - field**, the same clamped-only-at-the-hardware-rail answer Jaguar1/3 give; - **−64 is a judgement**, the point where the absolute target reaches 0 dBm. - Stopping there is backed by the sweep: the bottom 12 qdB already delivers - 0.125 dB/qdB against 0.25 nominal, so the loop is running out of authority - as the target nears zero, and below it nothing is characterised. The positive - half is deliberately NOT re-clamped at the PG table either: an EFUSE trimmed - too cold is what an operator calibrates their way out of. + one qdB per step) over `[-128, +127]` — the int8 delta field at both ends, + the same clamped-only-at-the-hardware-rail answer Jaguar1/3 give. The 0x3a00 + bytes are a signed offset from the 64 qdBm anchor, so that field spans + targets from −16 to +47.75 dBm. Neither end is re-clamped at something + softer: not at the PG table on the way up (an EFUSE trimmed too cold is what + an operator calibrates their way out of), and not at a 0 dBm target on the + way down (an earlier cut did, and it cost ~9 dB of working backoff — see + below). Where the chip stops *responding* is measured and documented, not + enforced. `SetTxPowerIndexOverride`, `SetTxPowerRateDiffs` and `ReApplyTxPower` stay unported. +- **The backoff floor is ~−96 qdB, not the 0 dBm target.** Sweeping past the + old −64 clamp (MCS0, ch36): the 0 dBm target read 62.07, and −80 qdB + (−4 dBm) read **54.80 — another 7.3 dB down, with no sign wrap**. It pins + from about −96 qdB (−8 dBm): 52.96 / 52.97 / 52.99 at −96 / −112 / −128, + flat within 0.03 dB. So the usable travel is ~23 dB below the 16 dBm clip, + not the ~16 dB the first cut allowed, and EVM only softens from −59 to −53 + across it. The lesson is the one this whole knob is about: the earlier floor + was a guess about what a negative absolute target *must* mean, and the guess + was worth 9 dB. - **Overdrive above the clip buys ~3 dB and then the PA compresses, and EVM is the only tell.** Sweeping UP from the clip (MCS0, ch36, witness reporting EVM beside RSSI): +16 qdB — the top of the PG table — gave +2.8 dB with EVM diff --git a/src/rtl8733b/Phy8733b.cpp b/src/rtl8733b/Phy8733b.cpp index 8905df6..f5bdf68 100644 --- a/src/rtl8733b/Phy8733b.cpp +++ b/src/rtl8733b/Phy8733b.cpp @@ -458,16 +458,19 @@ Phy8733b::tssi_rate_offsets(const TxPowerTargets8733b &targets, uint8_t band, * measured operating point cannot be commanded. */ int shifted = static_cast((std::min)(target, max_target_qdbm)) + offset_qdb; - if (shifted < 0) { - /* Below 0 qdBm the target has no meaning left — that is the low rail, and - * a closed-loop controller needs to be told rather than handed a wrapped - * value. */ - shifted = 0; + /* Both rails are the int8 delta field, nothing softer. The field is signed + * against the 64 qdBm anchor, so it spans targets from -16 dBm to + * +47.75 dBm, and the loop was measured to keep reducing power the whole + * way down to the bottom of it — no sign wrap, power still falling 7.3 dB + * between a 0 dBm and a -4 dBm target. Clamping at 0 dBm, as an earlier cut + * of this did, threw away ~9 dB of working backoff on the guess that a + * negative absolute target was meaningless. Where the loop actually stops + * responding (~-8 dBm) is a measured property documented in + * GetTxPowerCaps, not a limit imposed here. */ + if (shifted < -64) { + shifted = -64; rails.low = true; } - /* Floored at 0 above, so the delta cannot go below -64: only the int8 - * field's positive end needs clamping, and it is reachable only with a - * ceiling above the 64 qdBm anchor. */ int delta = shifted - 64; if (delta > 127) { delta = 127; diff --git a/src/rtl8733b/Rtl8733bDevice.cpp b/src/rtl8733b/Rtl8733bDevice.cpp index 7ca7184..4d6079b 100644 --- a/src/rtl8733b/Rtl8733bDevice.cpp +++ b/src/rtl8733b/Rtl8733bDevice.cpp @@ -714,18 +714,21 @@ devourer::AdapterCaps Rtl8733bDevice::GetAdapterCaps() { * imposes at or below this part's factory targets — which run 18..20 dBm at * 2.4 GHz and 16..19 dBm at 5 GHz (kMaxPgTargetQdbm8733b). * - * The range around it is ASYMMETRIC, and only one end is a hardware limit. The - * 0x3a00 bytes are a signed int8 offset from the 64 qdBm anchor, so the field - * itself spans [-128, +127] — targets from -16 dBm to +47.75 dBm: + * The range is the int8 delta field at BOTH ends, and neither end is a + * characterised PA window. The 0x3a00 bytes are a signed offset from the + * 64 qdBm anchor, so the field spans [-128, +127] — targets from -16 dBm to + * +47.75 dBm. Where the hardware stops responding is measured and documented + * per end rather than clamped, the same answer Jaguar1/3 give: * - * - Down to -64 qdB, which is NOT the field's floor (-128 is). It is where - * the absolute target reaches 0 dBm, and stopping there is a judgement - * backed by the sweep: the bottom 12 qdB of travel already delivers only - * 0.125 dB/qdB against 0.25 nominal — reproduced exactly across two passes - * — so the loop is visibly running out of authority as its target - * approaches zero. Below that nothing is characterised, and a knob that - * keeps accepting commands it cannot act on is the defect this whole - * lever exists to remove. saturated_low marks that boundary. + * - Down to -128 qdB, a -16 dBm target. An earlier cut stopped at -64 (a + * 0 dBm target) on the assumption that a negative absolute target was + * meaningless; the sweep says otherwise. Power keeps falling past it with + * no sign wrap — 7.3 dB more between the 0 dBm and -4 dBm targets — and + * only pins from about -96 qdB (-8 dBm), where three successive rungs read + * 52.96 / 52.97 / 52.99. Stopping at -64 threw away ~9 dB of working + * backoff, which is real range for a near-field bench or a link that wants + * to sit quiet. So the floor is the field, as at the top, and ~-96 qdB is + * the measured end of usable travel — documented, not enforced. * - Up to +127 qdB, the delta field's positive limit — the same * clamped-only-at-the-hardware-rail answer Jaguar1 (+126) and Jaguar3 * (+127) give. src/TxPower.h is deliberate that headroom above the @@ -777,8 +780,7 @@ devourer::TxPowerCaps Rtl8733bDevice::GetTxPowerCaps() { c.index_max = 0; c.step_qdb = 1; c.step_measured = false; - c.offset_min_qdb = - -static_cast(rtl8733b::kSafeTssiTargetQdbm8733b); + c.offset_min_qdb = -128; /* the int8 delta field's negative limit */ c.offset_max_qdb = 127; /* the int8 per-rate delta field's positive limit */ c.rate_diffs = false; c.rate_diffs_hw_table = false; diff --git a/tests/rtl8733b_txpwr_regcheck.sh b/tests/rtl8733b_txpwr_regcheck.sh index 0d75a50..60be641 100755 --- a/tests/rtl8733b_txpwr_regcheck.sh +++ b/tests/rtl8733b_txpwr_regcheck.sh @@ -22,9 +22,10 @@ # actuator, so an on-air null in this geometry is the code and not # the bench. # move -24 qdB shifts every capped rate byte by exactly -24 (0xe8). -# rails -200 clamps to -64 with saturated_low (the 0 dBm absolute target -# floor, NOT the int8 field's -128), +200 to +127 with -# saturated_high (that one IS the field's ceiling). +# rails -200 clamps to -128 and +200 to +127, both with their saturation +# flag - the int8 per-rate delta field at each end. Where the chip +# stops RESPONDING (~-96 qdB down, ~+32 up) is measured and +# documented in docs/rtl8733b.md, not clamped here. # sticky -24 qdB then a full SetMonitorChannel to another 5 GHz group # re-folds the offset against the new channel; a following # FastRetune leaves it in place (the hop rewrites the target table, @@ -80,8 +81,8 @@ echo "== DUT $PID@$VID (RTL8733B) ==" run_txpower "$OUT/base.log" --channel "$CH_A" --offset-start 0 --offset-stop 0 --step-ms 200 caps="$(grep -F '"ev":"txpwr.caps"' "$OUT/base.log" | head -1)" case "$caps" in - *'"supported":1'*'"max":0'*'"step_qdb":1'*'"min_qdb":-64'*'"max_qdb":127'*) - pass "caps: dBm model, delta-field range [-64, +127]" ;; + *'"supported":1'*'"max":0'*'"step_qdb":1'*'"min_qdb":-128'*'"max_qdb":127'*) + pass "caps: dBm model, delta-field range [-128, +127]" ;; "") fail "caps: no txpwr.caps event (bring-up failed? see $OUT/base.log)" ;; *) fail "caps: unexpected $caps" ;; esac @@ -128,8 +129,8 @@ fi run_txpower "$OUT/rails.log" --channel "$CH_A" --offset-start -200 --offset-stop 200 --step-qdb 400 --step-ms 200 lo_applied="$(offset_field "$OUT/rails.log" 1 applied)"; lo_sat="$(state_field "$OUT/rails.log" 2 satlo)" hi_applied="$(offset_field "$OUT/rails.log" 2 applied)"; hi_sat="$(state_field "$OUT/rails.log" 3 sathi)" -if [ "$lo_applied" = "-64" ] && [ "$lo_sat" = "1" ] && [ "$hi_applied" = "127" ] && [ "$hi_sat" = "1" ]; then - pass "rails: -200 -> -64 satlo=1, +200 -> +127 sathi=1" +if [ "$lo_applied" = "-128" ] && [ "$lo_sat" = "1" ] && [ "$hi_applied" = "127" ] && [ "$hi_sat" = "1" ]; then + pass "rails: -200 -> -128 satlo=1, +200 -> +127 sathi=1" else fail "rails: low(applied=$lo_applied satlo=$lo_sat) high(applied=$hi_applied sathi=$hi_sat)" fi diff --git a/tests/rtl8733b_txpwr_selftest.cpp b/tests/rtl8733b_txpwr_selftest.cpp index e7486c4..a9f7490 100644 --- a/tests/rtl8733b_txpwr_selftest.cpp +++ b/tests/rtl8733b_txpwr_selftest.cpp @@ -69,16 +69,36 @@ int main() { down24 && (*down24)[7] == -28 && ((*down24)[0] - (*down24)[7]) == ((*base)[0] - (*base)[7])); - /* 4. Rails. -64 qdB puts the anchor rates at a 0 qdBm target; the rate - * already 4 qdB colder runs out first and says so. */ + /* 4. The floor is the int8 delta field, not the 0 dBm target. -64 qdB lands + * the anchor rates on a 0 dBm target and rails NOTHING: the ladder keeps + * its calibrated spread straight through it, because the loop was measured + * to keep reducing power well past that point. */ rtl8733b::TssiOffsetSat8733b sat_clean; const auto floor_ok = rtl8733b::Phy8733b::tssi_rate_offsets(t, 0, 0, kCeiling, -64, &sat_clean); - expect("-64 qdB reaches the 0 qdBm target without railing the ladder", - floor_ok && (*floor_ok)[0] == -64 && !sat_clean.high); - expect("the colder rate rails first and is reported", - floor_ok && (*floor_ok)[7] == -64 && sat_clean.low); + expect("-64 qdB is not a rail — the ladder passes through the 0 dBm target", + floor_ok && (*floor_ok)[0] == -64 && (*floor_ok)[7] == -68 && + !sat_clean.low && !sat_clean.high); + + /* Well below it the spread is still intact — this is a shift, not a squeeze + * against a floor. */ + const auto deep = + rtl8733b::Phy8733b::tssi_rate_offsets(t, 0, 0, kCeiling, -100); + expect("the shape survives below the 0 dBm target", + deep && (*deep)[0] == -100 && (*deep)[7] == -104 && + ((*deep)[0] - (*deep)[7]) == ((*base)[0] - (*base)[7])); + + /* The real rail is the field: at -128 the hot rate sits exactly on it and the + * colder one cannot follow, which is what saturated_low is for. */ + rtl8733b::TssiOffsetSat8733b sat_rail; + const auto railed = + rtl8733b::Phy8733b::tssi_rate_offsets(t, 0, 0, kCeiling, -128, + &sat_rail); + expect("-128 qdB sits on the int8 field floor", + railed && (*railed)[0] == -128); + expect("the colder rate rails there and is reported", + railed && (*railed)[7] == -128 && sat_rail.low && !sat_rail.high); rtl8733b::TssiOffsetSat8733b sat_zero; const auto no_shift = rtl8733b::Phy8733b::tssi_rate_offsets( From 2956621354614feeecb448a3ed48ed344ed1308a Mon Sep 17 00:00:00 2001 From: snokvist Date: Sun, 16 Aug 2026 08:32:45 +0200 Subject: [PATCH 7/8] rtl8733b: fix two stale one-sided range claims (Qodo review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo's only finding on #399, and it was right. The member comment on _tx_offset_qdb still said "qdB, <= 0" and root CLAUDE.md still described a one-sided [-64, 0] range — both written when the range genuinely was one-sided, and neither updated when the positive half landed, let alone when the floor moved to the field limit. A contract that contradicts the code is how the next person clamps wrongly. Both now state [-128, +127] with the measured knees (~-96 qdB down, ~+32 up) noted as documented rather than enforced. Swept the rest of the tree for the same claim; nothing else carried it. ctest 54/54. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 8 +++++--- src/rtl8733b/Rtl8733bDevice.h | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5c060fd..8b2d7e8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -346,9 +346,11 @@ shape-preserving), `SetTxPowerIndexOverride` (flat absolute), `SetTxPowerRateDiffs` (replace the calibrated per-rate shape). The RTL8733B ports **only the first**, and on a different mechanism: its closed-loop TSSI target table, not a TXAGC index, so -its caps report the dBm model (`index_max = 0`) and a one-sided -`[-64, 0] qdB` range below the safe 16 dBm target — the knob can only back off -from the level that backend characterised. The flat index is refused there +its caps report the dBm model (`index_max = 0`) over the int8 delta field's +`[-128, +127] qdB`, centred on a safe 16 dBm first-light target. Neither end is +re-clamped at something softer; where the chip stops responding — about +-96 qdB down, about +32 up, where the PA compresses and only EVM shows it — is +measured and documented rather than enforced. The flat index is refused there because it was measured unable to carry HT at all. The contract — how they compose, the MCS7-anchor semantics, family step sizes, the write-only-family `hw_readback=false` diff --git a/src/rtl8733b/Rtl8733bDevice.h b/src/rtl8733b/Rtl8733bDevice.h index 11b423f..072a2f6 100644 --- a/src/rtl8733b/Rtl8733bDevice.h +++ b/src/rtl8733b/Rtl8733bDevice.h @@ -91,9 +91,11 @@ class Rtl8733bDevice : public IRtlDevice { bool _phy_ready = false; bool _tx_ready = false; bool _tssi_tracking = false; - /* Session TX-power offset (qdB, <= 0) and the rails the last apply hit. - * Sticky by construction: configure_tx_power folds it back in on every - * channel set, and FastRetune passes it to the in-place hop rewrite. */ + /* Session TX-power offset in qdB, over the int8 delta field's full + * [-128, +127] (GetTxPowerCaps argues the range and records where the chip + * stops responding at each end), plus the rails the last apply hit. Sticky + * by construction: configure_tx_power folds it back in on every channel set, + * and FastRetune passes it to the in-place hop rewrite. */ int16_t _tx_offset_qdb = 0; bool _tx_sat_low = false; bool _tx_sat_high = false; From 2b5d3a827f935be3d1e04f20e003f6fea4c6dcb9 Mon Sep 17 00:00:00 2001 From: snokvist Date: Sun, 16 Aug 2026 09:25:02 +0200 Subject: [PATCH 8/8] rtl8733b: fix stale saturation/range docs, guard a vacuous test cell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups from OpenIPC/devourer#399. All three are leftovers from the pre-03daab5 cut, where the floor was -64 qdB (a 0 qdBm target) before the bench sweep showed power keeps falling ~7 dB past it. - TssiOffsetSat8733b documented the inverse of the shipped clamp: it said `low` fires at the 0 qdBm floor "not the int8 field's -128", when the code clamps at `shifted < -64` — which IS the field's -128, a -16 dBm target — and deliberately does not stop at 0 qdBm. This is the struct a closed-loop controller reads to know the knob is out of travel, so it is the worst one to have backwards. - docs/rtl8733b.md still said `[-64, +127]` three paragraphs before the same section derives both ends from the int8 field. GetTxPowerCaps reports -128. - tests/txpwr_offset_onair.sh called 0 qdBm "its floor". The compression measured there is real, but it is not the floor; same stale claim, in a file the review did not cite. Also: the `+16 qdB rails nothing` cell was missing `up16 &&`. Rails publish on success only, so a nullopt would leave sat_up default-false and the cell would pass vacuously. The adjacent cell would still catch it, but the guard is free. And a note at the flat-PG GetTxPowerState site: flat_index >= 0 reads as "a flat override is active" per src/TxPower.h, which no caller can have set here. It is chip truth — the unit really does run a flat index — so the comment records the reading rather than changing the value. ctest 54/54, build clean. Co-Authored-By: Claude Opus 5 (1M context) --- docs/rtl8733b.md | 2 +- src/rtl8733b/Phy8733b.h | 8 +++++--- src/rtl8733b/Rtl8733bDevice.cpp | 8 ++++++++ tests/rtl8733b_txpwr_selftest.cpp | 2 +- tests/txpwr_offset_onair.sh | 6 +++++- 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/docs/rtl8733b.md b/docs/rtl8733b.md index 380d784..044117d 100644 --- a/docs/rtl8733b.md +++ b/docs/rtl8733b.md @@ -148,7 +148,7 @@ shape `fast_retune` uses for its per-channel rewrite (#389); the alternative, a TSSI disable/re-enable pair, costs ~165 ms and buys nothing here. The capability report is the dBm model — `index_max = 0`, one qdB per step — -over `[-64, +127]`. Offset 0 is `kSafeTssiTargetQdbm8733b` (16 dBm), a +over `[-128, +127]`. Offset 0 is `kSafeTssiTargetQdbm8733b` (16 dBm), a first-light clip sitting at or below this part's factory targets (18–20 dBm at 2.4 GHz, 16–19 at 5 GHz). diff --git a/src/rtl8733b/Phy8733b.h b/src/rtl8733b/Phy8733b.h index 0ea1269..cb1e619 100644 --- a/src/rtl8733b/Phy8733b.h +++ b/src/rtl8733b/Phy8733b.h @@ -91,9 +91,11 @@ inline constexpr uint8_t kMaxPgTargetQdbm8733b = 80; /* Which rail the runtime TX-power offset clamped at, if any — the signal a * closed-loop controller uses to know the knob has run out of travel * (IRtlDevice::GetTxPowerState). `low` is set when a rate's shifted target hit - * the 0 qdBm floor (the low bound is that absolute floor, not the int8 field's - * -128); `high` when a rate hit the int8 field's +127 ceiling. Both are - * per-rate facts: a shift can rail one rate while the rest still move, which + * the int8 delta field's -128 floor — a shifted target below -64 qdBm, i.e. + * -16 dBm — and deliberately NOT at the 0 qdBm target, which the loop keeps + * responding past by ~7 dB; `high` when a rate hit the field's +127 ceiling. + * Both rails are that field and nothing softer, and both are per-rate facts: + * a shift can rail one rate while the rest still move, which * is exactly what a shape-preserving offset does at the end of its range. */ struct TssiOffsetSat8733b { bool low = false; diff --git a/src/rtl8733b/Rtl8733bDevice.cpp b/src/rtl8733b/Rtl8733bDevice.cpp index 4d6079b..5b7c0a0 100644 --- a/src/rtl8733b/Rtl8733bDevice.cpp +++ b/src/rtl8733b/Rtl8733bDevice.cpp @@ -859,6 +859,14 @@ devourer::TxPowerState Rtl8733bDevice::GetTxPowerState() { * to both references and zeroes the per-rate diffs, so every * representative rate sits at that index. */ const rtl8733b::TxAgcState8733b agc = _phy.read_txagc_state(); + /* Note the reading this shares with src/TxPower.h's convention: there, + * flat_index >= 0 means "a flat override is active" and clearing it is the + * caller's move. Here nothing can have set one — SetTxPowerIndexOverride + * refuses — and there is nothing to clear. The index is simply what this + * unit runs at, because a no-TSSI-calibration EFUSE leaves bring-up's flat + * index as the level. A consumer reaching for + * SetTxPowerIndexOverride(-1) to "release" it gets a logged refusal, which + * is the honest answer: this unit has no runtime power actuator at all. */ s.flat_index = agc.ofdm_ref_a; s.cck_index = agc.cck_ref_a; s.ofdm_index = agc.ofdm_ref_a; diff --git a/tests/rtl8733b_txpwr_selftest.cpp b/tests/rtl8733b_txpwr_selftest.cpp index a9f7490..046ea45 100644 --- a/tests/rtl8733b_txpwr_selftest.cpp +++ b/tests/rtl8733b_txpwr_selftest.cpp @@ -118,7 +118,7 @@ int main() { up16 && (*up16)[0] == 16 && (*up16)[7] == 12 && ((*up16)[0] - (*up16)[7]) == ((*base)[0] - (*base)[7])); expect("a positive offset within the field rails nothing", - !sat_up.high && !sat_up.low); + up16 && !sat_up.high && !sat_up.low); /* Past the factory target too: the synthetic ladder's rates are calibrated * at 80 qdBm, and +32 commands 96 — uncalibrated by construction, reachable * by design, compliance the caller's (src/TxPower.h). */ diff --git a/tests/txpwr_offset_onair.sh b/tests/txpwr_offset_onair.sh index ba9d47e..da87916 100755 --- a/tests/txpwr_offset_onair.sh +++ b/tests/txpwr_offset_onair.sh @@ -62,7 +62,11 @@ cmake --build "$ROOT/build" -j --target txdemo rxdemo >/dev/null || exit 1 # not an index, which is also why its range is negative. Its transfer is # TSSI-reshaped like the 8822E's, so it takes the same monotone-lever # assertion: measured 0.222/0.231 dB per qdB overall across two passes, but -# only 0.125 in the bottom 12 qdB where the loop nears its 0 qdBm floor. +# only 0.125 in the bottom 12 qdB of this sweep, as the target nears 0 qdBm. +# That compression is not the floor: the API's floor is the int8 field at +# -128, and power keeps falling ~7 dB past 0 qdBm before pinning near +# -96 qdB. This sweep just stops at -64 because that is where the SHIPPED +# monotone assertion holds. # Its cells stop at 0 — the BACKOFF half. The API allows +127 qdB, but that # half is not monotone in received power and cannot be asserted this way: # +32 qdB reads 8.7 dB louder with EVM collapsed from -62 to -18, and +48/+64