Repository navigation
hw/arm/rk3566: model the camera pipeline and complete NPU reuse - #49
Conversation
The ISP model only forwarded host camera frames and latched the capture registers, so the vendor rkisp driver could stream but rkaiq had no SOF event or measurements and the statistics video node never completed a buffer. Generate a moving colour-bar frame every 1/30 s when no camera chardev is attached, keep the chardev path for injected frames, report V_START and FRAME interrupts separately so the driver queues SOF events, and fill the AE/AWB/histogram/AF blocks in the DDR statistics buffer announced through MI_SWS_3A_WR_BASE. The measurement register window keeps reporting done bits for the register readback path. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Implement the GC2093 I2C register contract used by the vendor driver: 16-bit register addressing with an auto-incrementing pointer, read-only chip identification (0x2093), soft reset through 0x3fe=0xf0 that restores the reset defaults while keeping the chip ID, streaming control through CTRL_MODE (0x3e=0x91/0x11) with the streaming GPIO following the mode, and register storage for the sensor mode table, long exposure, VTS, mirror/flip and gain registers. The exposure and VTS high bytes keep their 14-bit field width, and power or reset assertion restores the reset defaults. Update the RK3566 board documentation with the camera pipeline, the optional camera chardev and the known camera model limitations. Signed-off-by: Chao Liu <chao.liu@processmission.com>
The ISP generates frames and statistics without a host chardev, so update the camera section accordingly. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Introduce a RockchipRKNNSocConfig descriptor and make the RKNPU core realize the per-SoC values instead of machine-set properties: core count, command count scale, DMA address width, PC version and the legacy PC+0x8000 amount window. The rk3566-rknpu subtype now selects the RK356x description (one core, 32-bit DMA, amount window). Clamp co-work requests to the configured core count so a single-core RK3566 completes multi-core CNA tasks on its own core instead of waiting for peers that do not exist. Scan peers up to the per-SoC core count in the reset and migration paths. Expose the RK356x amount window as an additional sysbus region and map it at 0xfde48000 in the microduck machine. The four registers model the vendor driver clear-all strobe and the data read/write counters. Add a rknpu-reset gpio output to the RK3566 CRU driven by SOFTRST_CON2 bits 11/12 (SRST_A_NPU/SRST_H_NPU) and connect it to the NPU reset input, so the core register file, LUT, pending state, IRQ and amount counters are reset through the CRU. Add tests/qtest/rk3566-rknpu-test.c covering version/pointer tags, control command completion, IRQ mask/clear, fetch error, 32-bit DMA rejection, amount window semantics, CRU reset and single-core co-work. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 520a0901c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| isp_3a_store_words(buf, 0x0c00 + i * 0x800, s->stat_hist[i], | ||
| ISP_3A_HIST_BINS); |
There was a problem hiding this comment.
When DDR statistics are enabled, the i == 2 iteration writes stat_hist[2] at offset 0x1c00, but the subsequent stat_af_ram write uses the same offset and overwrites its first 225 bins. Consequently, the DDR buffer cannot contain both the third histogram and AF measurements correctly, and guest 3A consumers receive corrupted statistics. Use non-overlapping offsets matching the statistics buffer layout.
Useful? React with 👍 / 👎.
| static void isp_3a_store_words(uint8_t *buf, hwaddr offset, | ||
| const uint32_t *values, unsigned count) | ||
| { | ||
| memcpy(buf + offset, values, count * sizeof(uint32_t)); |
There was a problem hiding this comment.
On a big-endian host, this memcpy serializes every measurement word in host byte order. address_space_write() copies those bytes unchanged, so the little-endian RK3566 guest reads byte-swapped AE, histogram, AWB and AF statistics even though MMIO readback remains correct. Store each word with an explicit little-endian operation rather than copying native uint32_t representations.
Useful? React with 👍 / 👎.
| if (!s->busy) { | ||
| isp_synth_generate(s); | ||
| isp_submit_frame(s); |
There was a problem hiding this comment.
With no camera chardev attached, input_frames never advances: its only increment is in isp_receive(), which the synthetic path bypasses. Since isp_synth_generate() uses that counter as its animation tick, every generated frame is identical and the exposed input-frames property stays zero despite ongoing capture. Increment the counter for synthetic frames as well so the documented moving pattern actually changes between captures.
Useful? React with 👍 / 👎.
The third histogram block was written at the AF offset and the per-block measurement done bits shared one flag, so rkaiq could read a histogram filled with AF data and could observe measurements that were already acknowledged. Write the histogram blocks at 0x0c00, 0x1400 and 0x1800, track the measurement done state per AE, histogram, AF and AWB block, advance the synthetic frame counter so the generated pattern moves, and report a DMA error when the statistics buffer cannot be reached. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Keep the RK356x PC+0x8000 amount counters in the migration stream. Signed-off-by: Chao Liu <chao.liu@processmission.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6f35b9222
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| VMSTATE_UINT32_ARRAY_V(amount_counts, RockchipRKNNCoreState, | ||
| ROCKCHIP_RKNN_AMOUNT_COUNTERS, 2), |
There was a problem hiding this comment.
When loading a version-2 NPU migration stream or snapshot created before this change, this field is incorrectly considered present even though that format contains no amount counters. The loader consumes the first three CNA register words as counters, shifting all subsequent fields and causing state corruption or a load failure. This also affects RK3588, which uses the shared VMState description. Set the field's minimum version to 3, matching the version introduced by this commit.
Useful? React with 👍 / 👎.
|
Validation on the final head (b6f35b9, build with media libraries):
The v0.1.0 run also captured a 30-frame stream (93312000 bytes) and the mediad journal shows the 3A exposure loop adjusting the synthetic frame (mean luma 137.1 against a target of 90.0). Remaining device-side warnings are unchanged and documented in the commit messages: the NPU driver falls back from the 64 KiB region request and the |
The matmul co-work case starts a functional task that the model executes in an asynchronous worker and then steps the virtual clock in a tight loop. On a loaded CI host the worker may not have finished when the loop ends, so the case observed a still busy PC. Give the worker real time between the virtual clock steps and keep stepping until the task status reports completion. Signed-off-by: Chao Liu <chao.liu@processmission.com>
8d92032 to
2673c35
Compare
Move the RKNPU register file, command decoder, execution backend, DMA and interrupt logic into hw/misc/rockchip_rknpu.c and describe the per-SoC differences with RockchipRKNNSocConfig. RK3588 and RK3566 provide thin subclasses that select their configuration, and the machines instantiate the SoC-specific device type. Drop the pre-release migration compatibility: the VMState stream has a single version and all fields are unconditional, so the older version checks and the versioned fields are removed. Signed-off-by: Chao Liu <chao.liu@processmission.com>
|
Added a follow-up commit that shares the RKNPU core across SoCs:
Local validation after the refactor: checkpatch clean for the new and moved files; |
Boot the four official MicroDuck firmware releases with the microduck-beta machine and verify that the guest reaches a shell, reports the board model, exposes the rkisp video nodes, runs the rkaiq service, registers the rknpu DRM and debugfs nodes, and captures a mainpath frame. The release images are several gigabytes, so the assets are read from QEMU_TEST_MICRODUCK_FIRMWARE_DIR instead of being downloaded, and the tests are skipped unless that variable and QEMU_TEST_ALLOW_LARGE_STORAGE are set. Boot a release up to three times before failing it, because a TCG boot of the v0.1.0 image occasionally stalls before the serial getty starts, and keep the guest console quiet to avoid stalling the boot log. The tests join the aarch64 thorough functional suite. Signed-off-by: Chao Liu <chao.liu@processmission.com>
|
Added functional tests for the official firmware releases:
Local validation on this commit (all four releases): boot to shell, model |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd89dc9887
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| for (unsigned blk = 0; blk < ISP_3A_AE_BLOCKS; blk++) { | ||
| for (unsigned win = 0; win < ISP_3A_AE_WINDOWS; win++) { | ||
| unsigned lum = CLAMP(mean_y + (int)win * 4 - 6, 0, 0xff); |
There was a problem hiding this comment.
With an external camera frame whose mean luminance is below 6, mean_y being unsigned makes the subtraction wrap before CLAMP runs. For example, a completely black frame produces lum == 255 for windows 0 and 1 rather than zero, so their AE sums report maximum brightness while the histogram and mean measurements report darkness. Cast mean_y to a signed type before applying the window adjustment, as the mean-grid calculation below already does.
Useful? React with 👍 / 👎.
Summary
Model the RK3566 camera pipeline and complete the RK3566 NPU reuse.
Camera pipeline
camerachardev is attached; the optional chardev still overrides the generator with injected 640x360 UYVY frames.MI_SWS_3A_WR_BASE. The register readback path keeps reporting measurement done bits.With these changes the official MicroDuck firmware captures 1920x1080 NV12 frames from
/dev/video0without a host frame source andrkaiq_3Acompletesrk_aiq_uapi_sysctl_startandrkisp_init enginewithout AWB or SOF errors.RK3566 NPU
RockchipRKNNSocConfigto the shared RKNN core so per-SoC differences are described in one place: core count, command count scale, DMA address width, PC version and the amount window description. Therk3566-rknpusubtype selects the RK356x configuration.0xfde48000.rknpu-resetoutput to the RK3566 CRU (SOFTRST_CON2 bits 11/12) and connect it to the NPU reset input; reset clears the register file, LUT, pending command and interrupt state.tests/qtest/rk3566-rknpu-test.ccovering the version registers, pointer tags, command completion, interrupt masking, fetch errors, 32-bit DMA, the amount window, reset and single-core co-work.Validation
checkpatch.pl --filereports 0 errors and 0 warnings for every changed file.qemu:qtest-aarch64andqemu:qtest-arm: no failures (details in the CI run)./dev/video0;rkaiq_3Astarts the tuning engine; the rknpu driver reaches[drm] Initialized rknpu 0.9.8with IOMMU enabled.Known limitations