From 9350eb1b0a538413388212b66ebcbe9ad86542ad Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Tue, 5 May 2026 16:34:32 +0300 Subject: [PATCH] sensor monitor: add IMX291 + advertise the command in --help The `sensor monitor` subcommand was implemented (#149) for SC2315E and IMX385 but missing from `print_usage`, and the supported-sensors set didn't cover IMX291. Add IMX291 register table tuned for DOL/WDR debugging and document the feature with an example. The IMX291 set focuses on what matters when chasing WDR exposure bugs: HCG_FRSEL 0x3009 (1) HCG bit 4 + FRSEL bits 3:0 packed GAIN 0x3014 (1) analog gain VMAX 0x3018 (3) vertical period HMAX 0x301C (2) horizontal period SHS1 0x3020 (3) integration time (only functional shutter on IMX291) OPORTSEL 0x3046 (1) output mode SHS2 / RHS1 are deliberately omitted -- they read as 0 on IMX291 (present in the silicon address map but non-functional, see Sony IMX291 datasheet vs IMX290 datasheet -- only IMX290/307/327 expose DOL through that triplet). Watching `HCG_FRSEL` is particularly useful: AE writes this register when it wants to flip High Conversion Gain, and a buggy gu8HCGReg in the sensor driver can drop FRSEL bits there, kicking the sensor out of WDR for one frame. The fix in widgetii/sony_imx291@b51850c was diagnosed via this register; the docs include the symptom pattern to look for. `print_usage` now lists `sensor monitor` alongside `trace`, `gpio`, `reginfo` etc. with the supported-sensor list inline. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/sensor-driver-extraction.md | 35 ++++++++++++++++++++++++++++++-- src/main.c | 3 +++ src/snstool.c | 17 ++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/docs/sensor-driver-extraction.md b/docs/sensor-driver-extraction.md index e0a79fc..64f94ec 100644 --- a/docs/sensor-driver-extraction.md +++ b/docs/sensor-driver-extraction.md @@ -689,8 +689,8 @@ hit such a sensor. ## Stage 4 — Live-reading the AE state with `ipctool sensor monitor` -`ipctool sensor` is a built-in subcommand (separate from `trace`) that -reads a fixed list of AE/exposure registers from the running sensor +`ipctool sensor monitor` is a built-in subcommand (separate from `trace`) +that reads a fixed list of AE/exposure registers from the running sensor over I2C/SPI in a loop, decoded as labelled fields. Same idea as `_ae_step` but read-side: instead of inferring AE registers from a captured trace, you can poll the actual sensor while it's running. @@ -704,10 +704,41 @@ EXP ff AGAIN 330 DGAIN 80 VMAX 546 R3301 f R3314 14 R3632 8 HOLD same hot regs trace_to_driver picked up ``` +Currently supported sensors and what they expose: + +| Sensor | Registers monitored | +|----------|----------------------------------------------------------------------| +| SC2315E | `EXP`, `AGAIN`, `DGAIN`, `VMAX`, plus tuning regs `R3301/3314/3632/HOLD/R5781/R5785` | +| IMX291 | `HCG_FRSEL` (0x3009), `GAIN` (0x3014), `VMAX` (0x3018), `HMAX` (0x301C), `SHS1` (0x3020), `OPORTSEL` (0x3046) | +| IMX385 | `SHS1`, `GAIN`, `HCG`, `SHS2`, `VMAX`, `RHS1`, `YOUT` | + The reg list per supported sensor lives in `src/snstool.c` (a small table, ~10 entries). For SC2315E that table mirrors what `_ae_step` emits — both are the registers the running AE loop writes per frame. +The IMX291 set is geared at *DOL/WDR* debugging in particular — `HCG_FRSEL` +catches the case where AE flips the High Conversion Gain bit and clobbers +FRSEL on a packed-register write, dropping the sensor out of WDR for one +frame. `SHS1` alone is the integration-time control on this part (Sony's +multi-exposure WDR — there's no DOL on IMX291, despite the inherited IMX290 +silicon; SHS2 / RHS1 read as 0 and aren't in the table). + +Example output on a hi3516cv300 + IMX291 camera in WDR mode, sampled across +a varying-light scene: + +```console +$ ipctool sensor monitor +HCG_FRSEL 2 GAIN 0 VMAX 550 HMAX 1130 SHS1 528 OPORTSEL e1 +HCG_FRSEL 2 GAIN 0 VMAX 550 HMAX 1130 SHS1 528 OPORTSEL e1 +HCG_FRSEL 2 GAIN 1c VMAX 550 HMAX 1130 SHS1 4a3 OPORTSEL e1 # mid-light: SHS1 dropped, gain bumped +HCG_FRSEL 12 GAIN 50 VMAX afd HMAX 1130 SHS1 73 OPORTSEL e1 # low-light: HCG enabled, AE slowed VMAX, gain high +``` + +`HCG_FRSEL` going from `2` (HCG=0, FRSEL=2) to `12` (HCG=1, FRSEL=2) is +exactly the pattern AE uses to enable high-conversion-gain mode. If you +ever see the low nibble of `HCG_FRSEL` go to `1` instead of `2`, that's +the bug fixed in `widgetii/sony_imx291@b51850c` — DOL FRSEL got clobbered. + ### Pairing `sensor monitor` with `_ae_step` The trace-and-extract workflow gives you the **register set** of the diff --git a/src/main.c b/src/main.c index 2f7eebe..10c9e63 100644 --- a/src/main.c +++ b/src/main.c @@ -98,6 +98,9 @@ void print_usage() { " i2cdetect [-b, --bus] attempt to detect devices on I2C bus\n" " reginfo [--script] dump current status of pinmux registers\n" " gpio (scan|mux) GPIO utilities\n" + " sensor monitor poll AE/exposure registers from the\n" + " running sensor every 2s. Supported:\n" + " SC2315E, IMX291, IMX385.\n" " trace [--skip=usleep] [--output=PATH] " "[program arguments]\n" " dump original firmware calls and data " diff --git a/src/snstool.c b/src/snstool.c index fa5f3a7..75b7993 100644 --- a/src/snstool.c +++ b/src/snstool.c @@ -42,12 +42,29 @@ const Reg imx385_regs[] = { {"YOUT", 0x3357, 2}, {NULL}, }; +/* Sony IMX291: 16-bit reg addr, 8-bit data, little-endian wide values. + * 0x3009 packs HCG (bit 4) and FRSEL (bits 3:0) -- watch this register + * to catch AE flipping HCG and clobbering DOL FRSEL on multi-byte writes. + * SHS1 is the only functional integration-time control on IMX291; SHS2 + * (0x3024) and RHS1 (0x3030) are inherited from IMX290 silicon but + * non-functional, so they're not in this set. */ +const Reg imx291_regs[] = { + {"HCG_FRSEL", 0x3009, 1}, + {"GAIN", 0x3014, 1}, + {"VMAX", 0x3018, 3}, + {"HMAX", 0x301C, 2}, + {"SHS1", 0x3020, 3}, + {"OPORTSEL", 0x3046, 1}, + {NULL}, +}; + struct { const char *sns_name; const Reg *reg; uint8_t be; } sns_regs[] = { {"SC2315E", sc2315e_regs, .be = 1}, + {"IMX291", imx291_regs, .be = 0}, {"IMX385", imx385_regs, .be = 0}, };