diff --git a/phase1_plan_v3_amendment.md b/phase1_plan_v3_amendment.md new file mode 100644 index 0000000..4a3a246 --- /dev/null +++ b/phase1_plan_v3_amendment.md @@ -0,0 +1,49 @@ +# Phase 1 plan v3 — amendment per Janet re-review of v2 + +Date: 2026-05-17 ~01:55. Janet AMEND-cycle re-review of v2 returned verdict AMEND with two surgical corrections that don't require re-architecture. This document is the v3 delta; v2 stands as the architecture-of-record except for these two paragraphs. + +## Amendment A: VP9 writes FOUR register-block segments, not five + +v2 said `rkvdec-vdpu381-vp9.c` would model on `rkvdec-vdpu381-hevc.c`'s five-segment write pattern: COMMON_REGS / CODEC_PARAMS_REGS / COMMON_ADDR_REGS / CODEC_ADDR_REGS / POC_HIGHBIT_REGS. + +**Correction**: `OFFSET_POC_HIGHBIT_REGS` is HEVC + H.264 only — POC (Picture Order Count) is not a VP9 concept. BSP `hal_vp9d_vdpu382.c` writes only four segments (COMMON, CODEC_PARAMS, COMMON_ADDR, CODEC_ADDR) plus reads INTERRUPT_REGS + STATISTIC_REGS for status. No `poc_highbit` reference exists in any VP9 BSP file. + +**Implementation impact**: `rkvdec_vdpu381_vp9_run` issues 4 `rkvdec_memcpy_toio` calls (one per write segment), not 5. The `struct rkvdec_vp9_regs` definition in the extended `rkvdec-vdpu381-regs.h` omits a `poc_highbit` sub-struct. + +## Amendment B: Common-file split — static-inline helpers in a header, not a separate `.c` + +v2 proposed extracting `rkvdec-vp9-common.{c,h}` with both probability-table init AND `config_seg_registers` / `config_ref_registers` from legacy `rkvdec-vp9.c`. + +**Correction**: `config_registers`, `config_seg_registers`, `config_ref_registers` all write to `regs->...` (the LEGACY flat `struct rkvdec_regs`). They cannot move to a common module that the vdpu381 backend can consume, because the vdpu381 backend uses a different register struct (`struct rkvdec_vp9_vdpu381_regs`). Moving them would either require an abstract register-backend layer (over-engineering) or break RK3399 by replacing them with a vdpu381-flavoured version. + +**Revised boundary**: extract ONLY pure-computation / spec-only functions into `rkvdec-vp9-common.h` as `static inline` helpers (no `.c` file): +- Probability table init (`init_intra_only_probs`, `init_inter_probs`, `init_probs`) +- `rkvdec_vp9_priv_tbl` allocation/free (the buffer-layout logic, not the register-write side) +- Segmap state machine (`segmapid` tracking) +- Entropy update path (`rkvdec_vp9_done` → `rkvdec_init_v4l2_vp9_count_tbl`) + +`config_seg_registers` and `config_ref_registers` are **REWRITTEN** in `rkvdec-vdpu381-vp9.c` against the vdpu381 register struct; the legacy versions stay in `rkvdec-vp9.c` unchanged. Code duplication is acceptable here because the two register structs are genuinely different; abstracting them is more code than duplicating. + +**Regression risk**: drops to ~zero. The legacy `rkvdec-vp9.c` retains all its register-writing functions; the only delta is its `#include "rkvdec-vp9-common.h"` and the deletion of the spec functions whose bodies moved into the header. Compile-test of `rk3399_defconfig` becomes a fast sanity check, not a deep validation effort. R8 from v2 stands but is downgraded from BLOCKER-risk to AMENDMENT-risk. + +## Free implementation hint + +`VDPU381_MODE_VP9 = 2` is **already defined** in `rkvdec-vdpu381-regs.h:22`. The new backend's `dec_mode` field assignment is `regs->common.reg009_dec_mode.dec_mode = VDPU381_MODE_VP9` — constant exists, just write to it. Janet flagged this is the single most likely "copy-paste-bug" point: legacy writes `regs->common.reg02.dec_mode = RKVDEC_MODE_VP9` against a different struct. Do not let muscle-memory bridge the two namespaces. + +## RCB sizing — Phase 6 follow-up + +v2 marked RCB sizing as PROCEED-with-note (HEVC-tuned, oversized but harmless). Adding to Phase 6 punch-list: derive VP9-correct RCB sizes from BSP `vp9d_refine_rcb_size` (`mpp/hal/rkdec/vp9d/hal_vp9d_vdpu382.c:269`) as a follow-up patch after first-light VP9 decode works. Sub-optimal SRAM use, not blocking. + +## File list (final for Phase 2) + +| File | Action | Est. lines | +|---|---|---| +| `rkvdec-vdpu381-vp9.c` | NEW backend, 4-segment register write | 550-700 | +| `rkvdec-vp9-common.h` | NEW header, static-inline helpers only | 200-300 | +| `rkvdec-vdpu381-regs.h` | EXTEND with `struct rkvdec_vp9_regs` (4-segment) | +150-200 | +| `rkvdec-vp9.c` | MINIMAL refactor — `#include "rkvdec-vp9-common.h"` + delete moved bodies | -50, +1 | +| `rkvdec.c` | WIRE — `vdpu38x_vp9_ctrls` + entry in `vdpu381_coded_fmts[]` | +25 | + +## Verdict carry-forward + +Janet v2 verdict was AMEND with these two amendments. With Amendment A + B applied (as documented above), the plan is **PROCEED**. Phase 2 implementation can begin.