Skip to content

add Lattice Nexus vendor platform (using nextpnr-nexus and prjoxide) #759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from

Conversation

slagernate
Copy link
Contributor

sorry, github newb. Won't do that again. Tested again with blinking all LEDs. Output:

python3 -m amaranth_boards.lifcl_evn
init..
IDCODE: 0x110f1043 (LIFCL-40)
NX Status Register: 0x0000150100400100
reset..
NX Status Register: 0x0000150100400e50
programming..
NX Status Register: 0x0000150100400100
Bye.

Notes:
--Removed mention of ECP5 in lattice_nexus.py
--Removed OSC

@codecov
Copy link

codecov bot commented Mar 2, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.44%. Comparing base (b36e7e0) to head (51f5769).
Report is 9 commits behind head on main.

❗ Current head 51f5769 differs from pull request most recent head 8ccc083. Consider uploading reports for the commit 8ccc083 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #759      +/-   ##
==========================================
- Coverage   89.94%   89.44%   -0.50%     
==========================================
  Files          44       43       -1     
  Lines       10647     9066    -1581     
  Branches     2595     2150     -445     
==========================================
- Hits         9576     8109    -1467     
+ Misses        882      781     -101     
+ Partials      189      176      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@richardeoin
Copy link
Contributor

FWIW I've been using amaranth with radiant and a nexus LFD2NX-40 part, and intend to contribute a platform definition when I'm happy with it (likely in a few weeks). I'll check back to see what the status of this is then.

@whitequark
Copy link
Member

I'm currently waiting on a LIFCL-EVN board so that I can do regression testing for this platform.

@whitequark
Copy link
Member

Good news: I've received the devboard I can test this on! I'll merge it soon.

@slagernate
Copy link
Contributor Author

@richardeoin how's your platform file looking?

@richardeoin
Copy link
Contributor

@slagernate Here's the platform file I'm using with LFD2NX-40 + Radiant. I tweaked my formatting to make it somewhat more diff-able against yours.

Some differences

  • Adds a two-stage FF sync into i_GSR_N of GSR (is this necessary?)
  • Adds an OSCA if self.default_clk == "OSCA"
  • Specifies p_GSR="DISABLED" on IDDRX1 and IDDRX2 instances 😱 Probably don't do this
  • Less tortured templating syntax for generating the .pdc constraints

I initially tested without triggering a GSR after startup and observed some very unexpected behaviour from IDDR ODDR and BB primitives. After that I assume that triggering a GSR after startup is mandatory for correct operation on these parts. Skip my workaround of putting p_GSR="DISABLED" on IDDRX instances.

I've only tested with radiant and didn't use radiant's programmer tool at all.

@slagernate
Copy link
Contributor Author

slagernate commented May 28, 2023

See latest suggested commit where I've pulled in your platform file @richardeoin . This works for the Radiant and Oxide / NextPnr tool chain with the LIFCL-40-EVN (Crosslink-NX Eval Board).

@whitequark
Copy link
Member

The current plan is to go through the backlog of PRs after amaranth-lang/rfcs#2 gets merged.

@richardeoin
Copy link
Contributor

There have been subsequent improvements to amaranth/vendor and this PR needs at a minimum to be updated to include those.

@slagernate are you willing to maintain this PR (no is an ok answer!! :) )? It would be great to see it merged, and if you can't I'd happy to do the work (and have my own LFD2NX-40 test hardware).

@slagernate
Copy link
Contributor Author

@richardeoin more than willing to update the PR, just need to find some spare time (perhaps in the next week). Have been using Amaranth + Radiant quite a bit (the PCLKDIV primitive in Oxide is missing :| ). But also totally happy if you'd like to take the reins!

@whitequark
Copy link
Member

This PR has languished for a bit too long--I'll add it to the 0.5 milestone to make sure it's merged before then.

@whitequark whitequark added this to the 0.5 milestone Jan 30, 2024
@slagernate
Copy link
Contributor Author

Should be compatible with RFC#18 now.

@slagernate
Copy link
Contributor Author

Updated to use tcl_quote instead of tcl_escape.

{% for net_signal, port_signal, frequency in platform.iter_clock_constraints() -%}
{#
{% if port_signal is not none -%}
set_frequency "{{port_signal.name}}" {{frequency/1000000}};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this have different syntax than the below? Is one of the commands broken?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set_frequency is not supported by nextpnr-nexus, so this has been removed in 8ccc083 (tbh I don't remember why the syntax differs, or where set_frequency came from).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's legitimate--how would nextpnr know the timing target for your design? Or, equally, how do you know that your design will function with the clock you give it?

Copy link
Contributor Author

@slagernate slagernate Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried looking into how to pass timing info to nextpnr and according to nextpnr/docs/nexus.md:

"Timing constraints are currently ignored, but should be expected to be supported soon."

A grep of set_frequency in nextpnr only surfaces results for ice40.

A workaround I've been using is os.environ["AMARANTH_nextpnr_opts"] = "--freq 66.67 -r -v"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gatecat Do you think that's something we can have shipped in nextpnr soonish? It's fairly hard to convince myself to add support for the --freq workaround.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The create_clock command should work in a PDC file - it seems like the docs are just out of date, I'll fix them tomorrow.

Clocks should also be automatically propagated through PLLs when that is used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm, that works with Radiant as well, right?

return True
return False

def _get_xdr_buffer(self, m, pin, *, i_invert=False, o_invert=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that all of this stuff will need to be completely redone as per RFC 55 — see #1295 for an example of the changes required.

@whitequark
Copy link
Member

Superseded by #1342, since recent internal refactoring made this PR unmergeable. However, #1342 is based on this work--thank you!

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

Successfully merging this pull request may close these issues.

5 participants