VHDL-LS / rust_hdl_vscode

VHDL Language Support for VSCode
MIT License
50 stars 17 forks source link

AXI signals snippet #43

Open valerionew opened 3 years ago

valerionew commented 3 years ago

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 https://github.com/VHDL-LS/rust_hdl_vscode/issues/41#issuecomment-882059306_

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 https://github.com/VHDL-LS/rust_hdl_vscode/issues/41#issuecomment-882059306_

bpadalino commented 1 year ago

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?