Xilinx DDR output: Haskell model wrong for `Enable`

[DigitalBrains1]

oddr in Clash.Xilinx.DDR shares the Haskell simulation model with ddrOut in Clash.Explicit.DDR. However, they react differently to the Enable signal, making the model wrong for Xilinx's oddr.

The generic ddrOut and its model will gate the clocks at the registers, but the contents of the registers will keep being outputted on their respective edges. The Xilinx simulation model will freeze the output at the contents of the register holding the data to output on the positive edge.

This can be seen in this Vivado simulation. I deliberately drop the enable asynchronously to the clock to test its asynchronous behaviour. The data inputs to the DDR primitives are just counters, one counting from 100 and up and the other from 200 and up, so you can tell them apart immediately.

What the generic ddrOut does is gate the registers, freezing their respective values at 120 and 220, but still one register is outputted on the active edge (120) and the other on the falling edge (220), hence still outputting a changing signal. Below that is Xilinx's oddr, which gates the register at 120 but then also freezes the output, constantly outputting the 120 until the enable asserts again and both are once again the same, outputting 126 on the rising, active edge and outputting 226 on the falling and incrementing after that.

(Note that they also respond differently to the reset signal being asserted, but since they leave reset in the same fashion I consider it less important than this bug. Frankly, the simulation model in Vivado for the Xilinx primitive is a bit weird in that respect, reacting to reset on the falling edge even though the rising edge is the active edge.)

Although now I suddenly notice it is also reacting to the Enable in the same weird way, on the falling edge! Suddenly the propagation delay for the Enable is reduced to half a clock period instead of a full one. I wonder if the primitive comes with its own timing constraints that actually ensure that it is bounded that way? Weird!

The following is some code you could use to simulate stuff in Vivado. It's what I've been using to muck around with DDR outputs. I wanted testBench to output Reset Slow as well, but I hit a Clash compiler bug... And #2570 (or something like it) is preventing me from stopping the clocks, so you'll just need to stop the simulation in Vivado.

```haskell {-# OPTIONS_GHC -Wno-orphans #-} module DdrTest where import Clash.Annotations.TH import Clash.Explicit.DDR import Clash.Explicit.Prelude import Clash.Intel.DDR import Clash.Xilinx.DDR import Data.Bifunctor createDomain vXilinxSystem{vName="XilinxSysNeg", vActiveEdge=Falling} createDomain vSystem{vName="DdrA", vPeriod=5000} createDomain vXilinxSystem{vName="DdrS", vPeriod=5000} createDomain vXilinxSystem{vName="DdrSN", vActiveEdge=Falling, vPeriod=5000} createDomain vXilinxSystem{vName="Dom4", vPeriod=2500} type Slow = XilinxSystem type Fast = DdrS type D = Unsigned 9 topEntity :: Clock Slow -> Reset Slow -> Enable Slow -> Signal Slow (D, D) -> Signal Fast (D, D) topEntity clk rst en i = bundle (genO, vendorO) where genO = ddrOut clk rst en 0 i vendorO = fmap unpack $ vendorDdr clk rst en $ bimap pack pack <$> i {-# OPAQUE topEntity #-} vendorDdr :: KnownNat n => Clock Slow -> Reset Slow -> Enable Slow -> Signal Slow (BitVector n, BitVector n) -> Signal Fast (BitVector n) vendorDdr = xilinxDdr xilinxDdr :: KnownNat n => Clock Slow -> Reset Slow -> Enable Slow -> Signal Slow (BitVector n, BitVector n) -> Signal Fast (BitVector n) xilinxDdr = oddr intelDdr :: KnownNat n => Clock Slow -> Reset Slow -> Enable Slow -> Signal Slow (BitVector n, BitVector n) -> Signal Fast (BitVector n) intelDdr = altddioOut (SSymbol @"Cyclone IV E") testBench :: ( "clk1" ::: Clock Slow , "clk2" ::: Clock Fast , "en" ::: Enable Slow , "s" ::: Signal Fast ( "cntr" ::: D , "gen" ::: D , "vendor" ::: D ) ) testBench = ( clkSlow , clkFast , enSlow , bundle (cntr, genO, vendorO) ) where cntr = register clkFast noReset enableGen 0 $ cntr + 1 (genO, vendorO) = unbundle $ topEntity clkSlow rstSlow enSlow $ bundle (posD, negD) posD = register clkSlow noReset enableGen 100 $ posD + 1 negD = register clkSlow noReset enableGen 200 $ negD + 1 cntr4 :: Signal Dom4 (Unsigned 11) cntr4 = register clk4 noReset enableGen 0 $ cntr4 + 1 clk4 = clockGen clkFast = clockGen clkSlow = clockGen rstSlow = unsafeFromActiveHigh $ unsafeSynchronizer clk4 clkSlow $ (\n -> n >= 122 && n <= 141) <$> cntr4 -- rstSlow = resetGenN d4 enSlow = toEnable $ unsafeSynchronizer clk4 clkSlow $ (\n -> n < 82 || n > 101) <$> cntr4 {-# OPAQUE testBench #-} makeTopEntity 'testBench testBenchSignal :: Signal Fast (D, D, D) testBenchSignal = s where (_, _, _, s) = testBench ```

[DigitalBrains1]

I should try tomorrow at which clock edge the negative-side register latches its input... maybe they just accidentally apply the OPPOSITE_EDGE simulation model to the SAME_EDGE configuration.

[DigitalBrains1]

No, it reacts to both data inputs on the active, rising edge, so the model is configured as SAME_EDGE. It's just reset and enable that behave oddly.

[DigitalBrains1]

So deasserting the Enable is picked up on the falling edge, but reasserting it is picked up on the rising edge. Same for Reset. Some other time, I should study the timing diagrams in the Xilinx documentation. At a quick glance, I had trouble reading them, and I need to spend my effort elsewhere. That's also why I did this as an issue report instead of just fixing it myself.

Although frankly, I doubt we can make the Haskell simulation model fully accurate, as I suspect our discrete model of time and samples doesn't allow us to react to the incoming Enable or Reset earlier without "peeking into the future". And peeking into the future might cause deadlocks in simulation. I'm really not sure, though, perhaps it can be done. Edit: We can make the Haskell model fully accurate without peeking into the future. I do wonder whether the Vivado model actually corresponds to hardware...