Skip to content

Commit 89149c1

Browse files
widgetiiclaude
andauthored
sensor monitor: add IMX291 + advertise the command in --help (#159)
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) <noreply@anthropic.com>
1 parent e1f92fe commit 89149c1

3 files changed

Lines changed: 53 additions & 2 deletions

File tree

docs/sensor-driver-extraction.md

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -689,8 +689,8 @@ hit such a sensor.
689689

690690
## Stage 4 — Live-reading the AE state with `ipctool sensor monitor`
691691

692-
`ipctool sensor` is a built-in subcommand (separate from `trace`) that
693-
reads a fixed list of AE/exposure registers from the running sensor
692+
`ipctool sensor monitor` is a built-in subcommand (separate from `trace`)
693+
that reads a fixed list of AE/exposure registers from the running sensor
694694
over I2C/SPI in a loop, decoded as labelled fields. Same idea as
695695
`_ae_step` but read-side: instead of inferring AE registers from a
696696
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
704704
same hot regs trace_to_driver picked up
705705
```
706706

707+
Currently supported sensors and what they expose:
708+
709+
| Sensor | Registers monitored |
710+
|----------|----------------------------------------------------------------------|
711+
| SC2315E | `EXP`, `AGAIN`, `DGAIN`, `VMAX`, plus tuning regs `R3301/3314/3632/HOLD/R5781/R5785` |
712+
| IMX291 | `HCG_FRSEL` (0x3009), `GAIN` (0x3014), `VMAX` (0x3018), `HMAX` (0x301C), `SHS1` (0x3020), `OPORTSEL` (0x3046) |
713+
| IMX385 | `SHS1`, `GAIN`, `HCG`, `SHS2`, `VMAX`, `RHS1`, `YOUT` |
714+
707715
The reg list per supported sensor lives in `src/snstool.c` (a small
708716
table, ~10 entries). For SC2315E that table mirrors what `_ae_step`
709717
emits — both are the registers the running AE loop writes per frame.
710718

719+
The IMX291 set is geared at *DOL/WDR* debugging in particular — `HCG_FRSEL`
720+
catches the case where AE flips the High Conversion Gain bit and clobbers
721+
FRSEL on a packed-register write, dropping the sensor out of WDR for one
722+
frame. `SHS1` alone is the integration-time control on this part (Sony's
723+
multi-exposure WDR — there's no DOL on IMX291, despite the inherited IMX290
724+
silicon; SHS2 / RHS1 read as 0 and aren't in the table).
725+
726+
Example output on a hi3516cv300 + IMX291 camera in WDR mode, sampled across
727+
a varying-light scene:
728+
729+
```console
730+
$ ipctool sensor monitor
731+
HCG_FRSEL 2 GAIN 0 VMAX 550 HMAX 1130 SHS1 528 OPORTSEL e1
732+
HCG_FRSEL 2 GAIN 0 VMAX 550 HMAX 1130 SHS1 528 OPORTSEL e1
733+
HCG_FRSEL 2 GAIN 1c VMAX 550 HMAX 1130 SHS1 4a3 OPORTSEL e1 # mid-light: SHS1 dropped, gain bumped
734+
HCG_FRSEL 12 GAIN 50 VMAX afd HMAX 1130 SHS1 73 OPORTSEL e1 # low-light: HCG enabled, AE slowed VMAX, gain high
735+
```
736+
737+
`HCG_FRSEL` going from `2` (HCG=0, FRSEL=2) to `12` (HCG=1, FRSEL=2) is
738+
exactly the pattern AE uses to enable high-conversion-gain mode. If you
739+
ever see the low nibble of `HCG_FRSEL` go to `1` instead of `2`, that's
740+
the bug fixed in `widgetii/sony_imx291@b51850c` — DOL FRSEL got clobbered.
741+
711742
### Pairing `sensor monitor` with `_ae_step`
712743

713744
The trace-and-extract workflow gives you the **register set** of the

src/main.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ void print_usage() {
9898
" i2cdetect [-b, --bus] attempt to detect devices on I2C bus\n"
9999
" reginfo [--script] dump current status of pinmux registers\n"
100100
" gpio (scan|mux) GPIO utilities\n"
101+
" sensor monitor poll AE/exposure registers from the\n"
102+
" running sensor every 2s. Supported:\n"
103+
" SC2315E, IMX291, IMX385.\n"
101104
" trace [--skip=usleep] [--output=PATH] <full/path/to/executable> "
102105
"[program arguments]\n"
103106
" dump original firmware calls and data "

src/snstool.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,29 @@ const Reg imx385_regs[] = {
4242
{"YOUT", 0x3357, 2}, {NULL},
4343
};
4444

45+
/* Sony IMX291: 16-bit reg addr, 8-bit data, little-endian wide values.
46+
* 0x3009 packs HCG (bit 4) and FRSEL (bits 3:0) -- watch this register
47+
* to catch AE flipping HCG and clobbering DOL FRSEL on multi-byte writes.
48+
* SHS1 is the only functional integration-time control on IMX291; SHS2
49+
* (0x3024) and RHS1 (0x3030) are inherited from IMX290 silicon but
50+
* non-functional, so they're not in this set. */
51+
const Reg imx291_regs[] = {
52+
{"HCG_FRSEL", 0x3009, 1},
53+
{"GAIN", 0x3014, 1},
54+
{"VMAX", 0x3018, 3},
55+
{"HMAX", 0x301C, 2},
56+
{"SHS1", 0x3020, 3},
57+
{"OPORTSEL", 0x3046, 1},
58+
{NULL},
59+
};
60+
4561
struct {
4662
const char *sns_name;
4763
const Reg *reg;
4864
uint8_t be;
4965
} sns_regs[] = {
5066
{"SC2315E", sc2315e_regs, .be = 1},
67+
{"IMX291", imx291_regs, .be = 0},
5168
{"IMX385", imx385_regs, .be = 0},
5269
};
5370

0 commit comments

Comments
 (0)