Document safe PLL's

All the examples in the documentation are adjusted to use safe PLL's.

Documents all the changes in this PR

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 `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.
+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/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-prelude/src/Clash/Annotations/TopEntity.hs

@@ -1,7 +1,7 @@
 {-|
 Copyright  :  (C) 2015-2016, University of Twente,
                   2017     , Google Inc.,
-                  2021-2022, QBayLogic B.V.
+                  2021-2023, QBayLogic B.V.
 License    :  BSD2 (see the file LICENSE)
 Maintainer :  QBayLogic B.V. <devops@qbaylogic.com>
 
@@ -62,7 +62,11 @@ module Blinker where
 import Clash.Prelude
 import Clash.Intel.ClockGen
 
-'Clash.Explicit.Signal.createDomain' vSystem{vName=\"DomInput\", vPeriod=20000}
+-- Define a synthesis domain with a clock with a period of 20000 /ps/. Signal
+-- coming from the reset button is low when pressed, and high when not pressed.
+'Clash.Explicit.Signal.createDomain'
+  vSystem{vName=\"DomInput\", vPeriod=20000, vResetPolarity=ActiveLow}
+-- Define a synthesis domain with a clock with a period of 50000 /ps/.
 'Clash.Explicit.Signal.createDomain' vSystem{vName=\"Dom50\", vPeriod=50000}
 
 topEntity
@@ -75,35 +79,19 @@ topEntity clk20 rstBtn enaBtn modeBtn =
   'Clash.Signal.exposeClockResetEnable'
     ('Clash.Prelude.mealy' blinkerT initialStateBlinkerT . 'Clash.Prelude.isRising' 1)
     clk50
-    rstSync
+    rst50
     enaBtn
     modeBtn
  where
   -- Start with the first LED turned on, in rotate mode, with the counter on zero
   initialStateBlinkerT = (1, False, 0)
 
-  -- Signal coming from the reset button is low when pressed, and high when
-  -- not pressed. We convert this signal to the polarity of our domain with
-  -- /unsafeFromActiveLow/.
-  rst = 'Clash.Signal.unsafeFromActiveLow' ('Clash.Signal.unsafeFromReset' rstBtn)
-
-  -- Instantiate a PLL: this stabilizes the incoming clock signal and indicates
-  -- when the signal is stable. We're also using it to transform an incoming
-  -- clock signal running at 20 MHz to a clock signal running at 50 MHz.
-  (clk50, pllStable) =
-    'Clash.Intel.ClockGen.altpll'
-      \@Dom50
-      (SSymbol @"altpll50")
-      clk20
-      rst
-
-  -- Synchronize reset to clock signal coming from PLL. We want the reset to
-  -- remain active while the PLL is NOT stable, hence the conversion with
-  -- /unsafeFromActiveLow/
-  rstSync =
-    'Clash.Prelude.resetSynchronizer'
-      clk50
-      ('Clash.Signal.unsafeFromActiveLow' pllStable)
+  -- Instantiate a PLL: this stabilizes the incoming clock signal and releases
+  -- the reset output when the signal is stable. We're also using it to
+  -- transform an incoming clock signal running at 20 MHz to a clock signal
+  -- running at 50 MHz. Since the signature of topEntity already specifies the
+  -- Dom50 domain, we don't need any type signatures to specify the domain here.
+  (clk50, rst50) = 'Clash.Intel.ClockGen.altpllSync' clk20 rstBtn
 
 blinkerT
   :: (BitVector 8, Bool, Index 16650001)

clash-prelude/src/Clash/Explicit/Reset.hs

@@ -158,18 +158,23 @@ unsafeAndReset (unsafeFromReset -> rst0) (unsafeFromReset -> rst1) =
 -- === __Example 2__
 -- Similar to /Example 1/ this circuit detects a rising bit (i.e., a transition
 -- from 0 to 1) in a given argument. It takes a clock that is not stable yet and
--- a reset singal that is not synchronized to any other signals. It stabalizes
+-- a reset signal that is not synchronized to any other signals. It stabilizes
 -- the clock and then synchronizes the reset signal.
 --
+--
+-- Note that the function 'Clash.Intel.ClockGen.altpllSync' provides this
+-- functionality in a convenient form, obviating the need for
+-- @resetSynchronizer@ for this use case.
+--
 -- @
 -- topEntity
 --   :: Clock  System
 --   -> Reset  System
 --   -> Signal System Bit
 --   -> Signal System (BitVector 8)
 -- topEntity clk rst key1 =
---     let  (pllOut,pllStable) = altpll (SSymbol @"altpll50") clk rst
---          rstSync            = 'resetSynchronizer' pllOut (unsafeToActiveHigh pllStable)
+--     let  (pllOut,pllStable) = unsafeAltpll clk rst
+--          rstSync            = 'resetSynchronizer' pllOut (unsafeFromActiveLow pllStable)
 --     in   exposeClockResetEnable leds pllOut rstSync enableGen
 --   where
 --     key1R  = isRising 1 key1