Files

T

claude-noether 5a8403a0bb Phase 1 v3 amendment: VP9 writes 4 segments not 5, narrow common-file split

Janet AMEND-cycle re-review of v2 returned 2 surgical corrections:

A. OFFSET_POC_HIGHBIT_REGS is HEVC/H.264 only — POC isn't a VP9
   concept. rkvdec-vdpu381-vp9.c writes 4 segments (COMMON,
   CODEC_PARAMS, COMMON_ADDR, CODEC_ADDR), not 5. Confirmed via BSP
   hal_vp9d_vdpu382.c — no poc_highbit reference anywhere in VP9
   path.

B. Common-file split narrowed: only spec-computation functions
   (probability init, segmap state, entropy update) move into a
   rkvdec-vp9-common.h header as static-inline helpers. The
   register-writing functions (config_seg_registers,
   config_ref_registers, config_registers) STAY in legacy
   rkvdec-vp9.c — they touch the legacy flat struct rkvdec_regs
   and can't share with the new vdpu381 struct without abstracting
   the register backend (over-engineering). The new backend
   rewrites these against the vdpu381 struct.

Bonus implementation hint: VDPU381_MODE_VP9=2 is already defined in
rkvdec-vdpu381-regs.h:22 — Casanova's v7.0 series had VP9 in mind
from the start, just didn't ship a backend.

RCB sizing tuning deferred to Phase 6 (vp9d_refine_rcb_size formula
from BSP).

With both amendments applied, Janet's verdict carry-forward is
PROCEED. Phase 2 implementation begins next session.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

2026-05-16 23:12:47 +00:00

4.5 KiB

Raw Permalink Blame History

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.

4.5 KiB Raw Permalink Blame History