agnus: model sprite DMA at the register level

docs/internals/chipset.md

@@ -47,48 +47,47 @@ Alice adds the FMODE wide-fetch latch, which scales the bitplane and
 sprite fetch quanta (FMODE=0 stays byte-identical to the OCS/ECS slot
 timing).
 
-Sprite DMA retains its latched POS/CTL descriptor independently from the
-SPRxPT registers while a sprite data stream is active or waiting for VSTART.
-Standard PAL/NTSC hard vertical blank inhibits sprite DMA at the top of the
-field, so descriptor fetches are not replayed until PAL line $19 or NTSC line
-$14.
-If software rewrites SPRxPT on a later beam line while that descriptor is
-still pending, the retained POS/CTL stays latched and the new SPRxPT value
-retargets the descriptor's post-control data stream. A same-line rewrite
-after the descriptor fetch is treated as a descriptor restart for the next
-sprite DMA slot; otherwise freshly loaded descriptor words would be mistaken
-for sprite data.
-Software can also write SPRxPOS/SPRxCTL directly and let sprite DMA fetch
-data from the current SPRxPT stream; in that case SPRxPT names the first
-data word pair, not a memory descriptor. A save-state load clears
-Copperline's transient Agnus descriptor latch, so this register-stream case
-is reconstructed from Denise's retained armed state, SPRxPOS/SPRxCTL, and
-an after-slot SPRxPT low-word write in the rendered display area. If a DMA
-descriptor has already been retained and is still waiting for VSTART, later
-SPRxPOS/SPRxCTL writes
-update the live comparators but keep the descriptor's post-control data
-origin. The frame-start replay path mirrors this by replaying off-screen
-DMACON and SPRxPT writes in beam order before rendering the visible field.
-Enabled sprite slots that fetch no sprite data are not treated as observed
-sprite DMA; otherwise empty DMA slots would suppress valid register/manual
-sprite replay for that frame.
-
-SPRxPT advances through sprite DMA and is not snapped back to the last value
-the Copper/CPU wrote at the top of the next field. A channel that has read its
-terminating descriptor leaves SPRxPT parked at the DMA frontier past the
-consumed list, so the next field's replay is seeded from that frontier rather
-than the stale descriptor address. Programs must reload SPRxPT every field
-(normally from the Copper) to keep a sprite displayed; this is what prevents a
-reused sprite descriptor buffer that software overwrites between fields from
-being re-armed from its previous, now-overwritten address before the Copper's
-reload lands. A channel still mid-descriptor at field end keeps the written
-pointer so an active reused sprite is not skipped past its data.
-
-Sprite descriptors whose decoded VSTART equals VSTOP idle the current
-sprite stream until software rearms it or the next field fetches again;
-the following words are not scanned as another descriptor. This is distinct
-from an inverted VSTOP smaller than VSTART, where the stop comparator has
-already passed and DMA continues to the bottom of the field.
+Sprite DMA is modelled at the register level, the way the chips work:
+there is no separate "descriptor" concept. Each channel keeps Agnus-side
+copies of its SPRxPOS/SPRxCTL words and the vertical comparator values
+derived from them, updated identically by DMA fetches and by CPU/Copper
+pokes (POS supplies the vertical start's low bits, CTL the high bit, the
+whole vertical stop, and disarms the display latches). At each line start
+the comparators run: passing vstart sets the channel's DMA flip-flop,
+passing vstop clears it -- even when SPREN is off, which is what leaves a
+sprite dead until the next field when software disables DMA across its
+vstop line. Writes landing on the matching line re-evaluate immediately.
+
+A sprite line's two DMA slots ($15+4N and $17+4N) are evaluated at their
+own colour clocks. On the channel's vstop line the slots fetch the next
+control words (POS in the first slot, CTL in the second); on other lines
+an enabled channel fetches DATA and DATB, so chip RAM rewritten between
+the two slots is seen by DATB but not DATA. The vertical-blank reset
+(PAL line $19, NTSC line $14 -- sprite DMA is inhibited above it) forces
+every channel's vstop to that line, which is how each field's first
+control-word fetch happens; software reloads SPRxPT each vblank to point
+it at the sprite list. An "inverted" pair (vstop before vstart, or a
+$0000/$0000 terminator) simply parks the comparators on values the beam
+has passed or never shows: the terminator decodes to vstart=vstop=0 and
+the channel stays silent until the next field's reset.
+
+DMACON's SPREN is sampled by each DMA slot individually, so a mid-line
+edge fetches exactly one word of the pair. SPRxPT advances only on words
+actually fetched: a skipped slot leaves the pointer behind and the stream
+shifts accordingly. The display line assembles from the fetched words
+plus the stale latch on a skipped side; a missed DATA slot never arms the
+sprite, and an armed channel with DMA off keeps redisplaying its latches
+at the current POS/CTL decode until a CTL fetch or poke disarms it (the
+vAmigaTS sprena/sprdis families' vertical bars).
+
+Because SPRxPT only moves on fetches, a channel that consumed its list
+leaves the pointer parked at the DMA frontier past it, and the next
+field's replay seeds from that frontier rather than from the stale
+last-written value; programs must reload SPRxPT every field (normally
+from the Copper) to keep a sprite displayed. The frame-start replay path
+replays off-screen DMACON, SPRxPT and SPRxPOS/CTL writes in beam order
+before rendering the visible field. The whole per-channel register state
+is chip state: save states serialize and restore it directly.
 
 A modelling note that catches people out: OCS lo-res with BPU=7 is an
 overprogrammed mode. Denise still decodes six BPLDAT latches, but Agnus

src/bus.rs

@@ -307,19 +307,103 @@ pub struct HeldSpriteLine {
     pub vstop: i32,
 }
 
-#[derive(Clone, Copy, Debug, Default, serde::Serialize, serde::Deserialize)]
+/// Agnus's per-channel sprite DMA state, modelled at the register level the
+/// way the chip works: POS/CTL control words are fetched on the channel's
+/// vstop line through its two DMA slots, the vertical comparators run off
+/// these register copies at each line start (and immediately on CPU/Copper
+/// pokes), the DMA flip-flop is cleared on the vstop line even when SPREN
+/// is off, and SPRxPT advances only on words actually fetched.
+#[derive(Clone, Copy, Debug, serde::Serialize, serde::Deserialize)]
 struct DisplaySpriteDmaState {
-    control: Option<DisplaySpriteControl>,
-    #[serde(skip, default = "unset_sprite_control_loaded_vpos")]
-    control_loaded_vpos: i32,
-    next_ptr: Option<u32>,
-    terminated: bool,
-    data_dma_active: bool,
+    /// Channel copies of the SPRxPOS/SPRxCTL register words (from DMA
+    /// control-word fetches or CPU/Copper writes); the horizontal position
+    /// and attach decode for emitted lines comes from these.
+    pos: u16,
+    ctl: u16,
+    /// Vertical comparator values. Mostly derived from pos/ctl, but held
+    /// separately because the vertical-blank reset writes vstop without
+    /// touching the CTL word.
+    vstrt: i32,
+    vstop: i32,
+    /// The per-sprite DMA flip-flop: set when the line counter passes
+    /// vstrt, cleared when it passes vstop (even with SPREN off) and in
+    /// the frame's last line.
+    dma_enabled: bool,
+    /// Line whose start-of-line comparator update has already run (the
+    /// update is applied lazily, catching up over skipped lines).
+    #[serde(default = "unset_sprite_line_marker")]
+    comparator_vpos: i32,
+    /// Display latches from the last assembled line: a skipped slot reuses
+    /// the stale side, and with DMA off an armed channel keeps displaying
+    /// this line until a CTL fetch or poke disarms it.
     last_line: Option<DisplaySpriteLineData>,
+    /// Display line of the last DATA fetch: with SSCAN2 the hardware
+    /// fetches on every other line and redisplays in between.
+    last_data_fetch_vpos: i32,
+    /// DATA word(s) fetched by the sprite's first DMA slot, sampled at that
+    /// slot's beam time, awaiting the second slot to assemble the line.
+    pending_data: Option<(u16, [u16; 3])>,
+    /// Line the first slot's DATA fetch ran for.
+    #[serde(default = "unset_sprite_line_marker")]
+    pending_line_vpos: i32,
+    /// Line the first slot's evaluation ran for, so the second slot
+    /// completes the line rather than re-running the entry logic.
+    #[serde(default = "unset_sprite_line_marker")]
+    entry_line_vpos: i32,
+}
+
+fn unset_sprite_line_marker() -> i32 {
+    i32::MIN
 }
 
-fn unset_sprite_control_loaded_vpos() -> i32 {
-    i32::MIN
+impl Default for DisplaySpriteDmaState {
+    fn default() -> Self {
+        // The line-marker fields must start unset, not at line 0: a derived
+        // zero default would make a fresh state claim its per-line work
+        // already ran when the beam is on line 0.
+        Self {
+            pos: 0,
+            ctl: 0,
+            vstrt: 0,
+            vstop: 0,
+            dma_enabled: false,
+            comparator_vpos: unset_sprite_line_marker(),
+            last_line: None,
+            last_data_fetch_vpos: unset_sprite_line_marker(),
+            pending_data: None,
+            pending_line_vpos: unset_sprite_line_marker(),
+            entry_line_vpos: unset_sprite_line_marker(),
+        }
+    }
+}
+
+impl DisplaySpriteDmaState {
+    /// A POS word (fetch or poke) replaces the vertical start's low bits,
+    /// keeping the CTL-supplied high bit.
+    fn poke_pos(&mut self, value: u16) {
+        self.pos = value;
+        self.vstrt = (self.vstrt & 0x100) | i32::from(value >> 8);
+    }
+
+    /// A CTL word (fetch or poke) supplies vstart's high bit and the whole
+    /// vstop, and disarms the channel's display latches.
+    fn poke_ctl(&mut self, value: u16) {
+        self.ctl = value;
+        self.vstrt = (self.vstrt & 0x0FF) | (i32::from(value & 0x0004) << 6);
+        self.vstop = (i32::from(value & 0x0002) << 7) | i32::from(value >> 8);
+        self.last_line = None;
+    }
+
+    /// The comparators also fire immediately when a poke lands on the
+    /// matching line (vstop wins over vstrt when both match).
+    fn reevaluate_comparators_at(&mut self, beam_y: i32) {
+        if self.vstrt == beam_y {
+            self.dma_enabled = true;
+        }
+        if self.vstop == beam_y {
+            self.dma_enabled = false;
+        }
+    }
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -466,43 +550,6 @@ impl Default for BitplaneSlotPlanCache {
     }
 }
 
-#[derive(Clone, Copy, Debug, serde::Serialize, serde::Deserialize)]
-struct DisplaySpriteControl {
-    vstart: i32,
-    vstop: i32,
-    hstart: i32,
-    hsub_70ns: bool,
-    #[serde(skip, default = "unset_sprite_data_vstart")]
-    data_vstart: i32,
-    data_base: u32,
-    next_ptr: u32,
-    attached: bool,
-}
-
-fn unset_sprite_data_vstart() -> i32 {
-    i32::MIN
-}
-
-fn register_sprite_data_vstart() -> i32 {
-    i32::MIN + 1
-}
-
-impl DisplaySpriteControl {
-    fn effective_data_vstart(self) -> i32 {
-        if self.data_vstart == unset_sprite_data_vstart()
-            || self.data_vstart == register_sprite_data_vstart()
-        {
-            self.vstart
-        } else {
-            self.data_vstart
-        }
-    }
-
-    fn data_origin_is_register_stream(self) -> bool {
-        self.data_vstart == register_sprite_data_vstart()
-    }
-}
-
 #[derive(Clone, Copy, Debug, serde::Serialize, serde::Deserialize)]
 struct DisplaySpriteLineData {
     hstart: i32,
@@ -628,6 +675,10 @@ pub struct Bus {
     pub input: InputState,
 
     pub poll_stats: PollStats,
+    /// Host performance telemetry (call/probe/nanosecond counters), not
+    /// machine state: call chunking legitimately differs across a
+    /// save-state load, so it is excluded from the serialized layout.
+    #[serde(skip)]
     video_pipeline_stats: VideoPipelineStats,
 
     /// Set by MMIO writes that should end the current CPU slice after
@@ -872,14 +923,15 @@ pub struct Bus {
     /// frame boundary instead of re-seeding from `denise.sprpt` (the stale last
     /// write), so a sprite descriptor buffer that software rewrites every field
     /// is not re-armed from its previous, now-overwritten address before the
-    /// Copper reloads SPRxPT. Transient render state, rebuilt each frame (and
-    /// re-derived after a state load), so it is excluded from the serialized
-    /// layout.
-    #[serde(skip)]
+    /// Copper reloads SPRxPT. Captured at each frame start from the live
+    /// (fetch-advanced) pointers, and serialized: the live pointers move on
+    /// between the frame start and a mid-frame state save, so it cannot be
+    /// re-derived at load time.
+    #[serde(default)]
     sprite_dma_frame_start_ptr: [u32; 8],
-    // Derived from sprite DMA descriptor/control fetches. Kept in the bincode
-    // layout for compatibility, then reset after a state load so stale decoded
-    // latches are rebuilt from the restored pointer context.
+    // The register-level sprite DMA state (comparator copies, DMA
+    // flip-flops, display latches). Chip state: serialized and restored
+    // exactly by save states.
     display_dma_sprite_state: [DisplaySpriteDmaState; 8],
     display_dma_clipped_rows_advanced: bool,
     bitplane_dmacon_delay: Option<BitplaneDmaconDelay>,
@@ -2505,7 +2557,6 @@ impl Bus {
         self.last_frame_sprite_display_enable_x_by_y = empty_sprite_display_enable_x_by_y();
         self.current_frame_sprite_dma_observed = false;
         self.last_frame_sprite_dma_observed = false;
-        self.display_dma_sprite_state = [DisplaySpriteDmaState::default(); 8];
         self.current_frame_bus_trace.clear();
         self.last_frame_bus_trace = None;
     }
@@ -2578,6 +2629,7 @@ impl Bus {
         self.last_frame_beam_bottom_palette = Palette::new();
         self.last_frame_beam_bottom_palette_valid = false;
         self.reset_frame_capture_buffers();
+        self.display_dma_sprite_state = [DisplaySpriteDmaState::default(); 8];
         self.current_frame_display_snapshot_taken = false;
         self.ocs_same_line_diw_start_blocked_vpos = None;
         self.current_frame_render_blocked = false;
@@ -2727,7 +2779,6 @@ impl Bus {
         self.ocs_same_line_diw_start_blocked_vpos = None;
         self.reset_frame_capture_buffers();
         self.current_frame_render_blocked = self.agnus.vpos != 0 || self.agnus.hpos != 0;
-        self.sprite_dma_frame_start_ptr = self.display_dma_sprpt;
     }
 
     pub fn emulated_seconds(&self) -> f64 {
@@ -4768,12 +4819,11 @@ impl Bus {
                 let mut s = String::new();
                 for i in 0..8 {
                     let st = &self.display_dma_sprite_state[i];
-                    let act = st.data_dma_active as u8;
-                    let term = st.terminated as u8;
-                    let (cvs, cve) = st.control.map(|c| (c.vstart, c.vstop)).unwrap_or((-1, -1));
+                    let ena = st.dma_enabled as u8;
+                    let armed = st.last_line.is_some() as u8;
                     s.push_str(&format!(
-                        " s{i}[act={act} term={term} ctl_vs={cvs} ctl_ve={cve} nxt={:?}]",
-                        st.next_ptr
+                        " s{i}[ena={ena} armed={armed} vstrt={} vstop={} ptr={:06X}]",
+                        st.vstrt, st.vstop, self.display_dma_sprpt[i]
                     ));
                 }
                 log::info!(

src/bus/custom_regs.rs

@@ -1093,9 +1093,6 @@ impl Bus {
                         );
                     }
                     self.display_dma_sprpt[idx] = self.denise.sprpt[idx];
-                    if off & 2 != 0 {
-                        self.apply_display_sprite_pointer_low_write(idx);
-                    }
                 }
                 false
             }

src/bus/dma_slots.rs

@@ -586,6 +586,9 @@ impl Bus {
         if self.agnus.dmacon & DMACON_SPREN == 0 {
             return false;
         }
+        if self.sprite_dma_inhibited_by_vertical_blank_at(self.agnus.vpos) {
+            return false;
+        }
         // Sprite DMA slots sit on ODD color clocks (same parity as refresh/
         // disk/audio -- the HRM chart's fixed-DMA band), so they never block
         // the Copper's even-clock fetches.
@@ -596,7 +599,10 @@ impl Bus {
         if sprite >= 8 {
             return false;
         }
-        self.display_dma_sprite_state[sprite].data_dma_active
+        // A channel uses its slots when it is fetching data this line, or on
+        // its vstop line where the slots fetch the next POS/CTL control word.
+        let state = &self.display_dma_sprite_state[sprite];
+        state.dma_enabled || self.agnus.vpos as i32 == state.vstop
     }
 
     pub(super) fn record_bitplane_dmacon_write(&mut self, previous: u16) {