From 5e2c0919e19589c5cf9640945b00a559630addbb Mon Sep 17 00:00:00 2001 From: Alex Vanderveen Date: Mon, 24 Aug 2026 01:05:22 -0400 Subject: [PATCH] Reapply "cdrom: 8-slot sector buffer ring -- a delivery must not clobber a drain" This reverts commit 36796e52b255b98dc598dd2c3b56a46379a07491. --- runtime/include/cdrom.h | 6 ++ runtime/src/cdrom.c | 214 +++++++++++++++++++++++++------------ runtime/src/debug_server.c | 3 + runtime/src/dma.c | 4 + 4 files changed, 160 insertions(+), 67 deletions(-) diff --git a/runtime/include/cdrom.h b/runtime/include/cdrom.h index 8127c7db..950ee92d 100644 --- a/runtime/include/cdrom.h +++ b/runtime/include/cdrom.h @@ -52,6 +52,7 @@ void cdrom_warm_route_stats_json(char* out, int cap); void cdrom_timing_reset(void); void cdrom_timing_stats_json(char* out, int cap); int cdrom_get_delivered_lba(void); +void cdrom_notify_cd_dma_complete(void); /* Per-record view of the same ring, for localising a single lost/skipped * sector rather than summarising thousands. */ @@ -200,6 +201,11 @@ typedef struct CDROMDebugState { * int1_lost rising while the speed divisor != 1 is the regression. */ uint64_t accel_consumer_waits; uint64_t accel_consumer_wait_cycles; + /* 8-slot sector ring: sectors dropped because the guest fell a full + * ring behind (hardware does this too), and missed-sector INT1s + * re-announced when a drain completed with another sector waiting. */ + uint64_t ring_dropped; + uint64_t int1_redelivered; uint8_t int1_pending_now; } CDROMDebugState; diff --git a/runtime/src/cdrom.c b/runtime/src/cdrom.c index 59fc80d8..925040a5 100644 --- a/runtime/src/cdrom.c +++ b/runtime/src/cdrom.c @@ -147,10 +147,28 @@ static int response_count; #define FALLBACK_SECTOR_HEADER_SIZE 12 #define FALLBACK_WHOLE_SECTOR_SIZE (FALLBACK_SECTOR_HEADER_SIZE + SECTOR_SIZE) #define SECTOR_BUFFER_SIZE WHOLE_SECTOR_SIZE -static uint8_t sector_buffer[SECTOR_BUFFER_SIZE]; -static int sector_read_pos; -static int sector_available; -static int sector_size; +/* ---- 8-slot sector buffer ring -------------------------------------- + * Real hardware: the CDROM controller has 8 rotating 2340-byte sector + * buffers (nocash; DuckStation NUM_SECTOR_BUFFERS = 8). A delivered + * sector fills the NEXT slot; the INT1 that announces it latches the + * read slot; drains never race deliveries. + * + * The previous single buffer meant a sector arriving mid-drain clobbered + * the FIFO under the guest. Legend of Mana's land-creation loader drains + * TWO sectors per Setloc+ReadN cycle (the requested sector, then its + * continuation via the data-ready that follows); with one buffer the + * second drain re-read stale bytes and the palette load shifted by one + * sector -- raw file data rendered as vertex colours. Request streams + * measured identical on DuckStation; only the buffering differed. */ +#define CDROM_NUM_SECTOR_BUFFERS 8 +typedef struct { uint8_t data[SECTOR_BUFFER_SIZE]; int size; int pos; } CdSectorBuf; +static CdSectorBuf s_sector_ring[CDROM_NUM_SECTOR_BUFFERS]; +static int s_ring_read_buf; +static int s_ring_write_buf; +static uint64_t s_ring_dropped; /* unread slots overwritten (guest 8+ behind) */ +static uint64_t s_int1_redelivered; /* missed-sector INT1s recovered at drain end */ +#define RB_ (s_sector_ring[s_ring_read_buf]) +static int rb_available(void) { return RB_.size > 0 && RB_.pos < RB_.size; } static uint8_t last_sector_buffer[SECTOR_BUFFER_SIZE]; static int last_sector_lba; static int last_sector_size; @@ -730,9 +748,9 @@ static void trace_cdrom(uint8_t kind, uint32_t addr, uint32_t val, uint8_t width e->param_count = (uint8_t)param_count; e->response_read = (uint8_t)response_read; e->response_count = (uint8_t)response_count; - e->sector_available = (uint8_t)sector_available; - e->sector_read_pos = sector_read_pos; - e->sector_size = sector_size; + e->sector_available = (uint8_t)rb_available(); + e->sector_read_pos = RB_.pos; + e->sector_size = RB_.size; e->pending_cmd = pending.cmd; e->pending_pending = (uint8_t)pending.pending; e->pending_delay = pending_rem_cycles(); @@ -1369,36 +1387,40 @@ static int read_sector_at(int min, int sec, int sect) { delivery.skip_reason = CDROM_SKIP_XA_AUDIO_REALTIME; } - memset(sector_buffer, 0, sizeof(sector_buffer)); - if (delivery.data_delivered && (mode_reg & 0x20)) { - if (have_raw) { - memcpy(sector_buffer, raw_data + WHOLE_SECTOR_OFFSET, WHOLE_SECTOR_SIZE); - sector_size = WHOLE_SECTOR_SIZE; - history_bytes = sector_buffer; - history_size = sector_size; + /* Fill the NEXT ring slot. The slot being drained is untouched, so a + * sector arriving mid-drain no longer clobbers the FIFO under the + * guest. */ + CdSectorBuf *wb = NULL; + if (delivery.data_delivered) { + int wi = (s_ring_write_buf + 1) % CDROM_NUM_SECTOR_BUFFERS; + wb = &s_sector_ring[wi]; + if (wb->size > 0 && wb->pos < wb->size) + s_ring_dropped++; /* guest fell 8 sectors behind: hardware drop */ + memset(wb->data, 0, sizeof(wb->data)); + if (mode_reg & 0x20) { + if (have_raw) { + memcpy(wb->data, raw_data + WHOLE_SECTOR_OFFSET, WHOLE_SECTOR_SIZE); + wb->size = WHOLE_SECTOR_SIZE; + } else { + wb->data[0] = bin_to_bcd(min); + wb->data[1] = bin_to_bcd(sec); + wb->data[2] = bin_to_bcd(sect); + wb->data[3] = 0x02; /* Mode 2 sector. */ + memcpy(wb->data + FALLBACK_SECTOR_HEADER_SIZE, user_data, SECTOR_SIZE); + wb->size = FALLBACK_WHOLE_SECTOR_SIZE; + } } else { - sector_buffer[0] = bin_to_bcd(min); - sector_buffer[1] = bin_to_bcd(sec); - sector_buffer[2] = bin_to_bcd(sect); - sector_buffer[3] = 0x02; /* Mode 2 sector. */ - memcpy(sector_buffer + FALLBACK_SECTOR_HEADER_SIZE, user_data, SECTOR_SIZE); - sector_size = FALLBACK_WHOLE_SECTOR_SIZE; - history_bytes = sector_buffer; - history_size = sector_size; + memcpy(wb->data, user_data, SECTOR_SIZE); + wb->size = SECTOR_SIZE; } - } else if (delivery.data_delivered) { - memcpy(sector_buffer, user_data, SECTOR_SIZE); - sector_size = SECTOR_SIZE; - history_bytes = sector_buffer; - history_size = sector_size; - } else { - sector_size = 0; + wb->pos = 0; + s_ring_write_buf = wi; + history_bytes = wb->data; + history_size = wb->size; } - sector_read_pos = 0; - sector_available = delivery.data_delivered ? 1 : 0; if (delivery.data_delivered) { - memcpy(last_sector_buffer, sector_buffer, (size_t)sector_size); + memcpy(last_sector_buffer, wb->data, (size_t)wb->size); burst_note_sector(); if (s_warm_route_active) s_warm_route_sectors++; } else { @@ -1410,7 +1432,7 @@ static int read_sector_at(int min, int sec, int sect) { } } last_sector_lba = lba; - last_sector_size = delivery.data_delivered ? sector_size : history_size; + last_sector_size = delivery.data_delivered ? wb->size : history_size; last_sector_frame = (uint32_t)s_frame_count; last_sector_mode = mode_reg; last_sector_have_raw = (uint8_t)(have_raw ? 1 : 0); @@ -1440,9 +1462,13 @@ static void advance_msf(int* m, int* s, int* f) { } static void clear_sector_buffer(void) { - sector_read_pos = 0; - sector_size = 0; - sector_available = 0; + /* Drive-state change: the whole ring resets (DS ClearSectorBuffers). */ + for (int i = 0; i < CDROM_NUM_SECTOR_BUFFERS; i++) { + s_sector_ring[i].size = 0; + s_sector_ring[i].pos = 0; + } + s_ring_read_buf = 0; + s_ring_write_buf = 0; request_reg &= (uint8_t)~CDROM_REQUEST_BFRD; } @@ -1698,9 +1724,7 @@ static void process_cdda_stream(uint32_t cycles) { } static int data_fifo_ready(void) { - return (request_reg & CDROM_REQUEST_BFRD) && - sector_available && - sector_read_pos < sector_size; + return (request_reg & CDROM_REQUEST_BFRD) && rb_available(); } static uint64_t s_dataready_fires; /* INT1 (data-ready) raised per streamed sector — FMV dispatch probe */ @@ -1712,6 +1736,10 @@ static int deliver_read_sector(void) { if (!delivered) return 0; response_clear(); response_push(stat_reg); + /* The INT1 announces the newest slot; the guest's next drain reads it + * (DS: current_read_sector_buffer = current_write_sector_buffer at + * DataReady delivery). */ + s_ring_read_buf = s_ring_write_buf; set_irq(CDIRQ_DATA_READY); fire_cdrom_irq(); s_dataready_fires++; @@ -2315,6 +2343,7 @@ static void process_pending(uint32_t cycles) { advance_msf(&read_min, &read_sec, &read_sect); if (delivered) { response_push(stat_reg); + s_ring_read_buf = s_ring_write_buf; set_irq(CDIRQ_DATA_READY); fire_cdrom_irq(); } @@ -2495,8 +2524,8 @@ static void process_read_stream(uint32_t cycles) { uint64_t timing_seq = cd_timing_begin_sector( msf_to_lba(read_min, read_sec, read_sect)); if (irq_flag == 0) { - if (sector_available) { - trace_cdrom('O', 0, (uint32_t)sector_read_pos, 0); + if (rb_available()) { + trace_cdrom('O', 0, (uint32_t)RB_.pos, 0); } if (deliver_read_sector()) { cd_timing_flag(timing_seq, CDT_DATA); @@ -2549,16 +2578,51 @@ static void present_pending_dataready(void) { s_cd_timing_pending_seq = UINT64_MAX; response_clear(); response_push(pending_dataready_stat); + s_ring_read_buf = s_ring_write_buf; set_irq(CDIRQ_DATA_READY); fire_cdrom_irq(); cd_timing_arm_irq(timing_seq); s_dataready_fires++; } +/* Drain of the current slot finished. Hardware re-announces a sector the + * guest missed (DS CheckForSectorBufferReadComplete: "Redeliver missed + * sector on DMA/read complete"): if the newest slot already holds an + * unread sector, raise its data-ready now, or pend it behind a still + * -asserted INT. Without this, a game that drains two sectors per + * Setloc+ReadN cycle never hears about the second one -- the LoM palette + * failure. During an active multi-sector DMA the ring advances inline in + * cdrom_dma_read instead (the historical no-new-INT refill shape), and + * dma.c calls cdrom_notify_cd_dma_complete() at the end. */ +static void cdrom_ring_redeliver_check(void) { + if (s_ring_read_buf == s_ring_write_buf) return; + CdSectorBuf *nb = &s_sector_ring[s_ring_write_buf]; + if (nb->size > 0 && nb->pos == 0 && !pending_dataready) { + pending_dataready = 1; + pending_dataready_stat = stat_reg; + s_int1_redelivered++; + if (irq_flag == 0) + present_pending_dataready(); + } +} + +static void cdrom_ring_drain_complete(void) { + RB_.size = 0; + RB_.pos = 0; + if (!dma_cdrom_transfer_active()) + cdrom_ring_redeliver_check(); +} + +void cdrom_notify_cd_dma_complete(void) { + cdrom_ring_redeliver_check(); +} + void cdrom_init(const char* cue_path) { memset(param_fifo, 0, sizeof(param_fifo)); memset(response_fifo, 0, sizeof(response_fifo)); - memset(sector_buffer, 0, sizeof(sector_buffer)); + memset(s_sector_ring, 0, sizeof(s_sector_ring)); + s_ring_read_buf = 0; + s_ring_write_buf = 0; memset(last_sector_buffer, 0, sizeof(last_sector_buffer)); /* Rematch re-calls cdrom_init; boot must see 1x until game entry again. */ @@ -2575,9 +2639,6 @@ void cdrom_init(const char* cue_path) { param_count = 0; response_read = 0; response_count = 0; - sector_read_pos = 0; - sector_size = 0; - sector_available = 0; last_sector_lba = -1; last_sector_size = 0; last_sector_frame = 0; @@ -2678,9 +2739,9 @@ uint32_t cdrom_read(uint32_t addr) { case 0x1F801802: if (data_fifo_ready()) { - ret = sector_buffer[sector_read_pos++]; - if (sector_read_pos >= sector_size) { - sector_available = 0; + ret = RB_.data[RB_.pos++]; + if (RB_.pos >= RB_.size) { + cdrom_ring_drain_complete(); } } break; @@ -2745,7 +2806,7 @@ void cdrom_write(uint32_t addr, uint32_t value) { } else if (index_reg == 0) { request_reg = val; if (!(request_reg & CDROM_REQUEST_BFRD)) { - sector_read_pos = 0; + RB_.pos = 0; } } else if (index_reg == 1) { /* Controller IRQ acknowledge. irq_flag is a single numeric response @@ -2829,12 +2890,21 @@ void cdrom_tick(void) { uint32_t cdrom_dma_read(void) { uint32_t val = 0; int got = 0; - if ((request_reg & CDROM_REQUEST_BFRD) && sector_available && - sector_read_pos + 4 <= sector_size) { - memcpy(&val, sector_buffer + sector_read_pos, 4); - sector_read_pos += 4; - if (sector_read_pos >= sector_size) { - sector_available = 0; + if ((request_reg & CDROM_REQUEST_BFRD) && !rb_available() && + s_ring_read_buf != s_ring_write_buf && dma_cdrom_transfer_active()) { + /* Multi-sector DMA crossing a slot boundary: step to the next + * delivered slot -- the historical seamless-refill shape, now + * without clobbering the one being drained. */ + do { + s_ring_read_buf = (s_ring_read_buf + 1) % CDROM_NUM_SECTOR_BUFFERS; + } while (!rb_available() && s_ring_read_buf != s_ring_write_buf); + } + if ((request_reg & CDROM_REQUEST_BFRD) && rb_available() && + RB_.pos + 4 <= RB_.size) { + memcpy(&val, RB_.data + RB_.pos, 4); + RB_.pos += 4; + if (RB_.pos >= RB_.size) { + cdrom_ring_drain_complete(); } got = 1; } @@ -2854,13 +2924,15 @@ uint32_t cdrom_dma_read(void) { } int cdrom_dma_ready(void) { - return (request_reg & CDROM_REQUEST_BFRD) && - sector_available && - (sector_read_pos + 4 <= sector_size); + if ((request_reg & CDROM_REQUEST_BFRD) && !rb_available() && + s_ring_read_buf != s_ring_write_buf && dma_cdrom_transfer_active()) + return 1; /* next slot is ready; cdrom_dma_read will advance */ + return (request_reg & CDROM_REQUEST_BFRD) && rb_available() && + (RB_.pos + 4 <= RB_.size); } uint32_t cdrom_dma_sector_word_count(void) { - int size = sector_size; + int size = RB_.size; /* CD DMA BCR low half zero means transfer one sector-sized payload. * If DMA is armed just before the next sector becomes available, fall @@ -2893,9 +2965,9 @@ void cdrom_debug_snapshot(CDROMDebugState* out) { out->param_count = param_count; out->response_read = response_read; out->response_count = response_count; - out->sector_read_pos = sector_read_pos; - out->sector_available = sector_available; - out->sector_size = sector_size; + out->sector_read_pos = RB_.pos; + out->sector_available = rb_available(); + out->sector_size = RB_.size; out->reading = reading; out->read_min = read_min; out->read_sec = read_sec; @@ -2913,6 +2985,8 @@ void cdrom_debug_snapshot(CDROMDebugState* out) { out->int1_lost = s_int1_lost; out->accel_consumer_waits = s_accel_consumer_waits; out->accel_consumer_wait_cycles = s_accel_consumer_wait_cycles; + out->ring_dropped = s_ring_dropped; + out->int1_redelivered = s_int1_redelivered; out->int1_pending_now = pending_dataready; out->pending_pending = pending.pending; out->pending_delay = pending_rem_cycles(); @@ -2960,9 +3034,9 @@ uint32_t cdrom_debug_copy_last_sector(uint32_t offset, uint32_t len, CDROMSectorDebugState* state) { if (state) { memset(state, 0, sizeof(*state)); - state->current_available = sector_available; - state->current_read_pos = sector_read_pos; - state->current_size = sector_size; + state->current_available = rb_available(); + state->current_read_pos = RB_.pos; + state->current_size = RB_.size; state->last_lba = last_sector_lba; state->last_size = last_sector_size; state->last_frame = last_sector_frame; @@ -3082,7 +3156,10 @@ static int cdrom_snap_emit(PstW *w) { WU(cdrom_intc_latched_generation); WI(irq_present_rem_cycles()); WB(param_fifo); WI(param_count); WB(response_fifo); WI(response_read); WI(response_count); - WB(sector_buffer); WI(sector_read_pos); WI(sector_available); WI(sector_size); + for (int i = 0; i < CDROM_NUM_SECTOR_BUFFERS; i++) { + WB(s_sector_ring[i].data); WI(s_sector_ring[i].size); WI(s_sector_ring[i].pos); + } + WI(s_ring_read_buf); WI(s_ring_write_buf); WB(last_sector_buffer); WI(last_sector_lba); WI(last_sector_size); WB(last_valid_subq); WI(last_valid_subq_available); /* last_sector_frame is host s_frame_count — zero on the wire so netplay @@ -3130,7 +3207,10 @@ static int cdrom_snap_parse(PstR *r) { RU(cdrom_intc_latched_generation); RI(present_rem); RB(param_fifo); RI(param_count); RB(response_fifo); RI(response_read); RI(response_count); - RB(sector_buffer); RI(sector_read_pos); RI(sector_available); RI(sector_size); + for (int i = 0; i < CDROM_NUM_SECTOR_BUFFERS; i++) { + RB(s_sector_ring[i].data); RI(s_sector_ring[i].size); RI(s_sector_ring[i].pos); + } + RI(s_ring_read_buf); RI(s_ring_write_buf); RB(last_sector_buffer); RI(last_sector_lba); RI(last_sector_size); RB(last_valid_subq); RI(last_valid_subq_available); RU(last_sector_frame); R8(last_sector_mode); R8(last_sector_have_raw); diff --git a/runtime/src/debug_server.c b/runtime/src/debug_server.c index d637c904..3d357ad1 100644 --- a/runtime/src/debug_server.c +++ b/runtime/src/debug_server.c @@ -5592,6 +5592,7 @@ static void handle_cdrom_state(int id, const char *json) "\"read_hold_cycles\":%llu,\"read_hold_events\":%llu," "\"int1_pended\":%llu,\"int1_lost\":%llu,\"int1_pending_now\":%u," "\"accel_consumer_waits\":%llu,\"accel_consumer_wait_cycles\":%llu," + "\"ring_dropped\":%llu,\"int1_redelivered\":%llu," "\"filter_file\":%u,\"filter_channel\":%u,\"muted\":%u," "\"seek_msf\":[%u,%u,%u]," "\"pending\":{\"cmd\":\"0x%02X\",\"active\":%d,\"delay\":%d,\"phase\":%d}," @@ -5612,6 +5613,8 @@ static void handle_cdrom_state(int id, const char *json) s.int1_pending_now, (unsigned long long)s.accel_consumer_waits, (unsigned long long)s.accel_consumer_wait_cycles, + (unsigned long long)s.ring_dropped, + (unsigned long long)s.int1_redelivered, s.filter_file, s.filter_channel, s.muted, s.seek_min, s.seek_sec, s.seek_sect, s.pending_cmd, s.pending_pending, s.pending_delay, diff --git a/runtime/src/dma.c b/runtime/src/dma.c index 47c13599..4b80ca95 100644 --- a/runtime/src/dma.c +++ b/runtime/src/dma.c @@ -547,6 +547,10 @@ static void finish_async_cdrom_transfer(uint32_t final_addr) { channels[3].madr = final_addr; cancel_async_transfer(3); complete_transfer(3); + /* The DMA that drained a sector slot is done: if the next slot already + * holds an unread sector, the controller re-announces it now (DS + * CheckForSectorBufferReadComplete). */ + cdrom_notify_cd_dma_complete(); } static void advance_mdec_channel(int ch, uint32_t cycles) {