Skip to content

Commit 4292e02

Browse files
author
Nikolai-L
committed
Enhance HW reset recovery logic and streaming state management
- Implement a lightweight stability probe during HW reset recovery to ensure natural recovery before full verification. - Modify `ds5_mux_s_stream()` to enforce a stop on streaming if reset-generation invalidation is detected, ensuring proper reconfiguration.
1 parent 8cddf33 commit 4292e02

3 files changed

Lines changed: 83 additions & 36 deletions

File tree

.github/copilot-instructions.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ Linux kernel driver and userspace utilities for Intel RealSense D4XX series 3D d
3030
- `CONFIG_TEGRA_CAMERA_PLATFORM` — Tegra-specific camera platform integration.
3131
- `LINUX_VERSION_CODE` checks for API differences between kernel versions (4.9, 5.10, 5.15+).
3232
- **Prefer lazy invalidation over explicit loops**: when state must be invalidated across multiple instances (e.g. after a deserializer reset), increment an atomic generation counter (`atomic_inc()`) and let each instance detect the bump lazily (e.g. in `ds5_configure()`). Avoid O(N) loops that iterate `ds5_inited[]` to poke siblings. Combine lazy checks when possible — if an existing function already detects a generation mismatch, add new invalidation logic there rather than adding a separate check elsewhere.
33+
- **HW reset natural recovery guard**: do not bypass Step 10 entirely when Step 6 is skipped. Keep a lightweight natural-recovery stability probe (2 reads spaced 100ms), and if it fails, run Phase 1 SERDES recovery before entering the full Step 10 stability verification path.
34+
- **`ds5_mux_s_stream()` pre-toggle no-op rule**: for start (`on=1`), if reset-generation invalidation was detected and FW still reports streaming, do not return no-op; force a stop, clear cached stream state, and continue through normal start/configure flow.
3335

3436
### V4L2 Subdev Architecture
3537

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,3 +127,5 @@ The build system cross-compiles for ARM64. Toolchains vary by JetPack:
127127
- Protect per-camera mutable slot state (`ds5_primary`, `depth/rgb/ir/imu_streaming`) with `struct ds5_dev::lock`.
128128
- Protect per-deserializer slot assignment (`dser_dev`) with `struct dser_control::lock`.
129129
- For sibling-health checks, snapshot pointers/flags under lock and perform I2C probing after unlocking.
130+
- In HW reset Step 10, natural-recovery path must still run a lightweight stability gate (2 reads x 100ms); on failure, trigger Phase 1 SERDES recovery and then continue full stability verification.
131+
- In `ds5_mux_s_stream()`, treat pre-toggle "already streaming" as no-op only when state is coherent; after reset-generation invalidation on start path, force stop + state clear and proceed with normal reconfiguration flow.

kernel/realsense/d4xx.c

Lines changed: 79 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2337,6 +2337,8 @@ static int ds5_set_calibration_data(struct ds5 *state,
23372337
#define DS5_HW_RESET_STABILITY_READS 3 /* consecutive successful reads */
23382338
#define DS5_HW_RESET_STABILITY_INTERVAL_MS 200 /* between each check */
23392339
#define DS5_HW_RESET_STABILITY_TIMEOUT_MS 3000 /* max wait for stable link */
2340+
#define DS5_HW_RESET_NATURAL_STABILITY_READS 2
2341+
#define DS5_HW_RESET_NATURAL_STABILITY_INTERVAL_MS 100
23402342

23412343
/* Minimum interval between consecutive HW resets (ms).
23422344
* Rapid back-to-back resets degrade the GMSL link because each
@@ -2953,33 +2955,50 @@ static int ds5_hw_reset_with_recovery(struct ds5 *state)
29532955
}
29542956

29552957
#ifdef CONFIG_VIDEO_D4XX_SERDES
2956-
/* 10. Post-reset I2C stability verification (conditional).
2957-
* Only needed when SERDES recovery was actively performed in Step 6,
2958-
* because the serializer/deserializer re-init can cause transient
2959-
* I2C instability. When the GMSL link recovered naturally (Step 6
2960-
* was skipped), Steps 7-9 already verified the link via multiple
2961-
* I2C reads — an additional 600ms+ stability loop is unnecessary
2962-
* and causes timing-sensitive CI test failures.
2958+
/* 10. Post-reset I2C stability verification.
29632959
*
2964-
* Some camera SKUs have a secondary FW initialization phase that
2965-
* briefly drops the I2C bus ~65-110ms after the initial readiness
2966-
* checks (Steps 7-9) pass. This has been observed on both D401
2967-
* (FW 5.17.x) and D457 (FW 5.17.2.7). If we return now,
2968-
* userspace HWMC queries NAK, the viewer triggers another reset,
2969-
* and repeated rapid resets degrade the GMSL link until SERDES pipe
2970-
* setup fails and the device is unrecoverable without a host reboot.
2960+
* Natural-recovery path: perform a lightweight stability gate
2961+
* (2 reads, 100ms apart). This catches the FW secondary init
2962+
* window that can occur in parallel with Step 1/early ready checks.
2963+
* If any probe fails, run Phase 1 SERDES recovery and continue with
2964+
* the full Step 10 verification loop.
29712965
*
2972-
* Verify I2C stability by performing DS5_HW_RESET_STABILITY_READS
2973-
* consecutive successful reads spaced DS5_HW_RESET_STABILITY_INTERVAL_MS
2974-
* apart. If the link drops mid-verification, reset the counter and
2975-
* keep waiting (up to DS5_HW_RESET_STABILITY_TIMEOUT_MS). If it
2976-
* remains unstable, escalate to Phase 2 (full deserializer reset).
2966+
* Full verification loop: when SERDES recovery ran, verify I2C
2967+
* stability with DS5_HW_RESET_STABILITY_READS consecutive reads
2968+
* spaced DS5_HW_RESET_STABILITY_INTERVAL_MS apart. If unstable,
2969+
* escalate to Phase 2 (full deserializer reset).
29772970
*/
29782971
if (!serdes_recovery_ran) {
2979-
dev_info(&state->client->dev,
2980-
"%s(): SERDES recovery was skipped (natural link recovery), "
2981-
"bypassing Step 10 stability verification\n", __func__);
2982-
} else {
2972+
int natural_ok = 0;
2973+
u16 natural_val = 0;
2974+
2975+
for (; natural_ok < DS5_HW_RESET_NATURAL_STABILITY_READS;
2976+
natural_ok++) {
2977+
msleep(DS5_HW_RESET_NATURAL_STABILITY_INTERVAL_MS);
2978+
ret = ds5_read(state, DS5_FW_VERSION, &natural_val);
2979+
if (ret < 0 || natural_val == 0)
2980+
break;
2981+
}
2982+
2983+
if (natural_ok < DS5_HW_RESET_NATURAL_STABILITY_READS) {
2984+
dev_warn(&state->client->dev,
2985+
"%s(): natural recovery stability probe failed (ret=%d, val=0x%x), running Phase 1 SERDES recovery\n",
2986+
__func__, ret, natural_val);
2987+
ret = ds5_hw_reset_serdes_recovery(state, false);
2988+
if (ret < 0)
2989+
dev_err(&state->client->dev,
2990+
"%s(): Phase 1 recovery from natural path failed: %d\n",
2991+
__func__, ret);
2992+
serdes_recovery_ran = true;
2993+
} else {
2994+
dev_info(&state->client->dev,
2995+
"%s(): natural recovery stable (%d reads x %d ms), skipping full Step 10 loop\n",
2996+
__func__, DS5_HW_RESET_NATURAL_STABILITY_READS,
2997+
DS5_HW_RESET_NATURAL_STABILITY_INTERVAL_MS);
2998+
}
2999+
}
3000+
3001+
if (serdes_recovery_ran) {
29833002
int stable_count = 0;
29843003
unsigned long stab_ts = jiffies;
29853004
unsigned long stab_timeout =
@@ -5233,6 +5252,7 @@ static int ds5_mux_s_stream(struct v4l2_subdev *sd, int on)
52335252
struct ds5_sensor *sensor = state->mux.last_set;
52345253
u16 expected_streaming_state;
52355254
bool ds5_config_done = !on; /* for stop, skip config */
5255+
bool reset_invalidated = false;
52365256
bool *streaming_flag = NULL;
52375257
#ifdef CONFIG_VIDEO_D4XX_SERDES
52385258
int serdes_recovery_attempts = 0;
@@ -5251,6 +5271,7 @@ static int ds5_mux_s_stream(struct v4l2_subdev *sd, int on)
52515271
|| state->reset_ref_dser != cur_dser) {
52525272
ds5_invalidate_sensor(state, sensor);
52535273
sensor->streaming = false;
5274+
reset_invalidated = true;
52545275
state->reset_ref_ds5 = cur_ds5;
52555276
state->reset_ref_dser = cur_dser;
52565277
}
@@ -5322,20 +5343,42 @@ static int ds5_mux_s_stream(struct v4l2_subdev *sd, int on)
53225343
"stream %d in expected state, toggling to %d (status: 0x%04x) %dms\n",
53235344
stream_id, on, status, jiffies_to_msecs(jiffies - ts));
53245345
} else {
5325-
/* After HW reset the FW reboots and all streams return to
5326-
* idle. If VI error recovery tries to stop a stream that
5327-
* is already stopped (or start one already started), treat
5328-
* it as a no-op so the upper layer can proceed with
5329-
* restart instead of getting stuck in an EBUSY loop.
5346+
/* If state was invalidated by reset-generation bump and FW still
5347+
* reports this stream as active, force a stop to guarantee next
5348+
* start goes through full reconfiguration.
53305349
*/
5331-
dev_warn(&state->client->dev,
5332-
"stream %d in %d state already (status: 0x%04x) %dms, treating as no-op\n",
5333-
stream_id, on, status, jiffies_to_msecs(jiffies - ts));
5334-
mutex_lock(&state->ds5_dev->lock);
5335-
*streaming_flag = on;
5336-
mutex_unlock(&state->ds5_dev->lock);
5337-
sensor->streaming = on;
5338-
return 0;
5350+
if (on && reset_invalidated && (status & DS5_STATUS_STREAMING)) {
5351+
dev_warn(&state->client->dev,
5352+
"stream %d reports streaming after reset invalidation (status: 0x%04x), forcing stop and reconfigure\n",
5353+
stream_id, status);
5354+
5355+
ret = ds5_write(state, DS5_START_STOP_STREAM,
5356+
DS5_STREAM_STOP | stream_id);
5357+
if (ret < 0)
5358+
dev_warn(&state->client->dev,
5359+
"stream %d forced stop write failed (%d), continuing with reconfigure\n",
5360+
stream_id, ret);
5361+
5362+
mutex_lock(&state->ds5_dev->lock);
5363+
*streaming_flag = false;
5364+
mutex_unlock(&state->ds5_dev->lock);
5365+
sensor->streaming = false;
5366+
} else {
5367+
/* After HW reset the FW reboots and all streams return to
5368+
* idle. If VI error recovery tries to stop a stream that
5369+
* is already stopped (or start one already started), treat
5370+
* it as a no-op so the upper layer can proceed with
5371+
* restart instead of getting stuck in an EBUSY loop.
5372+
*/
5373+
dev_warn(&state->client->dev,
5374+
"stream %d in %d state already (status: 0x%04x) %dms, treating as no-op\n",
5375+
stream_id, on, status, jiffies_to_msecs(jiffies - ts));
5376+
mutex_lock(&state->ds5_dev->lock);
5377+
*streaming_flag = on;
5378+
mutex_unlock(&state->ds5_dev->lock);
5379+
sensor->streaming = on;
5380+
return 0;
5381+
}
53395382
}
53405383

53415384
restore_val = sensor->streaming;

0 commit comments

Comments
 (0)