diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a28e751a..7ec83dcc 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -30,6 +30,8 @@ Linux kernel driver and userspace utilities for Intel RealSense D4XX series 3D d - `CONFIG_TEGRA_CAMERA_PLATFORM` — Tegra-specific camera platform integration. - `LINUX_VERSION_CODE` checks for API differences between kernel versions (4.9, 5.10, 5.15+). - **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. +- **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. +- **`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. ### V4L2 Subdev Architecture diff --git a/CLAUDE.md b/CLAUDE.md index 20cf6d7d..f1e70200 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,3 +127,5 @@ The build system cross-compiles for ARM64. Toolchains vary by JetPack: - Protect per-camera mutable slot state (`ds5_primary`, `depth/rgb/ir/imu_streaming`) with `struct ds5_dev::lock`. - Protect per-deserializer slot assignment (`dser_dev`) with `struct dser_control::lock`. - For sibling-health checks, snapshot pointers/flags under lock and perform I2C probing after unlocking. +- 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. +- 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. diff --git a/kernel/realsense/d4xx.c b/kernel/realsense/d4xx.c index 64e64e0a..3e662d0d 100644 --- a/kernel/realsense/d4xx.c +++ b/kernel/realsense/d4xx.c @@ -2337,6 +2337,8 @@ static int ds5_set_calibration_data(struct ds5 *state, #define DS5_HW_RESET_STABILITY_READS 3 /* consecutive successful reads */ #define DS5_HW_RESET_STABILITY_INTERVAL_MS 200 /* between each check */ #define DS5_HW_RESET_STABILITY_TIMEOUT_MS 3000 /* max wait for stable link */ +#define DS5_HW_RESET_NATURAL_STABILITY_READS 2 +#define DS5_HW_RESET_NATURAL_STABILITY_INTERVAL_MS 100 /* Minimum interval between consecutive HW resets (ms). * Rapid back-to-back resets degrade the GMSL link because each @@ -2953,33 +2955,53 @@ static int ds5_hw_reset_with_recovery(struct ds5 *state) } #ifdef CONFIG_VIDEO_D4XX_SERDES - /* 10. Post-reset I2C stability verification (conditional). - * Only needed when SERDES recovery was actively performed in Step 6, - * because the serializer/deserializer re-init can cause transient - * I2C instability. When the GMSL link recovered naturally (Step 6 - * was skipped), Steps 7-9 already verified the link via multiple - * I2C reads — an additional 600ms+ stability loop is unnecessary - * and causes timing-sensitive CI test failures. + /* 10. Post-reset I2C stability verification. * - * Some camera SKUs have a secondary FW initialization phase that - * briefly drops the I2C bus ~65-110ms after the initial readiness - * checks (Steps 7-9) pass. This has been observed on both D401 - * (FW 5.17.x) and D457 (FW 5.17.2.7). If we return now, - * userspace HWMC queries NAK, the viewer triggers another reset, - * and repeated rapid resets degrade the GMSL link until SERDES pipe - * setup fails and the device is unrecoverable without a host reboot. + * Natural-recovery path: perform a lightweight stability gate + * (2 reads, 100ms apart). This catches the FW secondary init + * window that can occur in parallel with the earlier readiness + * checks (Steps 5 and 7–9). + * If any probe fails, run Phase 1 SERDES recovery and continue with + * the full Step 10 verification loop. * - * Verify I2C stability by performing DS5_HW_RESET_STABILITY_READS - * consecutive successful reads spaced DS5_HW_RESET_STABILITY_INTERVAL_MS - * apart. If the link drops mid-verification, reset the counter and - * keep waiting (up to DS5_HW_RESET_STABILITY_TIMEOUT_MS). If it - * remains unstable, escalate to Phase 2 (full deserializer reset). + * Full verification loop: when SERDES recovery ran, verify I2C + * stability with DS5_HW_RESET_STABILITY_READS consecutive reads + * spaced DS5_HW_RESET_STABILITY_INTERVAL_MS apart. If unstable, + * escalate to Phase 2 (full deserializer reset). */ if (!serdes_recovery_ran) { - dev_info(&state->client->dev, - "%s(): SERDES recovery was skipped (natural link recovery), " - "bypassing Step 10 stability verification\n", __func__); - } else { + int natural_ok = 0; + u16 natural_val = 0; + + for (; natural_ok < DS5_HW_RESET_NATURAL_STABILITY_READS; + natural_ok++) { + msleep(DS5_HW_RESET_NATURAL_STABILITY_INTERVAL_MS); + ret = ds5_read(state, DS5_FW_VERSION, &natural_val); + if (ret < 0 || natural_val == 0) + break; + } + + if (natural_ok < DS5_HW_RESET_NATURAL_STABILITY_READS) { + dev_warn(&state->client->dev, + "%s(): natural recovery stability probe failed (ret=%d, val=0x%x), running Phase 1 SERDES recovery\n", + __func__, ret, natural_val); + ret = ds5_hw_reset_serdes_recovery(state, false); + if (ret < 0) { + dev_err(&state->client->dev, + "%s(): Phase 1 recovery from natural path failed: %d\n", + __func__, ret); + return ret; + } + serdes_recovery_ran = true; + } else { + dev_info(&state->client->dev, + "%s(): natural recovery stable (%d reads x %d ms), skipping full Step 10 loop\n", + __func__, DS5_HW_RESET_NATURAL_STABILITY_READS, + DS5_HW_RESET_NATURAL_STABILITY_INTERVAL_MS); + } + } + + if (serdes_recovery_ran) { int stable_count = 0; unsigned long stab_ts = jiffies; unsigned long stab_timeout = @@ -5233,6 +5255,7 @@ static int ds5_mux_s_stream(struct v4l2_subdev *sd, int on) struct ds5_sensor *sensor = state->mux.last_set; u16 expected_streaming_state; bool ds5_config_done = !on; /* for stop, skip config */ + bool reset_invalidated = false; bool *streaming_flag = NULL; #ifdef CONFIG_VIDEO_D4XX_SERDES int serdes_recovery_attempts = 0; @@ -5251,6 +5274,7 @@ static int ds5_mux_s_stream(struct v4l2_subdev *sd, int on) || state->reset_ref_dser != cur_dser) { ds5_invalidate_sensor(state, sensor); sensor->streaming = false; + reset_invalidated = true; state->reset_ref_ds5 = cur_ds5; state->reset_ref_dser = cur_dser; } @@ -5322,20 +5346,42 @@ static int ds5_mux_s_stream(struct v4l2_subdev *sd, int on) "stream %d in expected state, toggling to %d (status: 0x%04x) %dms\n", stream_id, on, status, jiffies_to_msecs(jiffies - ts)); } else { - /* After HW reset the FW reboots and all streams return to - * idle. If VI error recovery tries to stop a stream that - * is already stopped (or start one already started), treat - * it as a no-op so the upper layer can proceed with - * restart instead of getting stuck in an EBUSY loop. + /* If state was invalidated by reset-generation bump and FW still + * reports this stream as active, force a stop to guarantee next + * start goes through full reconfiguration. */ - dev_warn(&state->client->dev, - "stream %d in %d state already (status: 0x%04x) %dms, treating as no-op\n", - stream_id, on, status, jiffies_to_msecs(jiffies - ts)); - mutex_lock(&state->ds5_dev->lock); - *streaming_flag = on; - mutex_unlock(&state->ds5_dev->lock); - sensor->streaming = on; - return 0; + if (on && reset_invalidated && (status & DS5_STATUS_STREAMING)) { + dev_warn(&state->client->dev, + "stream %d reports streaming after reset invalidation (status: 0x%04x), forcing stop and reconfigure\n", + stream_id, status); + + ret = ds5_write(state, DS5_START_STOP_STREAM, + DS5_STREAM_STOP | stream_id); + if (ret < 0) + dev_warn(&state->client->dev, + "stream %d forced stop write failed (%d), continuing with reconfigure\n", + stream_id, ret); + + mutex_lock(&state->ds5_dev->lock); + *streaming_flag = false; + mutex_unlock(&state->ds5_dev->lock); + sensor->streaming = false; + } else { + /* After HW reset the FW reboots and all streams return to + * idle. If VI error recovery tries to stop a stream that + * is already stopped (or start one already started), treat + * it as a no-op so the upper layer can proceed with + * restart instead of getting stuck in an EBUSY loop. + */ + dev_warn(&state->client->dev, + "stream %d in %d state already (status: 0x%04x) %dms, treating as no-op\n", + stream_id, on, status, jiffies_to_msecs(jiffies - ts)); + mutex_lock(&state->ds5_dev->lock); + *streaming_flag = on; + mutex_unlock(&state->ds5_dev->lock); + sensor->streaming = on; + return 0; + } } restore_val = sensor->streaming; @@ -6972,4 +7018,4 @@ MODULE_AUTHOR("Guennadi Liakhovetski ,\n\ Shikun Ding ,\n\ Dmitry Perchanov "); MODULE_LICENSE("GPL v2"); -MODULE_VERSION("1.0.2.22"); +MODULE_VERSION("1.0.2.23");