TbUtilPkg: CreateClock Features

[SebDunst]

Hi Jim,
what do you think about adding these features to the CreateClock function?
1.) Add an enable port to be able to start and stop the clock signal
2.) Add the possibility to choose with which level the clock is starting. Now it always starts with '0', which is ok for a single clock, but when starting several related clocks (e.g. 50 MHz and 100 MHz) at the same time, they always share the falling edge.
3.) Add a Log message when starting or stopping the clock.

A possible declaration could be:

procedure CreateClock (
constant AlertLogID : AlertLogIDType;
signal Clk : inout std_logic;
signal ClkEn : in std_logic;
constant Period : time;
constant ClkName : string;
constant DutyCycle : real := 0.5;
constant StartHigh : boolean := false
);

Regards,
Sebastian

[JimLewis]

Sebastian,
It does not sound unreasonable. I will look into it. It would have to be done via overloading as I want to keep basic clocks fast - and hence, no enables to check on every clock cycle.

If it gets too fancy, it may be something that gets added as an entity in the OSVVM/VerificationIP. This is something I am considering for reset as some want to trigger reset while a test is running and observe the operation.

Best Regards,
Jim

[Remillard]

My thought this morning would be an overloaded or wrapper for CreateClock that takes in frequency as a real instead of period. While I know they're interchangeable, anything that makes code a little more self documenting is probably a good thing. I suspect in the meantime I will make a wrapper procedure that does just this.

[JimLewis]

@Remillard That would require frequency to be defined somewhere - either in OSVVM or in VHDL - probably at least in OSVVM TbUtilPkg temporarily

[Remillard]

Yes, my testbenches define a real with the frequency. However as far as overloading it goes, I agree that there's no way for the procedure to know if the real is a period or a frequency especially if we're not doing named associations. So maybe it's best to just keep it as period as that matches the internals the best. My wrapper works fine for my purposes. I suppose a specific real subtype could be created that helps distinguish which overloaded version to use but that might be complicating matters more. If it doesn't work out, that's alright.

[JimLewis]

I was thinking that we need to create a physical type for frequency. OTOH, real may be better, but what are the units? HZ? The period is type time, so the overloading can work. Duty cycle is a real.

[Remillard]

I have been expressing as a real in Hz in my wrapper. That most directly converts to period without any unit shifts. Ultimately we need time units for wait statements. I suppose a physical type could be created, but I imagine there'd have to be some helper functions to translate the physical into real, and then real gets converted to time by * (1 SEC).

The reason I was concerned about overloading is that because the period parameter is real, if you are using positional parameters, how will a tool determine if one real is intended to be a period or if the real is intended to be a frequency which will then give basis for resolving which of the overloaded functions to use. If named association is used, that clears it up simply by having different parameter names, but for a procedure that's relatively simple like this, I have to imagine that positional calls is more desireable. So I'm not entirely sure how to square this circle.

[JimLewis]

Overloading is ok:

procedure CreateClock ( 
    signal   Clk           : inout std_logic ; 
    constant Period        : time ; 
    constant DutyCycle     : real := 0.5 
  )  ; 
procedure CreateClock ( 
    signal   Clk           : inout std_logic ; 
    constant FrequencyInHz : real ; 
    constant DutyCycle     : real := 0.5 
  )  ; 

My concern is accuracy of the clock period. With HZ, we convert to type time - there can be rounding. With type time, the period specified is the actual period of the waveform - rounding effects are factored out by using HIGH_TIME = Period*DutyCycle and LOW_TIME = Period - HIGH_TIME.

[Remillard]

I forgot that CreateClock took period as a time type so I agree that there's no trouble with type resolution if frequency to be delivered as a real.

As for the rounding -- I got nothing. It is a concern though the effect is the same with any period that is not a unit multiple of the simulator resolution. In that case, the engineer is directly in control of what time value is presented to the clock generation procedure so if it's wrong, it's on the engineer to resolve it. Once it's abstracted, the information is somewhat hidden.

The only solution to that I can think of is some diagnostic math at the beginning of the procedure. It would only execute once and display a warning if the module had determined that the time segments were not unit multiple of the simulator resolution -- that said, I don't remember if there's a std variable that tells the model what the current simulator resolution actually is so that may be a no go.

If there's no way to do that, it's absolutely safest to force the engineer to pass in period as time. Again, that way the information is directly generated by the engineer and not obscured.

[JimLewis]

When using period, the period will be accurate. The rounding will only impact duty cycle.

[Paebbels]

I think a complex clock generator should be a OSVVM Verification IP which a transaction record going to the test controller for more control like

• start,
• stop,
• spike injection,
• frequency change

This also allows then like other OSVVM based models to log messages like the proposed clock start/stop.

[amb5l]

When simulating a video design I needed a 100MHz system clock and a 148.5MHz video clock. The video clock period does not round to an exact number of picoseconds so cumulative errors creep in over time. Using the vendor PLL model was expensive in terms of simulation speed so I coded a component that behaves like my FPGA PLL without modelling the VCO etc - it was much quicker.

Doing a similar thing for OSVVM might look like this:

-- create a clock like a PLL; eliminate cumulative rounding errors
procedure CreateClock (
  signal   Clk            : inout std_logic        ; -- generated clock
  constant RefPeriod      :       time             ; -- reference clock period
  constant Multiplier     :       integer          ; -- multiplier
  constant DividerInt     :       integer          ; -- divider integer part
  constant DividerFracTop :       integer   := 0   ; -- divider fraction part numerator
  constant DividerFracBot :       integer   := 1   ; -- divider fraction part denominator
  constant PhaseShift     :       real             ; -- output clock phase shift (degrees)
  constant DutyCycle      :       real      := 0.5   -- output clock duty cycle
) is
  variable M              : integer ; -- simplified multiplier
  variable N              : integer ; -- simplified divider
  variable HCD            : integer ; -- highest common divisor of M and N
  variable RepeatPeriodTS : real    ; -- pattern repeat period in time steps
  variable ClockPeriodTS  : real    ; -- output clock period in time steps
  variable PhaseShiftTS   : real    ; -- phase shift in time steps
  variable PhaseDelay     : time    ; -- phase shift as a time delay
  variable NowTSInt       : integer ; -- now in time steps, within RepCycle
  variable NextTS         : real    ; -- next edge (time steps within repeating sequence)
  variable WaitTSInt      : integer ; -- time to wait until next edge

begin

  -- simplify multiplier/divider
  M := Multiplier * DividerFracBot ;
  N := (DividerInt * DividerFracBot) + DividerFracTop ;
  HCD := 1 ;
  for i in 2 to maximum(M,N) loop
    if M mod i = 0 and N mod i = 0 then
      HCD := i ;
    end if ;
  end loop ;
  M := M / HCD ;
  N := N / HCD ;

  -- calculations
  RepeatPeriodTS := real((N * RefPeriod) / resolution_limit) ;
  ClockPeriodTS  := RepeatPeriodTS / real(M) ;
  PhaseShiftTS   := (PhaseShift * ClockPeriodTS) / 360.0 ;
  PhaseDelay     := PhaseShiftTS * resolution_limit ;

  -- output, pattern repeats every M output (and every N reference) cycles
  Clk <= '0' ;
  wait for PhaseDelay ;
  Clk <= '1' ;
  loop
    NowTSInt := 0 ;
    for i in 0 to M-1 loop
      -- next falling edge
      NextTS := ((real(i) + DutyCycle) * RepeatPeriodTS) / real(M) ;
      WaitTSInt := integer(round(NextTS)) - NowTSInt ;
      wait for WaitTSInt * resolution_limit ;
      NowTSInt := NowTSInt + WaitTSInt ;
      Clk <= '0' ;
      -- next rising edge
      NextTS := (real(i+1) * RepeatPeriodTS) / real(M) ;
      WaitTSInt := integer(round(NextTS)) - NowTSInt ;
      wait for WaitTSInt * resolution_limit ;
      NowTSInt := NowTSInt + WaitTSInt ;
      Clk <= '1' ;
    end loop;
  end loop ;

end procedure CreateClock ;

Minimal testing seems to work. Might it be worth contributing this through proper channels?

[JimLewis]

Definitely open to these. I like clock edges to change on sim cycle 0 of a time step if possible. As a result, for somthing like this I use a pattern of:

Clk <= '0' after LowTime, '1' after TotalTime ; 
wait for TotalTime ; 

[Paebbels]

Using parameter names ...Top and Bot are not good. In mathematics, it's called - as your comments says, numerator and denominator.

As you procedure only drives the clock signal, it should be of mode out.

[amb5l]

Using parameter names ...Top and Bot are not good. In mathematics, it's called - as your comments says, numerator and denominator.

Agreed.

As you procedure only drives the clock signal, it should be of mode out.

I was thinking of reading Clock to work out which edge to output next but I'm going to follow @JimLewis' preference (Clk <= '0' after LowTime, '1' after TotalTime) so I'll change the mode.

Next step is a pull request - dev branch or a dedicated one? And should the procedure have a different name - CreatePLLClock perhaps? The argument in favour of this is that CheckClockPeriod will need to know that its checking a PLL clock, which can jitter by resolution_limit, so would probably need a different procedure name - CheckPLLClockPeriod.

[Paebbels]

I was wondering about 3 ideas/questions:

1. Why not an entity (even with less ports then a PLL, but similar to be a replacement)?
2. It should provide a locked or stable signal, with some delay to when it provides a clock.
   A synchronous reset needs to be derived from locked. A real world application, a system reset will be derived with an N-bit synchronizer into the PLLs output clock frequency domain.
3. What about a enable or reset signal to deactivate the PLLs outputs?

[amb5l]

1. Why not an entity (even with less ports then a PLL, but similar to be a replacement)?

My own PLL style clock generator is an entity. There is much more freedom to model vendor PLL primitive behaviour more closely.

2. It should provide a locked or stable signal, with some delay to when it provides a clock.

Agreed.

A synchronous reset needs to be derived from locked. A real world application, a system reset will be derived with an N-bit synchronizer into the PLLs output clock frequency domain.

Yes. And maybe support a random lock time (within a specified range).

3. What about a enable or reset signal to deactivate the PLLs outputs?

Agreed.

I would also detect instability in the reference clock, to model loss of PLL lock. And model some chaotic behaviour in the clock outputs while the PLL achieves lock, so that DUT behaviour during and coming out of reset can be fully exercised.

@JimLewis how do you feel about a generic PLL style clock generator VIP? Something to look at if we can roll out the procedures discussed in the posts above? Where would it go in the directory tree?

[JimLewis]

@amb5l I think it would go in its own library that has clocks and reset. We can do the procedures too if that makes sense. However, from a use model perspective, if a component declaration is included in a package, the big difference between calling a concurrent procedure vs instancing an component is the word port map.

[amb5l]

Before I create a PR, here's a small test to illustrate a PLL style CreateClock procedure. Any thoughts?
CreateClockTest.zip