|
| 1 | +# Copilot Instructions |
| 2 | + |
| 3 | +## Project Overview |
| 4 | + |
| 5 | +Linux kernel driver and userspace utilities for Intel RealSense D4XX series 3D depth cameras operating over GMSL (Gigabit Multimedia Serial Link) MIPI CSI-2 interface on NVIDIA Jetson platforms. Licensed under GPL-2.0. |
| 6 | + |
| 7 | +- **Supported platforms:** Jetson AGX Xavier (JetPack 4.6.1, 5.0.2, 5.1.2) and AGX Orin (JetPack 6.0, 6.1, 6.2, 6.2.1) |
| 8 | +- **Supported cameras:** D457 (primary), D401, D40x, D41x, D43x, D45x, D46x series |
| 9 | + |
| 10 | +## Coding Conventions |
| 11 | + |
| 12 | +### Kernel Driver (C — `kernel/realsense/d4xx.c`) |
| 13 | + |
| 14 | +- **Follow Linux kernel coding style**: tabs for indentation (8-space width), `/* */` block comments, max ~80–100 char lines. |
| 15 | +- **Keep changes minimal**: do not inflate code. Prefer single-line expressions over multi-line blocks, reuse existing helpers/paths instead of adding new ones, and avoid verbose comments that restate what the code already says. Every added line must earn its place — if a change can be expressed more concisely without losing clarity, use the shorter form. |
| 16 | +- **Before adding new code, check callers and existing paths**: when introducing a check, loop, or helper, first verify whether the same logic already exists elsewhere in the call chain. If a function has only one caller, consider placing the logic in the caller instead of duplicating it. Never add a second copy of logic that can be combined with an existing one — consolidate first, don't layer. |
| 17 | +- **After removing code, clean up stale references**: when deleting a function, code block, or feature, immediately search for defines, variables, struct fields, forward declarations, and comments that were only used by the removed code. Remove all of them in the same patch. Do not leave dead code behind. |
| 18 | +- **Function naming**: prefix all functions with `ds5_`. Mux-related functions use `ds5_mux_`. Examples: `ds5_read()`, `ds5_write()`, `ds5_probe()`, `ds5_mux_s_stream()`. |
| 19 | +- **Struct naming**: prefix with `ds5_`. Examples: `struct ds5`, `struct ds5_sensor`, `struct ds5_ctrls`, `struct ds5_format`. |
| 20 | +- **Macro naming**: prefix with `DS5_`. Register addresses: `DS5_FW_VERSION`, `DS5_START_STOP_STREAM`, `DS5_DEPTH_STREAM_DT`. |
| 21 | +- **Driver name**: `DS5_DRIVER_NAME` = `"d4xx"`, with variants `-awg`, `-asr`, `-class`, `-dfu`. |
| 22 | +- **I2C access**: use `ds5_read()` / `ds5_write()` wrappers around `regmap_raw_read()` / `regmap_raw_write()` with built-in retry logic (`DS5_I2C_RETRY_COUNT=5`, `DS5_I2C_RETRY_DELAY_US=5000`). |
| 23 | +- **Helper macros**: `ds5_read_with_check()`, `ds5_write_with_check()`, `ds5_raw_read_with_check()`, `ds5_raw_write_with_check()` — these return on error. |
| 24 | +- **Logging**: use `dev_err()`, `dev_warn()`, `dev_info()`, `dev_dbg()` with `&state->client->dev` as the device. Always include `__func__` in log messages. |
| 25 | +- **Locking**: `mutex_lock()` / `mutex_unlock()` for state synchronization. |
| 26 | +- **SERDES topology locking**: protect global topology scans/updates of `ds5_inited[]` and `dser_inited[]` with `serdes_lock__`. Use `struct ds5_dev::lock` for per-camera mutable fields (`ds5_primary`, `*_streaming`) and `struct dser_control::lock` for per-deserializer slot fields (`dser_dev`). For sibling checks, snapshot under lock and do I2C probing after unlocking. |
| 27 | +- **Module registration**: `module_i2c_driver()` pattern. |
| 28 | +- **Conditional compilation**: |
| 29 | + - `CONFIG_VIDEO_D4XX_SERDES` — SerDes (GMSL) support vs non-SerDes path. |
| 30 | + - `CONFIG_TEGRA_CAMERA_PLATFORM` — Tegra-specific camera platform integration. |
| 31 | + - `LINUX_VERSION_CODE` checks for API differences between kernel versions (4.9, 5.10, 5.15+). |
| 32 | +- **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 | + |
| 34 | +### V4L2 Subdev Architecture |
| 35 | + |
| 36 | +Each camera registers four sensor subdevices: Depth, RGB, IR (Y8/Y8I/Y12I), and IMU. Each has separate V4L2 subdev ops structs: |
| 37 | +- `ds5_depth_subdev_ops`, `ds5_ir_subdev_ops`, `ds5_rgb_subdev_ops`, `ds5_imu_subdev_ops` |
| 38 | +- Mux ops: `ds5_mux_subdev_ops`, `ds5_mux_pad_ops`, `ds5_mux_core_ops`, `ds5_mux_video_ops` |
| 39 | + |
| 40 | +A deserializer abstraction layer (`struct dser_interface`) provides function pointer tables for MAX9296 vs MAX96712 variants. |
| 41 | + |
| 42 | +- **SerDes pipe configuration**: the **driver** configures all four SerDes pipes (Depth, RGB, IR, IMU) at stream start via `ds5_configure()`. The D457 firmware does **not** configure any pipes. Do not add probe-time pipe setup or special-case individual pipes (e.g. IMU) in `ds5_probe()`. |
| 43 | +- **Device tree assumptions**: all supported device trees include all four sensor instances (Depth, RGB, IR, IMU). Do not add DT-scanning logic to check for the presence of individual sensor types. |
| 44 | + |
| 45 | +### Video Device Layout (per camera) |
| 46 | + |
| 47 | +| Device | Stream | Format | |
| 48 | +|---------|-----------------|------------------------| |
| 49 | +| video0 | Depth | Z16 | |
| 50 | +| video1 | Depth metadata | D4XX custom format | |
| 51 | +| video2 | Color RGB | RGB888/YUV422 | |
| 52 | +| video3 | Color metadata | D4XX custom format | |
| 53 | +| video4 | IR | GREY, Y8I, Y12I | |
| 54 | +| video5 | IMU | Custom | |
| 55 | + |
| 56 | +### Tests (Python — `test/v4l2_test/`) |
| 57 | + |
| 58 | +- **Framework**: pytest with marker `@pytest.mark.d457` on all test classes. |
| 59 | +- **Class-based tests**: `class TestCameraDiscovery`, `class TestLaserControl`, `class TestFirmwareVersion`, etc. |
| 60 | +- **Test naming**: `test_at_least_one_camera`, `test_driver_name`, `test_six_devices_exist`, `test_fw_version_format`. |
| 61 | +- **Fixtures**: session-scoped `all_cameras`, `camera`; per-test `depth_device`, `fw_version`. |
| 62 | +- **Constants**: `test/v4l2_test/d4xx/constants.py` mirrors driver CIDs from `d4xx.c`. |
| 63 | +- **Categories**: Discovery, Streaming, Controls, Metadata, Error Handling. |
| 64 | +- **Streaming validation**: FPS tolerance 5%, frame count 60, min 90% frame arrival, consecutive drop limit 2. |
| 65 | +- **Test timeout**: 200 seconds (configured in `test/pytest.ini`). |
| 66 | + |
| 67 | +### Shell Scripts |
| 68 | + |
| 69 | +- Use `#!/bin/bash` with `set -e` (fail-on-error). |
| 70 | +- Source `scripts/setup-common` for JetPack version normalization. |
| 71 | +- Use `DEVDIR=$(cd \`dirname $0\` && pwd)` to resolve repo root. |
| 72 | +- Branch on JetPack major version (4.x / 5.x / 6.x) for platform-specific logic. |
| 73 | + |
| 74 | +### Device Tree |
| 75 | + |
| 76 | +- **Xavier (Tegra194)**: `.dtsi` includes, pattern `tegra194-camera-d4xx-{variant}.dtsi`. |
| 77 | +- **Orin (Tegra234)**: DT overlays (`.dts`), pattern `tegra234-camera-d4xx-overlay-{variant}.dts`, uses `/dts-v1/; /plugin/;`. |
| 78 | +- Variants: `single`, `dual`, `single.calib`, `dual.calib`, `fg12-16ch`, `max96712-EVB`. |
| 79 | + |
| 80 | +## Build System |
| 81 | + |
| 82 | +### Prerequisites |
| 83 | + |
| 84 | +```bash |
| 85 | +sudo apt install -y build-essential bc wget flex bison curl libssl-dev xxd |
| 86 | +``` |
| 87 | + |
| 88 | +### Full Build Flow |
| 89 | + |
| 90 | +```bash |
| 91 | +./setup_workspace.sh <version> # Clone NVIDIA sources, install toolchain |
| 92 | +./apply_patches.sh <version> # Apply D4XX patches to kernel + NVIDIA OOT modules |
| 93 | +./build_all.sh <version> # Build kernel, DTBs, and driver modules |
| 94 | +``` |
| 95 | + |
| 96 | +Build outputs go to `images/<version>/`. |
| 97 | + |
| 98 | +### Version Mapping |
| 99 | + |
| 100 | +| JetPack | L4T Revision | Kernel Dir | Normalized | |
| 101 | +|---------|-------------|-------------------------------|------------| |
| 102 | +| 4.6.1 | 32.7.1 | `kernel/kernel-4.9` | 4.6.1 | |
| 103 | +| 5.0.2 | 35.1 | `kernel/kernel-5.10` | 5.x | |
| 104 | +| 5.1.2 | 35.4.1 | `kernel/kernel-5.10` | 5.x | |
| 105 | +| 6.0 | 36.3 | `kernel/kernel-jammy-src` | 6.x | |
| 106 | +| 6.1 | 36.4 | `kernel/kernel-jammy-src` | 6.x | |
| 107 | +| 6.2 | 36.4.3 | `kernel/kernel-jammy-src` | 6.x | |
| 108 | +| 6.2.1 | 36.4.4 | `kernel/kernel-jammy-src` | 6.x | |
| 109 | + |
| 110 | +### Cross-Compilation Toolchains |
| 111 | + |
| 112 | +- JP 4.6.1: Linaro GCC 7.3 |
| 113 | +- JP 5.x: Bootlin GCC 9.3 |
| 114 | +- JP 6.x: Bootlin GCC 11.3 (`aarch64-buildroot-linux-gnu`) |
| 115 | + |
| 116 | +### Patch Application |
| 117 | + |
| 118 | +`apply_patches.sh` copies `kernel/realsense/d4xx.c`, device tree files, and `nvidia-oot/max96712.h` into the NVIDIA source tree, applies git patches, and commits with `"RS patched"`. The `reset` action uses `git reset --hard` to a stored base commit. |
| 119 | + |
| 120 | +Camera variant flags: `--one-cam`, `--dual-cam`, `--max96712-EVB`, `--fg12-16ch`, `--fg12-16ch-dual` (only some apply to specific JetPack versions). |
| 121 | + |
| 122 | +## Key Directories |
| 123 | + |
| 124 | +| Path | Description | |
| 125 | +|------|-------------| |
| 126 | +| `kernel/realsense/d4xx.c` | Main V4L2 I2C subdevice driver (~6900 lines) | |
| 127 | +| `kernel/kernel-4.9/` | Kernel patches for JetPack 4.6.1 | |
| 128 | +| `kernel/kernel-5.10/` | Kernel patches for JetPack 5.x | |
| 129 | +| `kernel/kernel-jammy-src/` | Kernel patches for JetPack 6.x | |
| 130 | +| `kernel/nvidia/` | NVIDIA driver patches (MAX9295/9296 SerDes, VI capture) | |
| 131 | +| `nvidia-oot/` | Out-of-tree NVIDIA module patches for JetPack 6.x | |
| 132 | +| `hardware/realsense/` | Device tree source files | |
| 133 | +| `hardware/nvidia/` | Platform-level DT patches | |
| 134 | +| `scripts/` | Build orchestration and SerDes configuration scripts | |
| 135 | +| `test/v4l2_test/` | V4L2 pytest test suite | |
| 136 | +| `utilities/streamApp/` | C++ V4L2 streaming application | |
| 137 | +| `utilities/JsonToBin/` | Python JSON-to-binary preset converter | |
| 138 | + |
| 139 | +## CI |
| 140 | + |
| 141 | +CI workflows (`.github/workflows/build-jp*.yml`) build for each JetPack version on pushes to `master`/`dev` and all PRs. The V4L2 test workflow runs on a self-hosted Jetson runner when `kernel/realsense/**` or `test/v4l2_test/**` paths change. |
| 142 | + |
| 143 | +CI requires `git config user.email/name` to be set before `apply_patches.sh`. |
| 144 | + |
| 145 | +## Branching |
| 146 | + |
| 147 | +- `master` — primary/release branch |
| 148 | +- `dev` — active development branch |
| 149 | +- |
| 150 | +## Refactoring notes (recent commit) |
| 151 | + |
| 152 | +Short summary of approaches used in the refactor of `kernel/realsense/d4xx.c`: |
| 153 | + |
| 154 | +- **Encapsulated shared state**: introduced `struct ds5_dev` (per-camera) and |
| 155 | + `struct dser_control` (per-deserializer) to centralize reset generation, |
| 156 | + cached device type and last-reset timestamps instead of scattered globals. |
| 157 | +- **Separated reset generation**: split camera-level and deserializer-level |
| 158 | + reset counters so camera resets and deserializer resets are independent; |
| 159 | + callers check both generation counters and invalidate cached state on mismatch. |
| 160 | +- **Atomic, lazy invalidation**: replaced many explicit peer-invalidation loops |
| 161 | + with `atomic_inc()` of the appropriate reset-gen; peers detect the bump in |
| 162 | + `ds5_configure()` and invalidate themselves lazily, reducing O(N) churn. |
| 163 | +- **Single registration/link path**: factored duplicate SERDES/board registration |
| 164 | + logic into `ds5_setup_and_link()` to avoid repeated code paths and make |
| 165 | + primary/peer linking explicit. |
| 166 | +- **Keep streaming visibility consistent**: driver now maintains per-camera |
| 167 | + streaming flags (kept in sync from `ds5_mux_s_stream()`) so recovery and |
| 168 | + sibling checks can reliably detect active streams. |
| 169 | +- **Non-SERDES compatibility**: added small fallbacks so callers can use the |
| 170 | + same reset-gen accessors in non-SERDES builds (where applicable). |
| 171 | + |
| 172 | +Advantages: clearer ownership of shared state, fewer global arrays and linear |
| 173 | +searches, more deterministic recovery behavior, and reduced duplicate code. |
| 174 | + |
| 175 | +## Workflow Rules |
| 176 | + |
| 177 | +### Post-patch configuration review (mandatory) |
| 178 | + |
| 179 | +After every confirmed code patch, review and update this file and `CLAUDE.md`: |
| 180 | + |
| 181 | +1. **Stale facts**: correct or remove architectural claims, assumptions, or descriptions that the patch invalidated. |
| 182 | +2. **New conventions**: if the patch exposed a coding practice gap (e.g. a cleanup step that was missed), add a general coding convention so it is enforced going forward — do not just fix the specific instance. |
| 183 | + |
| 184 | +Do not leave stale or incorrect claims in configuration files after changing the code they describe. |
0 commit comments