4

I'm working on a design in Vivado. My top level design is a block diagram. The block diagram has IP blocks and my Verilog RTL modules. Whenever I change my main module and Verilog updates the block diagram, it always incorrectly infers the clock frequency for my module. How can I fix this?

This issue is infuriating, ask it breaks my design. All of the other AXI busses in the design are correctly using 10MHz, but whenever I change main and the block diagram is updated, Vivado decides main's AXI busses are at 100MHz. And as long as the clocks are mismatched, I can't build. I can manually update the frequency in the properties for the block in the block diagram, but those changes are wiped every time I update main (which is frequently, because it's my main module).

I've tried adding a dedicated clock and reset for each AXI bus (even though they all connect to the same net). I've messed around with X_INTERFACE_PARAMETER (is this documented anywhere?). All to no avail.

Also, M_AXIS_CMD is a master AXI interface and should be on the output side of the block for main. Not sure what's up with that. But that's a pretty minor issue compared to the clock.

Source: https://gitlab.com/tessera/pcd8544-tests

Block diagram

Module interface:

module main #(
        CLK_FREQ = 50000000// : CLK_FREQ > 0 // clock frequency
    )(
        (* X_INTERFACE_PARAMETER = "XIL_INTERFACENAME core_clk, ASSOCIATED_RESET core_rst, FREQ_HZ 10000000" *)
        (* X_INTERFACE_INFO = "xilinx.com:signal:clock:1.0 core_clk CLK" *)
        input clk,
        (* X_INTERFACE_PARAMETER = "XIL_INTERFACENAME core_rst, POLARITY ACTIVE_HIGH" *)
        (* X_INTERFACE_INFO = "xilinx.com:signal:reset:1.0 core_rst RST" *)
        input rst,
        (* X_INTERFACE_PARAMETER = "XIL_INTERFACENAME axi_rst, POLARITY ACTIVE_LOW" *)
        (* X_INTERFACE_INFO = "xilinx.com:signal:reset:1.0 axi_rst RST" *)
        input axi_rst,

        (* X_INTERFACE_INFO = "xilinx.com:interface:fifo_write:1.0 DOUT FULL" *)
        input wr_full,
        (* X_INTERFACE_INFO = "xilinx.com:interface:fifo_write:1.0 DOUT WR_DATA" *)
        output reg [8:0] wr_data,
        (* X_INTERFACE_INFO = "xilinx.com:interface:fifo_write:1.0 DOUT WR_EN" *)
        output reg wr_valid,

        // slave AXI-Lite write channel FROM PS
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG AWADDR"  *) input  wire [4:0]  s_axi_reg_awaddr,  // address
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG AWPROT"  *) input  wire [2:0]  s_axi_reg_awprot,  // channel protection type
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG AWVALID" *) input  wire        s_axi_reg_awvalid, // address valid
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG AWREADY" *) output wire        s_axi_reg_awready, // address ready
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG WDATA"   *) input  wire [31:0] s_axi_reg_wdata,   // data
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG WSTRB"   *) input  wire [3:0]  s_axi_reg_wstrb,   // strobes - one bit per byte of data
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG WVALID"  *) input  wire        s_axi_reg_wvalid,  // data/strobes valid
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG WREADY"  *) output wire        s_axi_reg_wready,  // data/strobes ready
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG BRESP"   *) output wire [1:0]  s_axi_reg_bresp,   // response
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG BVALID"  *) output wire        s_axi_reg_bvalid,  // response valid
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG BREADY"  *) input  wire        s_axi_reg_bready,  // response ready

        // slave AXI-Lite read channel FROM PS
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG ARADDR"  *) input  wire [4:0]  s_axi_reg_araddr,  // address
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG ARPROT"  *) input  wire [2:0]  s_axi_reg_arprot,  // channel protection type
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG ARVALID" *) input  wire        s_axi_reg_arvalid, // address valid
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG ARREADY" *) output wire        s_axi_reg_arready, // address ready
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG RDATA"   *) output wire [31:0] s_axi_reg_rdata,   // data
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG RRESP"   *) output wire [1:0]  s_axi_reg_rresp,   // response
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG RVALID"  *) output wire        s_axi_reg_rvalid,  // data/response valid
        (* X_INTERFACE_INFO = "xilinx.com:interface:aximm_rtl:1.0 S_AXI_REG RREADY"  *) input  wire        s_axi_reg_rready,  // data/response ready



        // master AXI-Stream command channel TO DataMover
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 M_AXIS_CMD TDATA"   *) output reg  [71:0] m_axis_cmd_data,
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 M_AXIS_CMD TREADY"  *) output reg         m_axis_cmd_valid,
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 M_AXIS_CMD TVALID"  *) input  wire        m_axis_cmd_ready,

        // slave AXI-Stream status channel FROM DataMover
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 S_AXIS_STS TDATA"   *) input  wire [7:0]  s_axis_status_data,
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 S_AXIS_STS TKEEP"   *) input  wire [0:0]  s_axis_status_keep,
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 S_AXIS_STS TLAST"   *) input  wire        s_axis_status_last,
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 S_AXIS_STS TREADY"  *) input  wire        s_axis_status_valid,
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 S_AXIS_STS TVALID"  *) output reg         s_axis_status_ready,

        // slave AXI-Stream data channel FROM DataMover
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 S_AXIS_DATA TDATA"  *) input  wire [31:0] s_axis_stream_data,
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 S_AXIS_DATA TKEEP"  *) input  wire [3:0]  s_axis_stream_keep,
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 S_AXIS_DATA TLAST"  *) input  wire        s_axis_stream_last,
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 S_AXIS_DATA TREADY" *) input  wire        s_axis_stream_valid,
        (* X_INTERFACE_INFO = "xilinx.com:interface:axis_rtl:1.0 S_AXIS_DATA TVALID" *) output reg         s_axis_stream_ready,

        // DataMover memory-mapped to stream error
        input wire datamover_mm2s_err
    );
Ethan Reesor
  • 2,090
  • 1
  • 23
  • 40

1 Answers1

5

it always incorrectly infers the clock frequency for my module. How can I fix this?

I pulled from your git repo and it worked fine in Vivado 2017.4. You may be conflating a couple different things, and are probably expecting something to happen that isn't supposed to. Which is all totally understandable given the dearth of documentation surrounding the propriety (but useful) Vivado IP packager and block diagram tools.

  • 2017.4 is functionally the same as 2017.3 with added parts, so this shouldn't be causing any differences

  • I removed the FREQ_HZ from the (* X_INTERFACE_PARAMETER ... *) for the clock. This might have caused the clock mismatch you mentioned, but even when I put it back, it works OK for me now. However, FREQ_HZ should only be used on output (generated) clocks; see below.

  • When I first open the diagram or update main.v, and click on the input pin, the properties say 100MHz, as you metioned. But after an F6 "Validate" command, the pin reports 10MHz correctly. This is all expected.

inputpin, pinprops

  • You should NOT expect the CLK_FREQ Verilog parameter to magically update based on the pin. Whenever the module is new the parameters in the GUI will take on the default values. They can be edited and should stay stay through an update cycle, but it's possible they will have to be reset due to major edits. It's up to you to setup these parameters to match.

  • Some Xilinx cores, like AXI Uartlite, use some fancy undocumented TCL scripting to automatically transfer the pin frequency to the Verilog parameters at verification. You can see how this is done, for example, in the /opt/Xilinx/Vivado/2017.4/data/ip/xilinx/axi_uartlite_v2_0/bd/bd.tcl file.

is this documented anywhere?

There is some documentation about using HDL modules in UG994, "Inferring Control Signals in a RTL Module", but it is not very complete. The best documentation for this is in the "Lite Bulb" guy (a distant cousin of Clippy) you get to by pressing the lite bulb in the toolbar of an editor.

litebulbguy

  • Unsolicited advice #1 - Using RTL modules sound like a great idea. Much easier than going through the work of packaging your own IP. Until you find out that: a) it's just packaging the module anyway in the background and automatically re-runs on each edit, b) it's flakier than even the packager, and c) you have no control over the interface inference and so on. Learn to use the packager to create real IP packages.

  • Unsolicited advice #2 - Save the block diagrams and project files at TCL scripts. Don't put the whole project or block diagram .xci and .xml files in Git. See commands like: write_bd_tcl and write_project_tcl. If you (often) need to regenerate the project if it get corrupted, this allows completely consistent results.

  • Finally, here's a snippet of the header the way I'd setup the (* X_INTERFACE... *) stuff. If the clock runs an AXI bus, add the ASSOCIATED_BUSIF flag too:

    (* X_INTERFACE_INFO = "xilinx.com:signal:clock:1.0 core_clk CLK" *)
    (* X_INTERFACE_PARAMETER = "ASSOCIATED_RESET reset" *)
    input clk,
    (* X_INTERFACE_INFO = "xilinx.com:signal:reset:1.0 core_rst RST" *)
    (* X_INTERFACE_PARAMETER = "POLARITY ACTIVE_HIGH" *)
    input rst,
    
Anders
  • 2,270
  • 11
  • 19
  • Advice #1: I looked at using the IP packager, but it seemed to want to create a system-wide IP module, and as far as I can tell it forces me to use two separate instances of Vivado to manage the project. I definitely want everything within git and I'd rather have everything within a single Vivado instance. My goal is most certainly **not** to develop a redistributable "IP". I want my Zynq to do a thing and I'm writing HDL to make it do that thing. So the top-level block diagram _is_ the project. – Ethan Reesor Feb 26 '18 at 22:34
  • One of the things that's confusing is that all of this worked perfectly fine when I only had a single AXI bus. Before I added the DataMover, all I had was `S_AXI_REG`, with it's own associated clock and reset. And Vivado never errored out. But when I added the three AXI busses for the DataMover, things stopped working. At one point, I had five clocks and five resets - the main `clk` and `rst`, and a dedicated clock and reset for each bus. I had `ASSOCIATED_BUSIF` and `ASSOCIATED_RESET` on each clock, copied from how the HDL generated for IP blocks is setup. – Ethan Reesor Feb 26 '18 at 22:39
  • I think it's reasonable for me to set up those parameters manually. What's unreasonable is that Vivado nukes all the manual changes with *every single* module update I've done. So definitely I have to manually change them any time I change the interface of the module, and possibly whenever I change anything in the module. – Ethan Reesor Feb 26 '18 at 22:41
  • 1. True, to manage IP Vivado wants to create it's own mini-project. But that's OK - it's just the way it does it. But search and you see that the HDL "module" feature just dynamically creates a component.xml somewhere deep in the fungible fragile project directory tree. So I create and store a TCL for generating the main project, and then one for each IP in my block diagram. I store these in GIT. And your "ip_repo" that contains the component.xml files can be in GIT too. In fact, you can add the lines to add your user repo to the TCL scripts. – Anders Feb 26 '18 at 22:43
  • #2 and #3. If things are working right, those things shouldn't happen. But projects get corrupt. Vivado crashes. It can even get so confused that it appears to work but gives incorrect results (like TCL command errors) for correct diagrams and scripts. So you have to restart the tool or rebuild the projects. I don't fault Vivado - it isn't perfect, but what it does is crazy complex and it works surprisingly well for an EDA tool. This isn't as simple as software. – Anders Feb 26 '18 at 22:45
  • Sounds like it's time to bite the bullet and set it up as a TCL script project. – Ethan Reesor Feb 26 '18 at 22:48
  • Yep. You can do File->Save Project TCL to get a starting point. I also use the `[file normalize [info script]/../../ip_repo]` construct to make paths relative to the script location (e.g. wherever git cloned to). Again, you want to do the same thing for the block diagrams too: write_bd_tcl gets you started on that. It's much more dependable and reproducible. And pretty readable too. – Anders Feb 26 '18 at 22:51
  • Good luck!!! I hope it works well for you. Ask again or message if you need more advice on issues related to your experience setting up the main or IP projects. I think it would be good to get some more info about how this works out there. I had to do this by trial and mostly error. – Anders Feb 26 '18 at 22:55
  • Do you have an existing public repo set up like this? It would be good to see an example. – Ethan Reesor Feb 26 '18 at 22:56
  • You know, I don't have anything public. That's a damn good idea. I've been thinking about blogging about this stuff. And I don't have a blog either... – Anders Feb 26 '18 at 22:58
  • Considering your knowledge and the lack of documentation on Vivado/Xilinx development (beyond simple surface level stuff), I think that would be a significant boon to the community. – Ethan Reesor Feb 26 '18 at 22:59
  • Is there a way to specify multiple bus interfaces for one clock with `ASSOCIATED_BUSIF`? – Ethan Reesor Feb 27 '18 at 03:37
  • You make a list with ":" like this: "ASSOCIATED_BUSIF s_axi00:s_axi01". – Anders Feb 27 '18 at 07:28