Skip to content
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

Add LFSR generator to standard library #72

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adamgreig
Copy link
Contributor

@adamgreig adamgreig commented Aug 5, 2024

This is my first draft of an LFSR generator for the standard library, following in the general design of the existing CRC generator. There are a few unresolved details and some remaining questions on exactly how the implementation will work, but I am working on a first pass at that too.

Rendered

@whitequark whitequark added the area:core RFC affecting APIs in amaranth-lang/amaranth label Aug 5, 2024
@adamgreig
Copy link
Contributor Author

We discussed this RFC in today's meeting.

On alternatives:

We could simplify the implementation and API by only supporting Galois or only Fibonacci implementations, with the downside of potentially less efficient implementations on some platforms. I haven't researched how serious an impact this would be.

Keep both for now. We could deprecate one later if needed without changing the interface.

We could not support multi-bit generation, but this is a useful feature for many applications.

Keep it as it's vital functionality for planned use cases.

The split between Algorithm and Parameters mirrors that in lib.crc, but with fewer parameters we could consider merging them or making the Parameters settings be part of the Processor creation.

Keep them, the utility seems worth the minimal complexity.

On the unresolved questions:

Not sure if we should add explicit ready/valid signals or a reset signal, or leave those to EnableInserter/ResetInserter.

Swap to using streams for ready/valid, and keep ResetInserter for resetting.

Should we support internally-inverted LFSRs using XNOR operations, permitting a valid all-0s internal state to make initialisation easier e.g. on ASICs?

Requiring all-0s initial values can be left to synthesis tools. No need to include here.

Future work

Add some way to load a specific new state. The proposed option after discussion is a state_init input (which defaults to 1), and whenever the internal state is 0, state_init is used instead. This way the default behaviour is sensible, a ResetInserter can be used to either reset to the normal starting state or to reload a specific new state, and the initial state can be either a fixed constant or changed at runtime. This may increase resource usage compared to a hardcoded initial state or a runtime-reloadable state without the all-0s check, however.

Other points

  • Clarify that the "infinite iterator" returned by Parameters.compute() is a generator
  • Some applications may want to use the state as the multi-bit output, but it's awkward to have this alongside the normal output in the stream. Instead, add an option to Parameters to allow the stream payload to be either the normal output or the internal state.
    • As an alternative, state could remain an output and be used alongside the stream, but this might also be confusing when the multi-bit generation is used.
  • Use an enum to select between Galois and Fibb rather than raw strings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core RFC affecting APIs in amaranth-lang/amaranth
Development

Successfully merging this pull request may close these issues.

2 participants