Problems with 'tperiod_Clk'

[Paebbels]

Thanks for yesterdays live debugging session.
We found our mistake in the usage of Axi4Manager and Axi4Memory.

The frequency of the OSVVM verification components was not matching the period of our testbench clocks.
We have 2 suggestions to prevent others from doing the same mistake:

1. Don't set a default value to tperiod_Clk, so users are forced to map a clock period.
2. Measure the clock period in the model.
   => We would prefer this method.

---

How can the latter be implemented in a save and reliable way?

1. Measure the clock period on Clk and save it in a local variable/signal.
2. When reset is done (rising_edge(nReset), assign tperiod_Clk.

What does this method achieve?

• No need for a generic / no dependency to the parent instance.
• Clock period can be changed in the testbench, while reset is high or retriggered.
  • Anyhow, clock must be stable before reset is released and it can't change frequency afterwards unless reset is asserted again.
• When reset is deasserted, tperiod_Clk can be used immediately.

Sketch of the solution:

architecture a of e is
  signal tperiod_Clk  : time := -1 ns;
begin
  process(Clk, nReset)
    variable LastRise : time :=  0 ns;
    variable Period   : time := -1 ns;
  begin
    if rising_edge(nReset) then
      tperiod_Clk <= Period;
    end if;

    if rising_edge(Clk) then
      Period   := now - LastRise;
      LastRise := now;
    end if;
  end process;
  
  -- ...
end architecture;

Note:

• negative times are intended, so simulation fails on uninitialized / unmeasured clocks.

---

/cc @stefanunrein

[riedel-ferringer]

I also think getting rid of the tperiod_Clk generic would be favorable. I have some alternative suggestions.

Looking at DoAxiReadyHandshake,

AXI4/common/src/Axi4CommonPkg.vhd

Line 136 in 3e52ab8

Ready <= transport '1' after ReadyDelayCycles + tpd_Clk_Ready ;

we could change the code to something like

if ReadyBeforeValid then
  WaitForClock(Clk, ReceiveReadyDelayCycles);
  Ready <= transport '1' after tpd_Clk_Ready ;
end if ;  

However, I am not sure if it is OK to make procedure DoAxiReadyHandshake explicitly wait here. It's probably OK because without a Ready signal nothing else happens on the bus anyway?

Or maybe something like this

  Ready <= transport '1' after Clk'last_event * 2 + tpd_Clk_Ready;

assuming Clk has a 50% duty cycle. Admittedly, I don't know if the second version would actually work, I have never used this attribute before.
Alternatively, the clock-period could also be provided by means of SetAxiStreamOptions(...).

What's the reason for this proposed change?
We have a DUT which has an AXI Stream as output. However, the DUT interfaces a high-speed serial link which operates on three different speeds (using auto-negotiation). Depending on the speed, also the AXI output's clock frequency changes during operation. Using a generic for providing the clock-period thus is not working in such a scenario.

I can prepare a PR in case you agree to one of the proposals.

[JimLewis]

@riedel-ferringer
I will take care of it. This sort of update is probably needed in a couple of places in the VC also.