Closed
Fix serdes_inited[] peer slot search to scan entire array#385
Conversation
Fix two HW reset issues: 1. [D457 GMSL] No device found after HW Reset - caused by chip-wide deserializer reset_oneshot disrupting sibling camera links 2. [D401 GMSL] Fail to stream after HW reset during active streaming - caused by stale sensor->streaming flags blocking subsequent stream starts Changes: - Stop active streams before sending HW reset command to ensure clean state - Clear sensor->streaming flags and release SERDES pipes eagerly during reset (uncomment and enhance the previously disabled invalidation block) - Replace chip-wide reset_oneshot with tiered SERDES recovery: Phase 1: serializer-only re-init (per-camera, non-disruptive) Phase 2: full deserializer reset only when all siblings are also dead, with I2C health check gate to protect actively streaming cameras - Replace global ds5_reset_gen counter with per-deserializer counters to prevent cross-camera state invalidation across independent GMSL links - Relocate serdes_inited[] and MAX_DEV_NUM earlier for visibility Fixes: RSDSO-21151, RSDSO-21257, RSDSO-21254
Each physical D4XX camera has 4 driver instances (Depth, RGB, IR, IMU) sharing the same MAX9295 serializer (ser_dev). The initial HW reset recovery implementation treated all instances on the same deserializer as siblings, which caused incorrect behavior: 1. Same-camera instances (same ser_dev) were treated as 'siblings with active streams' in Phase 2, blocking deserializer reset even when no true sibling camera was streaming. 2. The streaming flag check examined all 4 sensor types per instance, but each instance only manages ONE sensor type (via is_depth/is_rgb/ is_y8/is_imu flags). 3. Steps 1 and 2 (stop streams, invalidate state) only operated on the current instance, leaving peer instances of the same physical camera with stale streaming flags and SERDES pipe state after HW reset. Fix by: - Distinguishing same-camera peers (same ser_dev) from true siblings (different ser_dev, same dser_dev) in SERDES recovery Phase 2 - Using ds5_get_active_sensor() to check only the managed sensor type per instance instead of all 4 sensor structs - Iterating serdes_inited[] to stop streams and invalidate state on all peer instances of the same physical camera in Steps 1 and 2 - Using __maybe_unused for loop variable only needed in SERDES path JIRA: RSDSO-21151, RSDSO-21257, RSDSO-21254
…loop Two bugs found via post-HW-reset log analysis: Bug A: Only the first stream instance (Depth) per physical camera was registered in serdes_inited[]. RGB, Y8, and IMU instances returned -ENOTSUPP from ds5_board_setup() and were never registered. This made them invisible to peer iteration in ds5_hw_reset_with_recovery() steps 1 and 2, so their streaming flags and SERDES pipe state were never cleared after HW reset. Fix: Register ALL instances in serdes_inited[], even when they share the same serializer. The -ENOTSUPP still prevents duplicate SERDES setup but the instance is now registered before returning. Add a serdes_primary flag to struct ds5 so that only the primary instance tears down the serializer/deserializer on ds5_remove(). Bug B: After HW reset, all FW streams return to idle. When the VI capture engine detects frame loss and initiates error recovery, it calls ds5_mux_s_stream(off) to stop the stream. The pre-condition check at the top of ds5_mux_s_stream() sees the stream is already stopped (status 0x0000) and returns -EBUSY, which causes VI to fail the restart and enter an infinite retry loop. Fix: Treat stream-already-in-target-state as a no-op (return 0) instead of -EBUSY, allowing VI error recovery to proceed. Observed in log: d4xx 9-001b (RGB) stuck in endless 'stream 1 in 0 state already, return busy' loop after d4xx 9-001a (Depth) triggered HW reset. JIRA: RSDSO-21151, RSDSO-21257, RSDSO-21254
The 4th probe instance (IMU at 9-001d) fails to communicate with the D457 camera at the second I2C check in ds5_probe(), even though the first 3 instances succeed. Root cause analysis of the dmesg logs shows the first I2C check (with retries) passes, but the second check (single ds5_read, no outer retry) fails with -121 (EREMOTEIO / NAK). The D457 firmware has a brief I2C-unresponsive window during post-HW- reset initialization that the D401 does not exhibit. By the time the 4th instance reaches the second I2C check (~50ms after HW reset completion), the camera is momentarily unreachable. Three bugs fixed: 1. Second I2C check had no retry loop: replaced single ds5_read() with a retry loop (10 iterations, 50ms delay between retries) matching the pattern used by the first communication check. 2. Spurious pipe 0 release at probe: sensor pipe_ids default to 0 (kzalloc) instead of PIPE_NOT_CONFIGURED (-1). When hw_reset_with_recovery runs during probe (before ds5_sensor_init), step 2 sees pipe_id=0 >= 0 and releases pipe 0 from the MAX9296. Fix: initialize all four sensor pipe_ids to PIPE_NOT_CONFIGURED before calling hw_reset_with_recovery. 3. No stabilization delay after probe HW reset: added msleep(100) after hw_reset_with_recovery returns, giving the D457 firmware time to complete post-reset initialization before subsequent instances attempt I2C communication. Tested against logs: v1.0.2.16 (0/4 instances, old full SERDES re-init killed GMSL link), v1.0.2.18 pre-fix (3/4 instances, IMU fails at 2nd I2C check).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Nikolai-L <36960789+Nikolai-L@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] WIP Address feedback on HW reset GMSL recovery implementation
Fix serdes_inited[] peer slot search to scan entire array
Mar 2, 2026
Nikolai-L
force-pushed
the
fix/hw-reset-gmsl-recovery
branch
from
March 2, 2026 15:34
fd8f06c to
4254890
Compare
Contributor
|
@copilot rebase the fix |
Co-authored-by: Nikolai-L <36960789+Nikolai-L@users.noreply.github.com>
Author
Nikolai-L
force-pushed
the
fix/hw-reset-gmsl-recovery
branch
3 times, most recently
from
March 15, 2026 15:26
32795fe to
44dd50d
Compare
Contributor
|
Outdated |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix/hw-reset-gmsl-recovery(b2ca414)ds5_board_setupvariants: inner loop changed fromj = i+1toj = 0withj != iguard, so free slots are found even in fragmented arrays💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.