Lock WriteC64Spans slices to a sample store, and count the stores

Source/Teensy/MinimalBoot/Common/C64Spans.h

@@ -11,6 +11,16 @@ enum : uint32_t { C64SpansMax = 64, C64SlicesMax = 1024 };
 
 enum class C64SpansCheck : uint8_t { OK, BadCount, EmptySpan, Wraps, HitsIO, TooBig, TooManySlices };
 
+// WriteC64Spans header flags.  LockToStore starts each slice on the cycle after the C64 writes
+// StoreAddr, which the first span-list entry names in place of a span: a sample player's store to
+// its DAC comes right after its interrupt is acknowledged, so a slice that starts there and fits
+// the rest of the period never spans a second timer underflow.
+enum : uint8_t { C64SpansFlagLockToStore = 0x01, C64SpansFlagsKnown = C64SpansFlagLockToStore };
+
+// Counters of the sample store watch, IntervalCyc and LongestCyc in ARM cycles.  Locked and
+// Unlocked count the slices that did and did not find a store to start on.
+struct SampleWatchStats { uint32_t Intervals; uint64_t IntervalCyc; uint32_t LongestCyc, Locked, Unlocked; };
+
 // Every span must end at or below $FFFF and stay off $DE00-$DFFF, which is this cartridge's own
 // IO1/IO2 and not memory.  *Total is the payload they need, in span order, when OK.  SliceBytes
 // 0 is a span per slice.

Source/Teensy/MinimalBoot/Common/Common_Defs.h

@@ -73,6 +73,7 @@
 #define ResetC64Token     0x64EE
 #define WriteC64MemToken  0x64FB
 #define WriteC64SpansToken 0x64FC
+#define SampleWatchToken  0x64F9
 #define ReadC64MemToken   0x64FD
 #define RetryToken        0x9B7E
 #define FailToken         0x9B7F
@@ -170,6 +171,10 @@ enum DMA_States  //used with DMA_State
 #define DMA_TIMEOUT_CYCLES  5000
 uint32_t DMACycleCount;
 volatile uint8_t DMA_State = DMA_S_DisableReady;
+//isrPHI2 starts the staged transfer as soon as DMA is asserted and releases the bus as soon as it
+//lands, rather than waiting on thread mode for each step.  Only a job that stages its transfer
+//before starting DMA may set it: a pause started while it is set would run the last staged transfer.
+volatile bool DMA_AutoRun = false;
 
 bool (*fBusSnoop)(uint16_t Address, bool R_Wn) = NULL;    //Bus snoop routine, return true to skip out of phi2 isr
 

Source/Teensy/MinimalBoot/Common/DMAControl.c

@@ -264,18 +264,142 @@ FLASHMEM bool CloseDMA()
 // what its slice cap allows even on a bus that makes every handshake run to its own ceiling.
 #define DMA_SPANS_CEILING_mS  (C64SlicesMax * 5 + 1000)
 
+// The extension image includes this file but never reaches the store watch or locked slices,
+// and FASTRUN would put SampleStoreSnoop in its ITCM regardless, so they are main-image only.
+#ifndef MinimumBuild
+// The sample store watch: every CPU write to SampleStoreAddr is counted against the time since
+// the one before it, so a host that knows the store's period can count the stores that did not
+// happen -- NMIs lost to a halt -- at the source rather than from captured audio.  A slice armed
+// with SampleLockArmed starts on the cycle after the next one.
+uint32_t SampleStoreAddr;
+volatile bool SampleLockArmed, SampleHaveLast;
+volatile uint32_t SampleLastStoreCyc;
+volatile SampleWatchStats SampleStats;
+
+FASTRUN bool SampleStoreSnoop(uint16_t Address, bool R_Wn)
+{
+   if (R_Wn || Address != SampleStoreAddr) return false;
+   const uint32_t Now = StartCycCnt;
+   if (SampleHaveLast)
+   {
+      const uint32_t Gap = Now - SampleLastStoreCyc;
+      SampleStats.Intervals++;
+      SampleStats.IntervalCyc += Gap;
+      if (Gap > SampleStats.LongestCyc) SampleStats.LongestCyc = Gap;
+   }
+   SampleLastStoreCyc = Now;
+   SampleHaveLast = true;
+   if (SampleLockArmed)
+   {
+      SampleLockArmed = false;
+      DMA_State = DMA_S_StartAsynch;
+   }
+   return false;
+}
+
+// False when another user holds the single bus snoop slot, the way BusAnalysis declines it.  The
+// test is masked with the install because a C64 write to rCtlRunningPRG claims the slot from isrPHI2.
+FLASHMEM bool SampleWatchStart(uint16_t Address)
+{
+   __disable_irq();
+   if (fBusSnoop != &SampleStoreSnoop && (fBusSnoop != NULL || PendingfBusSnoop != NULL))
+   {
+      __enable_irq();
+      return false;
+   }
+   if (fBusSnoop != &SampleStoreSnoop || SampleStoreAddr != Address) SampleHaveLast = false;
+   SampleStoreAddr = Address;
+   fBusSnoop = &SampleStoreSnoop;
+   __enable_irq();
+   return true;
+}
+
+FLASHMEM void SampleWatchStop()
+{
+   __disable_irq();
+   if (fBusSnoop == &SampleStoreSnoop) fBusSnoop = NULL;
+   SampleLockArmed = SampleHaveLast = false;
+   __enable_irq();
+}
+
+// The counters since the last call.  The last store's stamp is kept, so the next interval
+// counted is the one that crosses this call and none goes missing between two reads.
+FLASHMEM SampleWatchStats SampleWatchTake()
+{
+   __disable_irq();
+   const SampleWatchStats Taken = { SampleStats.Intervals, SampleStats.IntervalCyc, SampleStats.LongestCyc,
+                                    SampleStats.Locked, SampleStats.Unlocked };
+   SampleStats.Intervals = SampleStats.LongestCyc = SampleStats.Locked = SampleStats.Unlocked = 0;
+   SampleStats.IntervalCyc = 0;
+   __enable_irq();
+   return Taken;
+}
+
+// Arms the staged slice to start on the next watched store, or starts it anyway once WaituS has
+// passed without one -- audio stopped, or a scene with no player -- so a write never waits on a
+// store that is not coming.
+static FLASHMEM void StartSliceOnStore(uint32_t WaituS)
+{
+   const uint32_t Began = micros();
+   SampleLockArmed = true;
+   while (SampleLockArmed && micros() - Began < WaituS);
+   __disable_irq();
+   const bool Missed = SampleLockArmed;
+   if (Missed)
+   {
+      SampleLockArmed = false;
+      DMA_State = DMA_S_StartAsynch;
+   }
+   __enable_irq();
+   if (Missed) SampleStats.Unlocked++;
+   else SampleStats.Locked++;
+}
+
+struct DMAAutoRunJob
+{
+   DMAAutoRunJob() { DMA_AutoRun = true; }
+   ~DMAAutoRunJob() { DMA_AutoRun = false; }
+};
+#endif
+
 // Writes Spans from Payload, packed in span order, SliceBytes at a time (0 = a span at a time),
 // giving the 6510 GapuS between slices.  Both are the caller's: only it knows how long a halt
 // its C64 code can take, and how long its interrupt handler needs to run between them.  Stops
 // at the first slice or release that fails, or at DMA_SPANS_CEILING_mS, with the bus released, and
 // returns false.  *Landed
 // counts every slice that finished, even one whose CloseDMA then failed; a slice aborted
 // mid-transfer may have partly landed and is not counted.
+//
+// Locked, each slice starts on the cycle after a watched store (SampleWatchStart must have
+// succeeded) and GapuS is instead how long to wait for one.  The ISR runs the slice and releases
+// the bus itself, since a round trip through thread mode costs more halt than the period allows.
 FLASHMEM bool WriteC64Spans(const C64Span *Spans, uint32_t Count, uint8_t *Payload, uint32_t SliceBytes, uint32_t GapuS,
-                            uint32_t *Landed)
+                            uint32_t *Landed, bool Locked)
 {
    const uint32_t Began = millis();
    *Landed = 0;
+#ifndef MinimumBuild
+   if (Locked)
+   {
+      DMAAutoRunJob Job;
+      for (uint32_t Num = 0; Num < Count; Num++)
+      {
+         for (uint32_t Done = 0; Done < Spans[Num].Len; )
+         {
+            uint32_t Slice = Spans[Num].Len - Done;
+            if (SliceBytes && Slice > SliceBytes) Slice = SliceBytes;
+            if (millis() - Began >= DMA_SPANS_CEILING_mS) return false;
+            SetUpDMA(DMA_WRITE, Spans[Num].Addr + Done, Payload + *Landed, Slice, DMA_ADDR_INCREMENT);
+            StartSliceOnStore(GapuS);
+            if (!WaitForDMAState(DMA_S_DisableReady, DMA_HANDSHAKE_CEILING_mS + DMATransferCeilingmS(Slice)))
+               return false;
+            Done += Slice;
+            *Landed += Slice;
+         }
+      }
+      return true;
+   }
+#endif
    for (uint32_t Num = 0; Num < Count; Num++)
    {
       for (uint32_t Done = 0; Done < Spans[Num].Len; )
@@ -361,7 +485,7 @@ void DMATransferISR()
 
       DMA_Count++;
    }
-   DMA_State = DMA_S_TransferComplete;
+   DMA_State = DMA_AutoRun ? DMA_S_StartDisable : DMA_S_TransferComplete;
 }
 
 #endif

Source/Teensy/MinimalBoot/Common/ISRs.c

@@ -52,7 +52,7 @@ void DMAWaitAssertReady()
 {
    WaitUntil_nS(nS_DMAAssert); 
    SetDMAAssert;
-   DMA_State = DMA_S_TransferReady;
+   DMA_State = DMA_AutoRun ? DMA_S_TransferExecuting : DMA_S_TransferReady;
 }
 
 //Phi2 rising edge:

Source/Teensy/SerUSBIO.ino

@@ -572,6 +572,9 @@ FLASHMEM void ProcessCommand()
       case WriteC64SpansToken: //Write C64 mem spans via DMA, sliced
          WriteC64SpansCommand();
          return;
+      case SampleWatchToken: //Count C64 writes to one address
+         SampleWatchCommand();
+         return;
       case ReadC64MemToken: //Write to C64 mem via DMA
          ReadC64MemCommand();
          return;
@@ -1316,8 +1319,9 @@ FLASHMEM void RefuseSpans(const char *Why, uint32_t Owed, uint32_t Began)
 // Workflow:
 // Receive <-- WriteC64SpansToken (0x64FC)
 // Send --> AckToken 0x64CC, or 0x9b7f and the reason while another DMA user holds the bus
-// Receive <-- Flags (0), slice bytes (0 = whole spans), gap between slices in uS, span count (1..64)
-// Receive <-- Span count x { C64 address (Hi,Low), length (Hi,Low) }
+// Receive <-- Flags (C64SpansFlag*), slice bytes (0 = whole spans), gap between slices in uS, span count (1..64)
+// Receive <-- Span count x { C64 address (Hi,Low), length (Hi,Low) }; with LockToStore, the first
+//             entry is { store address (Hi,Low), 0 (Hi,Low) } and the gap is the longest wait for a store
 // Send --> AckToken 0x64CC if the spans are accepted, 0x9b7f and the reason if not
 // Receive <-- Data bytes for every span, in span order
 // TR+ Performs DMA writes
@@ -1353,21 +1357,28 @@ FLASHMEM void WriteC64SpansCommand()
    // among them.  So the header alone waits out a stall.
    if (!ReceiveSpanBytes(Head, sizeof Head, 255 * 4, Began, sizeof List, true)) return;
    const uint32_t Count = Head[3];
-   if (Head[0]) return RefuseSpans("Flags must be 0", Count * 4, Began);
+   // Firmware before LockToStore refuses every flag with "Flags must be 0", so a sender tells the
+   // two apart by the refusal a flagged zero-span header gets: that text, or the span count's.
+   if (Head[0] & ~C64SpansFlagsKnown) return RefuseSpans("Flags must be 0", Count * 4, Began);
    if (Count > C64SpansMax) return RefuseSpans(Why[(int)C64SpansCheck::BadCount], Count * 4, Began);
    if (!ReceiveSpanBytes(List, Count * 4, 0, Began, sizeof List)) return;
+   const bool Locked = Head[0] & C64SpansFlagLockToStore;
+   const uint32_t First = Locked ? 1 : 0;
+   if (Count <= First) return RefuseSpans(Why[(int)C64SpansCheck::BadCount], 0, Began);
    for (uint32_t Num = 0; Num < Count; Num++)
       Spans[Num] = { (uint16_t)(List[Num*4] << 8 | List[Num*4+1]), (uint16_t)(List[Num*4+2] << 8 | List[Num*4+3]) };
+   if (Locked && Spans[0].Len) return RefuseSpans("Store entry's length must be 0", 0, Began);
 
    // WriteC64Mem's reach.
-   const C64SpansCheck Check = CheckC64Spans(Spans, Count, Head[1], 0x10000, &Total);
+   const C64SpansCheck Check = CheckC64Spans(Spans + First, Count - First, Head[1], 0x10000, &Total);
    if (Check != C64SpansCheck::OK) return RefuseSpans(Why[(int)Check], 0, Began);
+   if (Locked && !SampleWatchStart(Spans[0].Addr)) return RefuseSpans("Bus snoop in use", 0, Began);
    DMAStage Stage(Total);
    if (!Stage.Buf) return RefuseSpans("No memory to stage the payload", 0, Began);
    SendU16(AckToken);
    if (!ReceiveSpanBytes(Stage.Buf, Total, 0, Began, sizeof List + Total)) return;
 
-   if (!WriteC64Spans(Spans, Count, Stage.Buf, Head[1], Head[2], &Landed))
+   if (!WriteC64Spans(Spans + First, Count - First, Stage.Buf, Head[1], Head[2], &Landed, Locked))
    {
       SendU16(FailToken);
       CmdChannel->printf("Stopped at %lu of %lu bytes: C64 bus not clocking, or DMA timed out\n", Landed, Total);
@@ -1376,7 +1387,50 @@ FLASHMEM void WriteC64SpansCommand()
    SendU16(AckToken);
 }
 
-// Command: 
+// Command:
+// Count the C64's writes to one address -- a sample player's store to its DAC -- to measure the
+// NMIs a DMA halt costs it, and report the counts since the previous SampleWatch
+//
+// Workflow:
+// Receive <-- SampleWatchToken (0x64F9)
+// Receive <-- Mode (0 = stop watching, 1 = watch), C64 address (Hi,Low)
+// Send --> AckToken 0x64CC, or 0x9b7f and the reason
+// Send --> Intervals (4), interval ARM cycles (8), longest interval ARM cycles (4), locked slices (4),
+//          unlocked slices (4), ARM cycles per second (4); each Low first, all since the previous SampleWatch
+FLASHMEM void SampleWatchCommand()
+{
+   uint32_t Mode, Addr;
+   if (!GetUInt(&Mode, 1) || !GetUInt(&Addr, 2))
+   {
+      SendU16(FailToken);
+      CmdChannel->println("Error receiving mode and address!");
+      return;
+   }
+   if (Mode > 1)
+   {
+      SendU16(FailToken);
+      CmdChannel->println("Mode must be 0 or 1");
+      return;
+   }
+   if (Mode == 0) SampleWatchStop();
+   else if (!SampleWatchStart(Addr))
+   {
+      SendU16(FailToken);
+      CmdChannel->println("Bus snoop in use");
+      return;
+   }
+   const SampleWatchStats Stats = SampleWatchTake();
+   SendU16(AckToken);
+   SendU32(Stats.Intervals);
+   SendU32(Stats.IntervalCyc);
+   SendU32(Stats.IntervalCyc >> 32);
+   SendU32(Stats.LongestCyc);
+   SendU32(Stats.Locked);
+   SendU32(Stats.Unlocked);
+   SendU32(F_CPU_ACTUAL);
+}
+
+// Command:
 // read sequential C64 memory segment and send to host
 // 
 // Workflow:

docs/Architecture/Constraints.md

@@ -16,7 +16,7 @@ New cartridge/IO types are added by writing a new `IO_Handlers/*.c` file and reg
 
 ## `fBusSnoop` is a single contested global slot — never assign it directly if anything else might own it
 
-`fBusSnoop` (declared in `Common_Defs.h`) is checked unconditionally, ahead of everything else, at the top of `isrPHI2()` — whatever it points to gets first refusal on every single bus cycle, regardless of `CurrentIOHandler`. That priority is exactly why it's useful for anything that needs to intercept the bus independent of the active handler, and exactly why blindly assigning it (`fBusSnoop = X;`) is dangerous the moment more than one thing might want it at once: REU (`IOH_REU.c`) and Kernal Replace (`IOH_KernalReplace.c`) both use it, `BusSnoop.ino`'s `BusAnalysis()` diagnostic borrows it temporarily, and the PRG-load handshake below claims it for the duration of a handler swap. Two failure shapes to watch for:
+`fBusSnoop` (declared in `Common_Defs.h`) is checked unconditionally, ahead of everything else, at the top of `isrPHI2()` — whatever it points to gets first refusal on every single bus cycle, regardless of `CurrentIOHandler`. That priority is exactly why it's useful for anything that needs to intercept the bus independent of the active handler, and exactly why blindly assigning it (`fBusSnoop = X;`) is dangerous the moment more than one thing might want it at once: REU (`IOH_REU.c`) and Kernal Replace (`IOH_KernalReplace.c`) both use it, `BusSnoop.ino`'s `BusAnalysis()` diagnostic borrows it temporarily, `SampleWatchStart()` (`DMAControl.c`, for SampleWatch and locked WriteC64Spans) borrows it the same way, and the PRG-load handshake below claims it for the duration of a handler swap. Two failure shapes to watch for:
 
 - **Stomp on install**: something assigns `fBusSnoop = X` while another owner still needs it live, silently disabling that owner with no error.
 - **Stomp on cleanup**: something unconditionally resets `fBusSnoop = NULL` (or reinstalls its own value) when it's done, without checking whether a *different* owner has since taken over — wiping out state that has nothing to do with the code doing the resetting. `BusAnalysis()` hit exactly this: its original `fBusSnoop = NULL;` cleanup, if it ran while a PRG-load handshake had taken over the slot mid-sample, would silently hang the C64's poll loop forever. Fixed by checking ownership both at entry (`if (fBusSnoop!=NULL || PendingfBusSnoop!=NULL) { ...abort... }`) and at cleanup (`if (fBusSnoop == &BusCount) fBusSnoop = NULL;`) — never assume you're still the owner just because you were a moment ago.
@@ -38,6 +38,8 @@ The mechanism that replaced it, entirely without touching the ISR:
 
 **Deliberate exception — `InitHndlr_KERNALReplace_PreStart()`** (`IOH_KernalReplace.c`): Kernal Replace needs its hook live *before* BASIC even initializes, which is earlier than any PRG-load handshake could possibly be armed — so this one function is correctly allowed to assign `fBusSnoop = &KernalCheck` directly, unlike every other `InitHndlr`. It's a separate function from the struct's actual `InitHndlr` (`InitHndlr_KernalReplace()`, which *does* go through `PendingfBusSnoop`, since it runs later via the normal swap path and needs to re-stage the hook after the handshake temporarily takes the slot). That restage is conditional: only while the handshake owns `fBusSnoop` (a PRG load) and PreStart actually loaded a kernal (`KernalBin` non-NULL; PreStart frees it on a failed load). PreStart runs on PRG launches only, but a generic (type 0) CRT inherits the Special IO selection, so with Kernal Replace selected its `InitHndlr` also runs on CRT launches; an unconditional restage hooked `KernalCheck` there with an unloaded `KernalBin`, driving GAME/EXROM over the cart's own lines, and generic CRTs failed to start (betas 0.8.0.4 through 0.8.0.15). Conflating the two — same underlying "load the kernal binary and hook the check" logic, called from two different timing contexts — was a real mistake made once already during this design; the split and the `_PreStart` naming exist specifically to keep that from happening again.
 
+**Deliberate exception — `DMA_AutoRun`** (`Common_Defs.h`; read in `ISRs.c` `DMAWaitAssertReady()` and `DMAControl.c` `DMATransferISR()`): a locked `WriteC64Spans` slice has to start, run and release the bus inside one NMI period (38 free cycles at 12 kHz NTSC), and the thread-mode handshake between those steps costs 70–80 cycles, so there is no main-loop version of it. The two reads replace a state assignment with a selected one, on cycles that already run DMA code (the assert, and the end of a transfer), never on an ordinary bus cycle or before an IO1/ROM response. The flag is set only for the length of a locked job.
+
 ### The real dividing line: flash-backed vs. RAM-backed, not just "the ISR function itself"
 
 The no-new-logic rule extends to everything reachable from the ISR's call path, not just `isrPHI2()` literally: `ROMLHndlr`/`ROMHHndlr`/`IO1Hndlr`/`IO2Hndlr`/`CycleHndlr` of the *active* IO handler are all called every cycle and must stay just as disciplined. Within that call path, only cheap, deterministic operations are safe — register reads/writes, small variable updates, anything backed by RAM1/RAM2. Nothing backed by flash is safe, and that's a wider category than it first looks: