Skip to content

Fix extended native CRT protocol compatibility - #5

Open
wizzomafizzo wants to merge 1 commit into
masterfrom
fix/mister-native-video-contract
Open

wizzomafizzo wants to merge 1 commit into
masterfrom
fix/mister-native-video-contract

Conversation

@wizzomafizzo

@wizzomafizzo wizzomafizzo commented Sep 16, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Accept both legacy native-video magic 0x5A50 and frontend extended 0x5A51, decoding their different mode/vertical-offset layouts explicitly.
  • Widen signed vertical path and implement the common safe -31..+9 pixel / -14..+2 line trim window while preserving legacy clamps and line/field totals.
  • Cover exact PAL control word, wider trims, legacy compatibility, unknown magic, and progressive/interlaced timing in RTL regressions.

Verification

All six Icarus suites passed. Quartus compile and setup/hold/recovery/removal/pulse-width timing gate passed; timing-gate Python tests passed. Hardware PAL output visible with unchanged extended word 0x5A5100F6 after explicitly loading new RBF; updated HDMI scanout/OSD also checked.

Analog CRT quality remains unqualified. Kernel/module ABI unchanged. Load this Menu before enabling Slint native CRT; legacy writers remain supported. Replacing RBF and restarting Main alone does not program FPGA—use normal load_core path. Rollback RBF preserved.

Related frontend: ZaparooProject/zaparoo-frontend#457
Related Main: ZaparooProject/Main_MiSTer#26

Summary by CodeRabbit

  • New Features
    • Added support for extended video control blocks with expanded signed horizontal and vertical offset ranges.
    • Added support for PAL and 480i timing adjustments using the extended control format.
    • Preserved compatibility with legacy control blocks, including their existing offset limits.
  • Bug Fixes
    • Invalid control modes now safely default to mode 0.
    • Improved handling of extreme timing offsets while preserving correct video field timing.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 965ea0d5-d544-4f6a-bd7e-daa66bc2d1b0

📥 Commits

Reviewing files that changed from the base of the PR and between e8c8680 and 5c2cb49.

📒 Files selected for processing (5)
  • rtl/native_video_reader.sv
  • rtl/native_video_timing.sv
  • rtl/native_video_top.sv
  • tb/native_video_reader_tb.sv
  • tb/native_video_timing_tb.sv

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The video path now supports legacy 0x5A50 and extended 0x5A51 control blocks. It carries signed 6-bit vertical offsets, applies expanded horizontal and vertical timing ranges, preserves legacy clamping, and adds protocol and timing coverage.

Changes

Extended video offsets

Layer / File(s) Summary
Reader protocol handling
rtl/native_video_reader.sv, tb/native_video_reader_tb.sv
The reader recognizes 0x5A50 and 0x5A51, parses extended mode and signed offsets, applies legacy clamping, widens v_offset_out, and rejects unknown magic values. The reader testbench covers extended, legacy, and invalid blocks.
Timing offset ranges
rtl/native_video_timing.sv, rtl/native_video_top.sv, tb/native_video_timing_tb.sv
The timing path carries signed 6-bit vertical offsets and clamps horizontal offsets to -31..+9 and vertical offsets to -14..+2. The synchronizer and timing tests use the widened signal and verify NTSC, PAL, and 480i timing.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant DDR
  participant native_video_reader
  participant native_video_top
  participant native_video_timing
  DDR->>native_video_reader: Provide 0x5A50 or 0x5A51 control block
  native_video_reader->>native_video_top: Output mode and signed offsets
  native_video_top->>native_video_timing: Synchronize and forward offsets
  native_video_timing->>native_video_timing: Clamp offsets and calculate sync starts
Loading

Merge Risk: ⚪ Minimal · up to 5c2cb

The extended protocol and signed offset paths are consistent across the reader, top-level, timing module, and covered testbenches, with no unresolved merge risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: support for extended native CRT protocol compatibility. It is concise and directly related to the protocol, RTL, and testbench updates.

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant