From 25cf29808d9ad78613bf0b9ddc9658eb77ac23c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Montiel=20Cardona?= Date: Fri, 17 Jul 2026 03:14:36 -0600 Subject: [PATCH] Drive SysTick off the core cycle counter, not the peripheral tick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit machine.bitstream() produced degenerate pulses: every high pulse lasted exactly the same time for a bit 0 and a bit 1, so a WS2812 frame left the chip carrying no information and neopixel always painted (0,0,0). The rp2 port times bitstream against SysTick. For each bit it writes CVR, raises the pin, and busy-waits on `(start_ticks - systick_hw->cvr) < t[0]` before lowering it (ports/rp2/machine_bitstream.c, PICO_ARM path). SysTick was only advanced from ITickable.Tick, which RP2040Machine.Run calls once per batch of instructions — 500 K in the test harness. Inside a batch CVR was therefore frozen, so the very first comparison already read a distance that cleared both t[0] and t[1] and each wait fell straight through. Both delays collapsed to the same handful of instructions. SysTick counts off the core clock, so its value is a pure function of the owning core's cycle counter and must be derived from it on demand rather than integrated at the tick quantum. Anchor CVR to CortexM0Plus.Cycles and resolve it lazily on every SysTick register access; Tick() no longer needs the shared delta the router passes, which was also wrong for core 1 (each core's private SysTick advances with its own cycles). The CVR write path now seeds the reload instead of parking the counter at zero: ARMv6-M says a CVR write must not raise a SysTick exception, but a counter left at zero rolls over on the next cycle. Only observable now that the counter resolves per-instruction. Pulse-width histogram for `np.write()` over 3 pixels on GP2 (72 bits, of which 48 zeros and 24 ones), MicroPython 1.28.0: before 15:72x (one cluster — bits unrecoverable) after 51:48x, 99:24x (two clusters, ratio 1.94 ~ T1H/T0H) The new integration test boots real MicroPython, runs the neopixel script and decodes the captured widths back into the GRB bytes the firmware wrote. --- .../Peripherals/Ppb/PpbPeripheral.cs | 42 +++++-- .../Tests/MicroPythonBitstreamTests.cs | 113 ++++++++++++++++++ 2 files changed, 145 insertions(+), 10 deletions(-) create mode 100644 tests/RP2040Sharp.IntegrationTests/Tests/MicroPythonBitstreamTests.cs diff --git a/src/RP2040Sharp/Peripherals/Ppb/PpbPeripheral.cs b/src/RP2040Sharp/Peripherals/Ppb/PpbPeripheral.cs index 6fb50cb..62d0c6d 100644 --- a/src/RP2040Sharp/Peripherals/Ppb/PpbPeripheral.cs +++ b/src/RP2040Sharp/Peripherals/Ppb/PpbPeripheral.cs @@ -44,7 +44,8 @@ public sealed class PpbPeripheral : IMemoryMappedDevice, ITickable // SysTick state private uint _systCsr; private uint _systRvr; - private long _systCvr; // kept as long to handle large delta gracefully + private long _systCvr; // kept as long to handle large delta gracefully + private long _systAnchor; // _cpu.Cycles at which _systCvr was last brought up to date // NVIC priority registers — 8 × uint → 32 IRQs, 2 priority bits each (bits 7:6) private readonly uint[] _nvicIpr = new uint[8]; @@ -58,20 +59,32 @@ public PpbPeripheral(CortexM0Plus cpu) // ── ITickable ──────────────────────────────────────────────────── - /// Advance SysTick by cycles. - public void Tick(long deltaCycles) + /// Bring SysTick up to date with the owning core's cycle counter, firing COUNTFLAG and + /// the SysTick exception for any reload boundaries crossed since the last sync. + /// SysTick counts off the core clock, so it must be sampled against + /// and never against the peripheral tick quantum: firmware busy-waits on CVR + /// (machine.bitstream times WS2812 bit widths this way), and a CVR that only moves on the + /// tick boundary makes every such wait expire at the same instant. + /// is ignored — the router ticks both cores' PPBs with a shared + /// delta, but each core's SysTick advances with that core's own cycles. + public void Tick(long deltaCycles) => SyncSysTick(); + + private long SystReload => _systRvr > 0 ? _systRvr : 0xFFFFFF; + + private void SyncSysTick() { - if ((_systCsr & 1) == 0) return; // SysTick not enabled + var now = _cpu.Cycles; + var delta = now - _systAnchor; + _systAnchor = now; + + if ((_systCsr & 1) == 0 || delta <= 0) return; // SysTick not enabled, or nothing elapsed - _systCvr -= deltaCycles; + _systCvr -= delta; - // Handle one or more rollovers (usually 0 or 1 per Tick call) while (_systCvr <= 0) { _systCsr |= 1u << 16; // COUNTFLAG - - long reload = _systRvr > 0 ? (long)_systRvr : 0xFFFFFF; - _systCvr += reload; + _systCvr += SystReload; if ((_systCsr & 2) != 0) // TICKINT _cpu.TriggerSysTick(); @@ -87,6 +100,9 @@ public uint ReadWord(uint address) if (offset >= NVIC_IPR0 && offset <= NVIC_IPR7) return _nvicIpr[(offset - NVIC_IPR0) >> 2]; + if (offset is SYST_CSR or SYST_CVR) + SyncSysTick(); + return offset switch { SYST_CSR => _systCsr, @@ -130,15 +146,21 @@ public void WriteWord(uint address, uint value) switch (offset) { case SYST_CSR: + SyncSysTick(); _systCsr = value & 0x7; // ENABLE | TICKINT | CLKSOURCE break; case SYST_RVR: + SyncSysTick(); _systRvr = value & 0x00FFFFFF; break; case SYST_CVR: - _systCvr = 0; + SyncSysTick(); + // ARMv6-M: a CVR write clears the counter and COUNTFLAG and must not raise a SysTick + // exception. Parking at 0 would instead roll over on the very next cycle, so seed the + // reload the hardware performs on the following clock. + _systCvr = SystReload; _systCsr &= ~(1u << 16); // clear COUNTFLAG break; diff --git a/tests/RP2040Sharp.IntegrationTests/Tests/MicroPythonBitstreamTests.cs b/tests/RP2040Sharp.IntegrationTests/Tests/MicroPythonBitstreamTests.cs new file mode 100644 index 0000000..8cbf6ac --- /dev/null +++ b/tests/RP2040Sharp.IntegrationTests/Tests/MicroPythonBitstreamTests.cs @@ -0,0 +1,113 @@ +using FluentAssertions; +using RP2040Sharp.IntegrationTests.Infrastructure; + +namespace RP2040Sharp.IntegrationTests.Tests; + +/// +/// Regression tests for machine.bitstream (the transport under the neopixel module). +/// The rp2 port encodes each bit as a high-pulse width and busy-waits on SysTick's CVR between the +/// pin_high and pin_low writes, so a SysTick that only advances on the peripheral tick boundary +/// collapses both delays to zero and every bit leaves the chip with the same width. +/// +[Trait("Category", "Integration")] +public sealed class MicroPythonBitstreamTests +{ + private static bool ShouldSkip => + Environment.GetEnvironmentVariable("SKIP_INTEGRATION_TESTS") == "1"; + + private const string Version = "v1.21.0"; + private const int DataPin = 2; + + /// + /// Drives a 3-pixel WS2812 frame whose GRB bytes are a known mix of 0x00 and 0xFF, so the + /// wire carries 24 one-bits and 48 zero-bits, and asserts that the captured high-pulse widths + /// fall into two separated clusters with a ~2x ratio (WS2812 T0H 0.4us vs T1H 0.8us). + /// + [Fact] + public async Task Bitstream_ZeroAndOneBits_ProduceDistinguishableHighPulses() + { + if (ShouldSkip) return; + + await using var runner = await MicroPythonRunner.CreateAsync(Version); + if (runner is null) return; + + runner.WaitForPrompt().Should().BeTrue("MicroPython must reach the REPL"); + + runner.ExecuteAndWait("import machine, neopixel", ">>> ").Should().BeTrue(); + runner.ExecuteAndWait($"np = neopixel.NeoPixel(machine.Pin({DataPin}), 3)", ">>> ").Should().BeTrue(); + runner.ExecuteAndWait("np[0] = (255, 0, 0); np[1] = (0, 255, 0); np[2] = (0, 0, 255)", ">>> ") + .Should().BeTrue(); + + var widths = CaptureHighPulseWidths(runner); + + widths.Count.Should().BeGreaterThanOrEqualTo(72, + "a 3-pixel frame carries 72 bits, each one a high pulse"); + + var shortest = widths.Min(); + var longest = widths.Max(); + + longest.Should().BeGreaterThan(shortest, + "a zero-bit and a one-bit must not leave the pin with the same high time"); + + var ratio = (double)longest / shortest; + ratio.Should().BeInRange(1.5, 3.0, + "WS2812 encodes the bit as T1H/T0H ~ 2x; got {0} vs {1}", longest, shortest); + + // Every pulse must belong to one of the two clusters — a spread continuum would mean the + // widths are noise rather than an encoding. + var midpoint = (shortest + longest) / 2.0; + var zeros = widths.Where(w => w < midpoint).ToList(); + var ones = widths.Where(w => w >= midpoint).ToList(); + + (zeros.Max() - zeros.Min()).Should().BeLessThan((long)((longest - shortest) / 4.0), + "the zero-bit pulses must form a tight cluster"); + (ones.Max() - ones.Min()).Should().BeLessThan((long)((longest - shortest) / 4.0), + "the one-bit pulses must form a tight cluster"); + + // The point of the encoding is that a downstream LED can recover the payload: threshold the + // widths at the midpoint and the frame must decode back to the GRB bytes that were written. + var decoded = new byte[9]; + for (var bit = 0; bit < 72; bit++) + if (widths[bit] >= midpoint) + decoded[bit / 8] |= (byte)(0x80 >> (bit % 8)); + + decoded.Should().Equal(new byte[] + { + 0x00, 0xFF, 0x00, // pixel 0 = (255, 0, 0) sent G, R, B + 0xFF, 0x00, 0x00, // pixel 1 = (0, 255, 0) + 0x00, 0x00, 0xFF, // pixel 2 = (0, 0, 255) + }, "the pulse widths must carry the frame the firmware wrote"); + } + + /// + /// Runs np.write() while recording, for every SIO write that flips the data pin, the + /// core-0 cycle count, and returns the width of each high pulse. + /// + private static List CaptureHighPulseWidths(MicroPythonRunner runner) + { + var machine = runner.Simulation.Rp2040; + var sio = machine.Sio; + var cpu = machine.Cpu; + + var widths = new List(); + var level = sio.GetGpioOut(DataPin); + long? roseAt = null; + + sio.OnGpioChanged = () => + { + var now = sio.GetGpioOut(DataPin); + if (now == level) return; + level = now; + if (now) + roseAt = cpu.Cycles; + else if (roseAt is { } start) + widths.Add(cpu.Cycles - start); + }; + + runner.Execute("np.write()"); + runner.Simulation.RunMilliseconds(200); + sio.OnGpioChanged = null; + + return widths; + } +}