clash-cores: Fix xpmCdcHandshake (#2610)

* The HDL generated incorrectly set DEST_EXT_HSK=0,
   configuring it to generate acks automatically.
   While the exposed API and simulation model assumed external handshaking.
 * The dstStages and srcStages settings were flipped.

This also updates the test so it can detect that first error.
And improves the haddock a bit to clarify what the settings do and relate them to the XPM documentation.

Author: leonschoorl

clash-cores/src/Clash/Cores/Xilinx/Xpm/Cdc/Handshake.hs

@@ -1,5 +1,6 @@
 {-|
-  Copyright   :  (C) 2023, Google LLC
+  Copyright   :  (C) 2023, Google LLC,
+                     2023, QBayLogic B.V.
   License     :  BSD2 (see the file LICENSE)
   Maintainer  :  QBayLogic B.V. <[email protected]>
 -}
@@ -81,11 +82,28 @@ xpmCdcHandshake = xpmCdcHandshakeWith XpmCdcHandshakeConfig{..}
 {-# INLINE xpmCdcHandshake #-}
 
 -- | Configuration for 'xpmCdcHandshakeWith'
+--
+-- Other attributes that are hardcoded:
+--
+-- +------------------+-------+
+-- | Attribute        | Value |
+-- +==================+=======+
+-- | @DEST_EXT_HSK@   |     1 |
+-- +------------------+-------+
+-- | @SIM_ASSERT_CHK@ |     0 |
+-- +------------------+-------+
 data XpmCdcHandshakeConfig srcStages dstStages = XpmCdcHandshakeConfig
-  { -- | Number of synchronization stages in the source domain
+  { -- | Number of registers, clocked by the src clock, that are used to synchronize @dest_ack@ to @src_rcv@.
+    --
+    -- This is what [PG382](https://docs.xilinx.com/r/en-US/pg382-xpm-cdc-generator/XPM_CDC_HANDSHAKE)
+    -- calls @SRC_SYNC_FF@.
     srcStages :: SNat srcStages
 
-    -- | Number of synchronization stages in the destination domain
+    -- | Number of registers, clocked by the dst clock,
+    -- that are used to synchronize between the input register of @src_send@ and the output register of @dest_req@.
+    --
+    -- This is what [PG382](https://docs.xilinx.com/r/en-US/pg382-xpm-cdc-generator/XPM_CDC_HANDSHAKE)
+    -- calls @DEST_SYNC_FF@.
   , dstStages :: SNat dstStages
 
     -- | Initialize registers used within the primitive to /0/. Note that
@@ -95,6 +113,9 @@ data XpmCdcHandshakeConfig srcStages dstStages = XpmCdcHandshakeConfig
     --
     -- This value is ignored in Clash simulation on domains configured to not
     -- support initial values.
+    --
+    -- This is what [PG382](https://docs.xilinx.com/r/en-US/pg382-xpm-cdc-generator/XPM_CDC_HANDSHAKE)
+    -- calls @INIT_SYNC_FF@.
   , initialValues :: Bool
   }
 

clash-cores/src/Clash/Cores/Xilinx/Xpm/Cdc/Handshake/Internal.hs

@@ -1,5 +1,6 @@
 {-|
-  Copyright   :  (C) 2023, Google LLC
+  Copyright   :  (C) 2023, Google LLC,
+                     2023, QBayLogic B.V.
   License     :  BSD2 (see the file LICENSE)
   Maintainer  :  QBayLogic B.V. <[email protected]>
 -}
@@ -111,11 +112,11 @@ xpmCdcHandshakeTF# bbCtx
       let
         generics :: [(Text, DSL.LitHDL)]
         generics =
-          [ ("DEST_EXT_HSK", DSL.I 0)
-          , ("DEST_SYNC_FF", DSL.I srcStages0)
+          [ ("DEST_EXT_HSK", DSL.I 1)
+          , ("DEST_SYNC_FF", DSL.I dstStages0)
           , ("INIT_SYNC_FF", if initValues0 then 1 else 0)
           , ("SIM_ASSERT_CHK", 0)
-          , ("SRC_SYNC_FF", DSL.I dstStages0)
+          , ("SRC_SYNC_FF", DSL.I srcStages0)
           , ("WIDTH", DSL.I width)
           ]
 
@@ -208,8 +209,8 @@ xpmCdcHandshake# initVals srcStages dstStages clkSrc clkDst srcIn srcSend dstAck
     , initialValues = initVals
     , registerInput = False }
 
-  srcSendFfSynced = xpmCdcSingleWith (defOpts srcStages) clkSrc clkDst srcSendFf
-  srcRcv = xpmCdcSingleWith (defOpts dstStages) clkDst clkSrc dstAck
+  srcSendFfSynced = xpmCdcSingleWith (defOpts dstStages) clkSrc clkDst srcSendFf
+  srcRcv = xpmCdcSingleWith (defOpts srcStages) clkDst clkSrc dstAck
 
   srcSendFf = delay clkSrc enableGen (initVal False) srcSend
   srcHsDataFf = delay clkSrc (toEnable (not <$> srcSendFf)) (initVal (unpack 0)) srcIn

tests/shouldwork/Cores/Xilinx/XpmCdcHandshakeTypes.hs

@@ -15,7 +15,7 @@ createDomain vXilinxSystem{vName="D5",  vPeriod=hzToPeriod 50e6}
 createDomain vXilinxSystem{vName="D10", vPeriod=hzToPeriod 100e6}
 createDomain vXilinxSystem{vName="D11", vPeriod=hzToPeriod 110e6}
 
-data State = WaitForDeassert | WaitForAssert deriving (Generic, NFDataX)
+data State = WaitForDeassert | WaitForAssert (Index 2) deriving (Generic, NFDataX)
 
 -- | Transfer 1, 2, 3, ... to destination domain
 srcFsm ::
@@ -29,10 +29,11 @@ srcFsm ::
   Signal src (a, Bool)
 srcFsm clk = mealy clk noReset enableGen go (0, WaitForDeassert)
  where
-  go (n, WaitForDeassert) True  = ((n,     WaitForDeassert), (n,     False))
-  go (n, WaitForDeassert) False = ((n + 1, WaitForAssert),   (n + 1, True))
-  go (n, WaitForAssert)   False = ((n,     WaitForAssert),   (n,     True))
-  go (n, WaitForAssert)   True  = ((n,     WaitForDeassert), (n,     False))
+  go (n, WaitForDeassert) True  = ((n,     WaitForDeassert),         (0,     False))
+  go (n, WaitForDeassert) False = ((n + 1, WaitForAssert maxBound),  (n + 1, True))
+  go (n, WaitForAssert _) False = ((n,     WaitForAssert maxBound),  (n,     True))
+  go (n, WaitForAssert 0) True  = ((n,     WaitForDeassert),         (0,     False))
+  go (n, WaitForAssert w) True  = ((n,     WaitForAssert (w-1)),     (n,     True))  -- seen src_rcv, wait a little before dropping src_send
 {-# NOINLINE srcFsm #-}
 
 -- | Receives data from source domain
@@ -42,12 +43,13 @@ dstFsm ::
   Clock dst ->
   Signal dst (Bool, a) ->
   Signal dst (Bool, Maybe a)
-dstFsm clk = mealy clk noReset enableGen go WaitForAssert
+dstFsm clk = mealy clk noReset enableGen go (WaitForAssert maxBound)
  where
-  go WaitForAssert   (False, _) = (WaitForAssert,   (False, Nothing))
-  go WaitForAssert   (True,  n) = (WaitForDeassert, (True,  Just n))
-  go WaitForDeassert (True,  _) = (WaitForDeassert, (True,  Nothing))
-  go WaitForDeassert (False, _) = (WaitForAssert,   (False, Nothing))
+  go (WaitForAssert _)  (False, _) = (WaitForAssert maxBound,   (False, Nothing))
+  go (WaitForAssert 0)  (True,  n) = (WaitForDeassert,          (True,  Just n))
+  go (WaitForAssert w)  (True,  n) = (WaitForAssert (w-1),      (False, Nothing))  -- seen dest_req, wait a little before asserting dest_ack
+  go WaitForDeassert    (True,  _) = (WaitForDeassert,          (True,  Nothing))
+  go WaitForDeassert    (False, _) = (WaitForAssert maxBound,   (False, Nothing))
 {-# NOINLINE dstFsm #-}
 
 -- | Composition of 'srcFsm' and 'dstFsm'