Enhance HW reset recovery logic and streaming state management

.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
 

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.

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 <guennadi.liakhovetski@intel.com>,\n\
 				Shikun Ding <shikun.ding@intel.com>,\n\
 				Dmitry Perchanov <dmitry.perchanov@intel.com>");
 MODULE_LICENSE("GPL v2");
-MODULE_VERSION("1.0.2.22");
+MODULE_VERSION("1.0.2.23");