Merge pull request #2592 from clash-lang/safe_pll

Offer safe PLL's

changelog/2023-03-17T14_40_13+01_00_diff_clk

@@ -1,2 +1 @@
-NEW: `seClockToDiffClock`, to create a differential clock signal in a test
-bench. It is not suitable for synthesising a differential output in hardware.
+NEW: `clockToDiffClock`, to create a differential clock signal in a test bench. It is not suitable for synthesizing a differential output in hardware.

changelog/2023-07-05T16_49_21+02_00_diffclock

@@ -1,9 +1,3 @@
 ADDED: A new clock type `DiffClock` is introduced to signify a differential
 clock signal that is passed to the design on two ports in antiphase. This is
-used by the Xilinx `clockWizardDifferential` IP generator.
-
-CHANGED: Xilinx `clockWizardDifferential` now gets its input clock as a
-`DiffClock` type; use `seClockToDiffClock` to generate this in your test bench
-if needed. Previously, the function received two clock inputs, but this
-generated `create_clock` statements in the top-level SDC file for both phases
-which is incorrect.
+used by the differential Xilinx clock wizards in `Clash.Xilinx.ClockGen`.

changelog/2023-09-11T17_11_44+02_00_safe_pll

@@ -0,0 +1,21 @@
+REMOVED: The module `Clash.Clocks.Deriving` has been removed.
+
+FIXED: `altpll` and `alteraPll` in `Clash.Intel.ClockGen` now account for the input domain's `ResetPolarity`. Before this fix, the reset was always interpreted as an active-high signal.
+
+CHANGED: The IP core generators in `Clash.Intel.ClockGen` now declare that their input domain needs to have asynchronous resets (`HasAsynchronousReset`), as the functions react asynchronously to their reset input and thus need to be glitch-free. The functions marked `unsafe` do not have this constraint; instead, the function documentation calls attention to the requirement.
+
+INTERNAL NEW: `Clash.Primitives.DSL.declareN`, a companion to `declare` which declares multiple signals in one go.
+
+DEPRECATED: The functions `altpll` and `alteraPll` in `Clash.Intel.ClockGen` have been deprecated because they are unsafe to use while this is not apparent from the name. The `locked` output signal of these functions is an asynchronous signal which needs to be synchronized before it can be used (something the examples did in fact demonstrate). For the common use case, new functions are available, named `altpllSync` and `alteraPllSync`. These functions are safe. For advanced use cases, the old functionality can be obtained through `unsafeAltpll` and `unsafeAlteraPll`.
+
+NEW: `altpllSync` and `alteraPllSync` in `Clash.Intel.ClockGen`. These replace the deprecated functions without the `Sync` suffix. Unlike the old functions, these functions are safe to use and have a reset signal for each output domain that can be used to keep the domain in reset while the clock output stabilizes. All PLL functions now also support multiple clock outputs like the old `alteraPll` did.
+
+CHANGED: The wizards in `Clash.Xilinx.ClockGen` have been completely overhauled. The original functions were unsafe and broken in several ways. See the documentation in `Clash.Xilinx.ClockGen` for how to use the new functions. Significant changes are:
+ * `clockWizard` and `clockWizardDifferential` now output a `Clock` and a `Reset` which can be directly used by logic. Previously, it outputted a clock and an asynchronous `locked` signal which first needed to be synchronized before it could be used (hence the old function being unsafe). Additionally, the original `locked` signal was strange: it mistakenly was an `Enable` instead of a `Signal dom Bool` and there was a polarity mismatch between Clash simulation and HDL. The `locked` signal was also not resampled to the output domain in Clash simulation.
+ * There are new functions `unsafeClockWizard` and `unsafeClockWizardDifferential` for advanced use cases which directly expose the `locked` output of the wizard.
+ * All clock generators now have the option to output multiple clocks from a single instance.
+ * `clockWizardDifferential` now gets its input clock as a `DiffClock` type; use `clockToDiffClock` to generate this in your test bench if needed. Previously, the function received two clock inputs, but this generated `create_clock` statements in the top-level SDC file for both phases which is incorrect.
+ * A constraint was removed: The /output/ clock domain no longer requires asynchronous resets. This was originally intended to signal that the outgoing lock signal is an asynchronous signal. The constraint does not convey this  information at all and is wrong; it also prevents using synchronous resets in the circuit as recommended by Xilinx. Note that if you use the `unsafe` functions, it is still necessary to synchronize the `locked` output in your design.
+ * The port names of the primitives in HDL are now correctly lower case.
+ * Add Tcl generation. This moves the responsibility of MMCM component generation from the user to `clashConnector.tcl`, which can be found in [`clash-lib:Clash.DataFiles`](https://hackage.haskell.org/package/clash-lib-1.8.0/docs/Clash-DataFiles.html).
+ * The wizards now use the user-provided name as the name of the /instance/ rather than the name of the /IP core/. This change was also done for `Clash.Intel.ClockGen` in Clash v1.2.0 in March 2020, when Clash started generating Intel Qsys files. Before that, the user needed to generate a Qsys component manually. Now, in Clash v1.8.0, we also generate the Tcl for Xilinx wizards. When the user is responsible for creating the IP core, it makes sense to always set the component name to the user-provided value. But when that is also generated by Clash, that is no longer needed. Allowing users to set the instance name instead makes it possible to match on the instance in SDC files and such.

clash-lib/prims/common/Clash_Intel_ClockGen.primitives.yaml

@@ -1,5 +1,5 @@
 - BlackBox:
-    name: Clash.Intel.ClockGen.altpll
+    name: Clash.Intel.ClockGen.unsafeAltpll
     format: Haskell
     includes:
     - name: altpll
@@ -10,7 +10,7 @@
     templateFunction: Clash.Primitives.Intel.ClockGen.altpllTF
     workInfo: Always
 - BlackBox:
-    name: Clash.Intel.ClockGen.alteraPll
+    name: Clash.Intel.ClockGen.unsafeAlteraPll
     format: Haskell
     includes:
     - name: altera_pll

clash-lib/prims/common/Clash_Xilinx_ClockGen.primitives.yaml

@@ -1,5 +1,16 @@
 - BlackBox:
-    name: Clash.Xilinx.ClockGen.clockWizardDifferential
+    name: Clash.Xilinx.ClockGen.unsafeClockWizard
+    kind: Declaration
+    format: Haskell
+    templateFunction: Clash.Primitives.Xilinx.ClockGen.clockWizardTF
+    includes:
+      - name: clk_wiz
+        extension: clash.tcl
+        format: Haskell
+        templateFunction: Clash.Primitives.Xilinx.ClockGen.clockWizardTclTF
+    workInfo: Always
+- BlackBox:
+    name: Clash.Xilinx.ClockGen.unsafeClockWizardDifferential
     kind: Declaration
     format: Haskell
     templateFunction: Clash.Primitives.Xilinx.ClockGen.clockWizardDifferentialTF

clash-lib/prims/commonverilog/Clash_Explicit_Testbench.primitives.yaml

@@ -6,6 +6,6 @@
     template: assign ~RESULT = 1'b1;
     workInfo: Always
 - BlackBox:
-    name: Clash.Explicit.Testbench.seClockToDiffClock
+    name: Clash.Explicit.Testbench.clockToDiffClock
     kind: Expression
     template: '{~ARG[1], ~ ~ARG[1]}'

clash-lib/prims/vhdl/Clash_Explicit_Testbench.primitives.yaml

@@ -156,6 +156,6 @@
     template: ~RESULT <= true;
     workInfo: Always
 - BlackBox:
-    name: Clash.Explicit.Testbench.seClockToDiffClock
+    name: Clash.Explicit.Testbench.clockToDiffClock
     kind: Expression
     template: (~ARG[1], not ~ARG[1])

clash-lib/src/Clash/DataFiles.hs

@@ -46,9 +46,10 @@ import Paths_clash_lib (getDataFileName)
   > route_design
   > write_bitstream ${clash::topEntity}.bit
 
-  @clash-cores:Clash.Cores.Xilinx@ modules make use of the IP instantiating
-  functionality; XDC metadata functionality is not currently used as the IP is
-  already packaged with correct constraints by Vivado.
+  "Clash.Xilinx.ClockGen" and @clash-cores:Clash.Cores.Xilinx@
+  modules make use of the IP instantiating functionality; XDC metadata
+  functionality is not currently used as the IP is already packaged with correct
+  constraints by Vivado.
 
   More documentation about the Tcl Connector and the Clash\<->Tcl API will be
   made available later.

clash-lib/src/Clash/Driver.hs

@@ -602,6 +602,7 @@ knownTemplateFunctions =
     , ('P.alteraPllTF, P.alteraPllTF)
     , ('P.altpllTF, P.altpllTF)
     , ('P.fromIntegerTFvhdl, P.fromIntegerTFvhdl)
+    , ('P.clockWizardTF, P.clockWizardTF)
     , ('P.clockWizardDifferentialTF, P.clockWizardDifferentialTF)
     , ('P.clockWizardTclTF, P.clockWizardTclTF)
     , ('P.clockWizardDifferentialTclTF, P.clockWizardDifferentialTclTF)

clash-lib/src/Clash/Primitives/DSL.hs

@@ -38,6 +38,7 @@ module Clash.Primitives.DSL
   , declaration
   , declarationReturn
   , declare
+  , declareN
   , instDecl
   , instHO
   , viaAnnotatedSignal
@@ -286,6 +287,25 @@ declare decName ty = do
   uniqueName <- declare' decName ty
   pure (TExpr ty (Identifier uniqueName Nothing))
 
+-- | Declare /n/ new signals with the given type and based on the given name
+declareN
+  :: Backend backend
+  => Text
+  -- ^ Name hint
+  -> [HWType]
+  -- ^ Types of the signals
+  -> State (BlockState backend) [TExpr]
+  -- ^ Expressions pointing the the new signals
+declareN decName tys = do
+  firstName <- Id.makeBasic decName
+  nextNames <- Id.nextN (length tys - 1) firstName
+  let uniqueNames = firstName : nextNames
+  zipWithM
+    (\uniqueName ty -> do
+      addDeclaration $ NetDecl' Nothing uniqueName ty Nothing
+      pure $ TExpr ty (Identifier uniqueName Nothing)
+    ) uniqueNames tys
+
 -- | Assign an expression to an identifier, returns the new typed
 --   identifier expression.
 assign