Skip to content

phy: Align Artix-7 DDR3 reads without altering CL - #400

Merged
enjoy-digital merged 1 commit into
masterfrom
fix-a7-ddr3-read-alignment
Jul 23, 2026
Merged

phy: Align Artix-7 DDR3 reads without altering CL#400
enjoy-digital merged 1 commit into
masterfrom
fix-a7-ddr3-read-alignment

Conversation

@enjoy-digital

@enjoy-digital enjoy-digital commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Follow-up to #397 and @gatecat's observation about the similar Artix-7 read-path workaround.

Summary

  • Keep the frequency-selected DDR3 CL unchanged in MR0.
  • Move the Artix-7 read command by one memory-clock phase to retain the established ISERDESE2 word alignment.
  • Fold DFI phase wrapping into the controller-visible read latency.
  • Add Series-7 settings and MR0 tests covering default frequencies, every legal DDR3 CL, command latencies, and the unchanged Kintex-7 path.

Rationale

Since 6f323f6, the Artix-7 PHY has calculated its read phase and system latency from the selected CL but programmed CL + 1 in MR0. This places returned data one memory-clock phase into the ISERDESE2 word and improved read-leveling reliability, but it represents PHY alignment by changing the DRAM timing configuration. It also turns an explicitly selected CL 14 into the unsupported MR0 value 15.

The new schedule keeps the same intra-word read position by incrementing rdphase instead. divmod() normalizes the phase and carries a phase wrap into read_latency, making both parts of the timing relationship explicit.

Testing

  • pytest -q test/test_s7ddrphy.py test/test_ddr3_phy_settings.py test/test_nxddrphy.py test/test_init.py test/test_gen.py: 13 passed, 86 subtests passed.
  • Artix-7 PHY standalone Verilog elaboration: 2,628 lines generated with the expected OSERDESE2/ISERDESE2 instances.
  • Digilent Arty SoC generation with gateware/software compilation disabled: completed successfully; generated settings are CL 6, CWL 5, read phase 3, and write phase 3 at 100 MHz.

Hardware Validation

This is intentionally a draft until it has been exercised on Artix-7 hardware. The useful checks are read-leveling windows and repeated memory tests at 50, 75, 100, and 125 MHz. A phase-wrap configuration, such as CL 13, should also be tested if practical because it reduces the controller-visible read latency by one system cycle.

The Artix-7 DDR3 PHY previously programmed MR0 with CL + 1 while calculating its read phase and latency from the selected CL. This placed returned data one memory-clock phase into the ISERDESE2 word, but mixed PHY alignment with the DRAM timing configuration and made an explicit CL 14 unencodable.

Keep the selected CL in MR0 and move the Artix-7 read phase by one memory clock instead. Fold phase wrapping into the controller-visible read latency so the returned data keeps the required word alignment.

Add settings and MR0 tests across the default frequency ranges, all legal DDR3 CL values, command latencies, and the unchanged Kintex-7 path.
@enjoy-digital
enjoy-digital marked this pull request as ready for review July 23, 2026 21:15
@enjoy-digital

Copy link
Copy Markdown
Owner Author

Validated 624444c on an Arty A7-35 with LiteX 93c8d230, LiteX-Boards
2f06d08a, and Vivado 2025.2.

Build results:

  • Default Arty design at 100MHz, with DDR3 at 800MT/s.
  • Synthesis, placement, routing, DRC, and bitstream generation completed successfully.
  • Timing met with setup WNS +0.571ns and hold WHS +0.017ns, with no failing endpoints.

Hardware results:

  • Loaded the bitstream into an Arty A7-35 over JTAG.
  • The BIOS reports CL-6 CWL-5, confirming that MR0 keeps the selected DDR3 CL instead of using the previous CL+1 workaround.
  • Read leveling found stable windows on both byte lanes (18+-6 and 18+-5 taps).
  • The automatic 2MiB memory test completed with Memtest OK.
  • An additional 8MiB sdram_test completed with Memtest OK.

This confirms the adjusted read schedule works on Artix-7 hardware while preserving the frequency-selected DDR3 CL. CI is also green, so I am going to merge this.

@enjoy-digital
enjoy-digital merged commit 1b7cc79 into master Jul 23, 2026
2 checks passed
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