soc/cores/led: Review/Rework #1265.

- Split FSM in Main FSM/Xfer FSM to decouple Led data read from bit xfer and do read during xfer.
- Only keep optimization that are easily to understand.
- Default to new WS2812 revision (Since also works on old revision).
- Test 75/50/25MHz sys_clk_freq.

litex/soc/cores/led.py

@@ -1,10 +1,12 @@
 #
 # This file is part of LiteX.
 #
-# Copyright (c) 2020-2021 Florent Kermarrec <florent@enjoy-digital.fr>
+# Copyright (c) 2020-2022 Florent Kermarrec <florent@enjoy-digital.fr>
 # Copyright (c) 2022 Wolfgang Nagele <mail@wnagele.com>
 # SPDX-License-Identifier: BSD-2-Clause
 
+import math
+
 from migen import *
 from migen.genlib.misc import WaitTimer
 
@@ -16,24 +18,6 @@ from litex.soc.interconnect import wishbone
 _CHASER_MODE  = 0
 _CONTROL_MODE = 1
 
-
-# Based on: migen.genlib.misc.WaitTimer
-class ModifiableWaitTimer(Module):
-    def __init__(self, t):
-        self.wait = Signal()
-        self.done = Signal()
-        self.reset = Signal(bits_for(t), reset=t)
-
-        # # #
-
-        count = Signal(bits_for(t), reset=t)
-        self.comb += self.done.eq(count == 0)
-        self.sync += \
-            If(self.wait,
-                If(~self.done, count.eq(count - 1))
-            ).Else(count.eq(self.reset))
-
-
 class LedChaser(Module, AutoCSR):
     def __init__(self, pads, sys_clk_freq, period=1e0):
         self.pads = pads
@@ -74,13 +58,6 @@ class LedChaser(Module, AutoCSR):
 class WS2812(Module):
     """WS2812/NeoPixel Led Driver.
 
-    Hardware Revisions
-    ------------------
-    WS2812 hardware has several different revisions that have been released over the years.
-    Unfortunately not all of them are compatible with the timings they require. Especially 
-    the reset timing has substantial differences between older and newer models.
-    Adjust the hardware_revision to your needs depending on which model you are using.
-
     Description
     -----------
 
@@ -95,7 +72,7 @@ class WS2812(Module):
      - Each Led will "digest" a 24-bit control word:  (MSB) G-R-B (LSB).
      - Leds can be chained through DIN->DOUT connection.
 
-     Each control sequence is separated by a reset code: Line low for > 50us.
+     Each control sequence is separated by a reset code: Line low for > 50us (old) or > 280us (new).
      Ones are transmitted as:
                        ┌─────┐
                        │ T0H │           │  T0H = 400ns +-150ns
@@ -107,6 +84,14 @@ class WS2812(Module):
                        │          │ T1L  │  T1L = 450ns +-150ns
                                   └──────┘
 
+    Hardware Revisions
+    ------------------
+    Different revision of WS2812 have been released over the years. Unfortunately not all of them
+    have compatible timings, especially on reset that has a minimal width of 50us on older models
+    and 280us on newer models. By default, the core will use the timings of the newer models since
+    also working on older models. Reset pulse and refresh rate can be improve for older models by
+    setting revision to "old".
+
     Integration
     -----------
 
@@ -141,12 +126,12 @@ class WS2812(Module):
      sys_clk_freq: int, in
          System Clk Frequency.
     """
-    def __init__(self, pad, nleds, sys_clk_freq, bus_mastering=False, bus_base=None, hardware_revision="old", test_data=None):
+    def __init__(self, pad, nleds, sys_clk_freq, bus_mastering=False, bus_base=None, revision="new", init=None):
         if bus_mastering:
             self.bus  = bus = wishbone.Interface(data_width=32)
         else:
             # Memory.
-            mem = Memory(32, nleds, init=test_data)
+            mem = Memory(32, nleds, init=init)
             port = mem.get_port()
             self.specials += mem, port
 
@@ -160,33 +145,36 @@ class WS2812(Module):
 
 
         # Internal Signals.
-        led_data  = Signal(24)
+        led_count  = Signal(max=nleds)
-        bit_count = Signal(8)
+        led_data   = Signal(24)
-        led_count = Signal(max = nleds + 1)
+        xfer_start = Signal()
+        xfer_done  = Signal()
+        xfer_data  = Signal(24)
 
         # Timings
-        self.trst = trst = 285e-6 if hardware_revision == "new" else 55e-6
+        self.trst = trst = {"old": 50e-6*1.25, "new": 280e-6*1.25}[revision]
-        self.t0h = t0h = 0.40e-6
+        self.t0h  = t0h  = 0.40e-6
-        self.t1h = t1h = 0.80e-6
+        self.t0l  = t0l  = 0.85e-6
-        self.t0l = t0l = 0.85e-6
+        self.t1h  = t1h  = 0.80e-6
-        self.t1l = t1l = 0.45e-6
+        self.t1l  = t1l  = 0.45e-6
 
         # Timers.
-        t0h_timer = ModifiableWaitTimer(int(t0h*sys_clk_freq))
+        trst_timer = WaitTimer(int(trst*sys_clk_freq))
-        t0l_timer = ModifiableWaitTimer(int(t0l*sys_clk_freq) - 1) # compensate for data clk in cycle
-        self.submodules += t0h_timer, t0l_timer
-
-        t1h_timer = ModifiableWaitTimer(int(t1h*sys_clk_freq))
-        t1l_timer = ModifiableWaitTimer(int(t1l*sys_clk_freq) - 1) # compensate for data clk in cycle
-        self.submodules += t1h_timer, t1l_timer
-
-        trst_timer = ModifiableWaitTimer(int(trst*sys_clk_freq))
         self.submodules += trst_timer
 
-        # FSM
+        t0h_timer = WaitTimer(int(t0h*sys_clk_freq))
+        t0l_timer = WaitTimer(int(t0l*sys_clk_freq) - 1) # Compensate Xfer FSM latency.
+        self.submodules += t0h_timer, t0l_timer
+
+        t1h_timer = WaitTimer(int(t1h*sys_clk_freq))
+        t1l_timer = WaitTimer(int(t1l*sys_clk_freq) - 1) # Compensate Xfer FSM latency.
+        self.submodules += t1h_timer, t1l_timer
+
+        # Main FSM.
         self.submodules.fsm = fsm = FSM(reset_state="RST")
         fsm.act("RST",
-            trst_timer.wait.eq(1),
+            NextValue(led_count, 0),
+            trst_timer.wait.eq(xfer_done),
             If(trst_timer.done,
                 NextState("LED-READ")
             )
@@ -199,69 +187,74 @@ class WS2812(Module):
                 bus.sel.eq(2**(bus.data_width//8)-1),
                 bus.adr.eq(bus_base[2:] + led_count),
                 If(bus.ack,
-                    NextValue(bit_count, 24-1),
                     NextValue(led_data, bus.dat_r),
-                    NextState("BIT-TEST")
+                    NextState("LED-SEND")
                 )
             )
         else:
             self.comb += port.adr.eq(led_count)
             fsm.act("LED-READ",
-                NextValue(bit_count, 24-1),
+                NextState("LED-LATCH")
-                NextValue(led_count, led_count + 1),
+            )
+            fsm.act("LED-LATCH",
                 NextValue(led_data, port.dat_r),
-                NextState("BIT-TEST")
+                NextState("LED-SEND")
             )
 
-        fsm.act("BIT-TEST",
+        fsm.act("LED-SEND",
-            # by including the cycles spent on checking for conditions
+            If(xfer_done,
-            # data shifting, etc. we make the timing more precise
+                xfer_start.eq(1),
-            If(bit_count == 0,
-                # BIT-SHIFT + LED-SHIFT + LED-READ + BIT-TEST
-                NextValue(t0l_timer.reset, t0l_timer.reset.reset - 4),
-                NextValue(t1l_timer.reset, t1l_timer.reset.reset - 4)
-            ).Else(
-                # BIT-SHIFT + BIT-TEST
-                NextValue(t0l_timer.reset, t0l_timer.reset.reset - 2),
-                NextValue(t1l_timer.reset, t1l_timer.reset.reset - 2)
-            ),
-            If(led_data[-1] == 0,
-                NextState("ZERO-SEND"),
-            ),
-            If(led_data[-1] == 1,
-                NextState("ONE-SEND"),
-            ),
-        )
-        fsm.act("ZERO-SEND",
-            t0h_timer.wait.eq(1),
-            t0l_timer.wait.eq(t0h_timer.done),
-            pad.eq(~t0h_timer.done),
-            If(t0l_timer.done,
-                NextState("BIT-SHIFT")
-            )
-        )
-        fsm.act("ONE-SEND",
-            t1h_timer.wait.eq(1),
-            t1l_timer.wait.eq(t1h_timer.done),
-            pad.eq(~t1h_timer.done),
-            If(t1l_timer.done,
-                NextState("BIT-SHIFT")
-            )
-        )
-        fsm.act("BIT-SHIFT",
-            NextValue(led_data, Cat(Signal(), led_data)),
-            If(bit_count == 0,
                 NextState("LED-SHIFT")
-            ).Else(
-                NextValue(bit_count, bit_count - 1),
-                NextState("BIT-TEST")
             )
         )
         fsm.act("LED-SHIFT",
-            If(led_count == nleds,
+            If(led_count == (nleds - 1),
-                NextValue(led_count, 0),
                 NextState("RST")
             ).Else(
+                NextValue(led_count, led_count + 1),
                 NextState("LED-READ")
             )
         )
+
+        # XFER FSM.
+        xfer_bit = Signal(5)
+        xfer_fsm = FSM(reset_state="IDLE")
+        self.submodules += xfer_fsm
+        xfer_fsm.act("IDLE",
+            xfer_done.eq(1),
+            If(xfer_start,
+                NextValue(xfer_bit, 24-1),
+                NextValue(xfer_data, led_data),
+                NextState("RUN")
+            )
+        )
+        xfer_fsm.act("RUN",
+            # Send a one.
+            If(xfer_data[-1],
+                t1h_timer.wait.eq(1),
+                t1l_timer.wait.eq(t1h_timer.done),
+                pad.eq(~t1h_timer.done),
+            # Send a zero.
+            ).Else(
+                t0h_timer.wait.eq(1),
+                t0l_timer.wait.eq(t0h_timer.done),
+                pad.eq(~t0h_timer.done),
+            ),
+
+            # When bit has been sent:
+            If(t0l_timer.done | t1l_timer.done,
+                # Clear wait on timers.
+                t0h_timer.wait.eq(0),
+                t0l_timer.wait.eq(0),
+                t1h_timer.wait.eq(0),
+                t1l_timer.wait.eq(0),
+                # Shift xfer_data.
+                NextValue(xfer_data, Cat(Signal(), xfer_data)),
+                # Decrement xfer_bit.
+                NextValue(xfer_bit, xfer_bit - 1),
+                # When xfer_bit reaches 0.
+                If(xfer_bit == 0,
+                    NextState("IDLE")
+                )
+            )
+        )

test/test_led.py

@@ -11,17 +11,17 @@ from migen import *
 from litex.soc.cores.led import WS2812
 
 
-TEST_CLK_FREQS = (20e6, 16e6, 15e6, 10e6)
-
-
 class TestWS2812(unittest.TestCase):
-    def generator(self, dut, led_signal, led_data, sys_clk_freq, iterations):
+    test_clk_freqs = [75e6, 50e6, 25e6]
-        error_margin = 0.15e-6 # defined in datasheet
 
-        # cap on how long a sequence will be evaluated
+    def generator(self, dut, led_signal, led_data, sys_clk_freq, iterations):
+        # Error Margin from WS2812 datasheet.
+        error_margin = 150e-9
+
+        # Cap on how long a sequence will be evaluated.
         max_cycles_per_seq = int(dut.trst * sys_clk_freq * 2)
 
-        # initial reset
+        # Verify initial reset.
         rst_cycles = 0
         for _ in range(max_cycles_per_seq):
             if (yield led_signal) != 0:
@@ -31,16 +31,21 @@ class TestWS2812(unittest.TestCase):
         rst_time = rst_cycles / sys_clk_freq
         assert rst_time >= dut.trst
 
+        # Verify generated data pulses.
         length = len(led_data)
         for _ in range(iterations):
-            for i_num, num in enumerate(led_data, start = 1):
+            for i_num, num in enumerate(led_data, start=1):
-                for idx_bit, bit in enumerate(TestWS2812.to_bits(num), start = 1):
+                for idx_bit, bit in enumerate(TestWS2812.to_bits(num), start=1):
-                    exp_high, exp_low = (dut.t0h, dut.t0l) if bit == 0 else (dut.t1h, dut.t1l)
+                    exp_high, exp_low = {
+                        0 : (dut.t0h, dut.t0l),
+                        1 : (dut.t1h, dut.t1l)
+                    }[bit]
 
-                    # end of chain reset
+                    # On end of chain, add reset time to exp_low
                     if i_num == length and idx_bit == 24:
                         exp_low += dut.trst
 
+                    # Verify high cycle.
                     high_cycles = 0
                     for _ in range(max_cycles_per_seq):
                         if (yield led_signal) != 1:
@@ -51,6 +56,7 @@ class TestWS2812(unittest.TestCase):
                     assert high_time >= exp_high - error_margin
                     assert high_time <= exp_high + error_margin
 
+                    # Verify low cycle.
                     low_cycles = 0
                     for _ in range(max_cycles_per_seq):
                         if (yield led_signal) != 0:
@@ -61,23 +67,21 @@ class TestWS2812(unittest.TestCase):
                     assert low_time >= exp_low - error_margin
                     assert low_time <= exp_low + error_margin
 
-
     def to_bits(num, length = 24):
         return ( int(x) for x in bin(num)[2:].zfill(length) )
 
 
-    def run_test(self, hardware_revision, sys_clk_freq):
+    def run_test(self, revision, sys_clk_freq):
         led_signal = Signal()
-        led_data = [ 0x100000, 0x200000, 0x300000, 0x400000, 0x500000, 0x600000, 0x700000, 0x800000, 0x900000 ]
+        led_data   = [0x100000, 0x200000, 0x300000, 0x400000, 0x500000, 0x600000, 0x700000, 0x800000, 0x900000]
-        iterations = 3
+        iterations = 2
-        dut = WS2812(led_signal, len(led_data), sys_clk_freq, hardware_revision = hardware_revision, test_data = led_data)
+        dut = WS2812(led_signal, len(led_data), sys_clk_freq, revision=revision, init=led_data)
-        run_simulation(dut, self.generator(dut, led_signal, led_data, sys_clk_freq, iterations))
+        run_simulation(dut, self.generator(dut, led_signal, led_data, sys_clk_freq, iterations), vcd_name="sim.vcd")
-
 
     def test_WS2812_old(self):
-        for sys_clk_freq in TEST_CLK_FREQS:
+        for sys_clk_freq in self.test_clk_freqs:
             self.run_test("old", sys_clk_freq)
 
     def test_WS2812_new(self):
-        for sys_clk_freq in TEST_CLK_FREQS:
+        for sys_clk_freq in self.test_clk_freqs:
             self.run_test("new", sys_clk_freq)