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

AXI signals snippet #43

Open
valerionew opened this issue Jul 19, 2021 · 1 comment
Open

AXI signals snippet #43

valerionew opened this issue Jul 19, 2021 · 1 comment

Comments

@valerionew
Copy link
Contributor

valerionew commented Jul 19, 2021

PR #41 highlighted the need for AXI snippets. @Bochlin provided a starting point for the requirements of such snippets. Their comment is reported below, and this issue is the place to discuss this topic.

Originally posted by @Bochlin in #41 (comment)

I think that having AXI snippets (as well as snippets for other industry standard busses) would be a very good addition. Some comments though:

I think that aligning could be done as long as it is done using spaces, e.g:

s_axi_awaddr  : in  std_ulogic_vector(11 downto 0);
s_axi_awready : out std_ulogic;

With regards to coding style, there is also the matter of the usage of upper/lower case. While I personally avoid using UPPER_CASE for names except for generics/constants I know that there are shops where e.g. ports are always written in upper case (and to be fair, the AXI signals are defined in UPPER_CASE in the specification). It should also be noted that while the std and ieee libraries do name types using UPPER_CASE many still refer to them using lower case.

Another problem, albeit probably smaller, is the type used for the signals. 'std_logic(_vector)' is in my experience by far the most commonly used, however at my current work, we use 'std_ulogic(_vector)' to allow for (theoretically) faster simulation and the ability for the simulator to detect multiple drivers at compile time.

Using generics to set the widths should also be optional. While it provides a single point of definition for the address and data widths, it does at the same time indicate to the user of the entity that they may be set freely, i.e. the design supports different widths which it may not do. Another use case for not using generics could be that the widths are defined using custom constants in a package

I also think that the name/prefix of the interface, e.g. the M_AXI_ should also be a variable, although it should have a default.

For the full AXI interface, it should be enough with a snippet with all the AXI signals and the user will then have to remove the unused ones.

With AXI5 being released, it might also make sense to add a "4" somewhere in the snippet name.

The above requires the snippets to support a couple of customizations.

  1. UPPER_CASE vs. lower_case identifiers
  2. UPPER_CASE vs. lower_case subtype_indication
  3. Using generics for widths or not
  4. Customizable prefix
  5. Sideband signals for AXI Streaming interfaces.

Some proposals for further discussion (not sure how VS Code treats identical names and upper/lower case in the prefix). One way to have upper/lower case variation in the identifiers would be to make two variants of each template, one with the prefix in lower case and one with the prefix in uppercase.

Snippets

Name Prefix Description
AXI4-Stream Master Interface axi4sm / AXI4SM Basic AXI4 Stream Master interface with tdata, tvalid, tready
AXI4-Stream Slave Interface axi4ss / AXI4SS Basic AXI4 Stream Slave interface with tdata, tvalid, tready
AXI4-Stream Master Interface axi4sm / AXI4SM Basic AXI4 Stream Master interface with tdata, tvalid, tready
AXI4-Stream Slave Interface axi4ss / AXI4SS Basic AXI4 Stream Slave interface with tdata, tvalid, tready
AXI4-Stream Master Interface /w Sideband ? Full AXI4 Stream Master interface
AXI4-Stream Slave Interface /w Sideband ? Full AXI4 Stream Slave interface
AXI4-Lite Master Interface axi4lm AXI4 Lite Master interface
AXI4-Lite Slave Interface axi4ls AXI4 Lite Slave interface
AXI4 Master Interface axi4m AXI4 Master interface
AXI4 Slave Interface axi4s AXI4 Slave interface

Originally posted by @Bochlin in #41 (comment)

@bpadalino
Copy link
Contributor

This issue hasn't gotten any attention in quite some time, but I am curious to resurrect it given some work I've done looking at VHDL-2019 and their interfaces which they call a view.

I wonder if just including some nicely laid out packages are good enough to help out in this scenario. In my case, I prototyped using: axi4l, axi4mm, and axi4s.

The idea is to have a top level package which defines the common datatypes like axi4mm_t for the unconstrained memory mapped record. There is a nested generic package called make which allows for constraining the record in a way that enforces compliant rules with the standard. For example, if tdata is 256 bits wide, then tkeep and tstrb are required to be 256/8. With the regular unconstrained record, there is no way to enforce this. One would then be able to:

package axi256 is new package interfaces.axi4mm.make generic map(READ_CONFIG => CONFIG256, WRITE_CONFIG => CONFIG256) ;

There is then the 256-bit interface in work.axi256.axi4mm_t as well as the unconstrained one in interfaces.ax4imm.axi4mm_t - the former which is a subtype of the latter. Moreover, if a signal is not used, such as tqos or tregion - they are set to a null array (i.e 1 to 0).

With the view of manager and subordinate - I envision a procedure to attach one to the other. Without VHDL-2019, the semantics are less enforced and the signals need to be marked as inout for the procedure, but I imagine through the use of std_ulogic, the error is found at compilation time.

With this being said, do you think snippets are still a good idea for this or would just having a well thought out package and library be enough since the language server should help with finding those items?

NOTE: The nested package idea doesn't currently work with Riviera-PRO, so it might have to turn into a separate package instead of being nested. I liked the idea of the nested package to give hierarchy and context to what it's doing. What do you think?

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

No branches or pull requests

2 participants