cdrom: a Read that continues the current stream must not restart the drive

Legend of Mana's land-creation effect rendered as hard-edged coloured
wedges instead of a soft bloom. The cause was in the CD-ROM controller,
not the renderer.

The game streams a file by issuing Setloc(next)+ReadN roughly 10,800
guest cycles after each sector arrives. Every ReadN called
start_read_stream(), which clears the sector buffer -- destroying the
sector that had already been delivered and announced but not yet drained
by the guest. Measured at the palette load:

  buffer      psx-runtime   DuckStation
  0x0E1F18      125111        125111
  0x0E2718      125113        125112   <- the effect's colour palette
  0x0DCF18      125113        125113

Sector 125112 carries the effect's 5-colour palette (0x0888F8 =
(248,136,8), 0xB0F8F8 = (248,248,176), verified against the disc image).
Destroyed before the guest could take it, its buffer received 125113's
raw file bytes instead, and the colour routine faithfully scaled those
into 216 distinct vertex colours -- the wedges. The table now holds 5.

DuckStation only calls BeginReading -- the function that clears the
buffers and re-arms the read-start latency -- when the request actually
moves the drive:

  if ((!setloc_pending || setloc_position.ToLBA() == GetNextSectorToBeRead())
      && (drive_state == Reading || ...))
    "Ignoring read command with pending/same setloc, already reading"
  else
    BeginReading();

A Read issued while the drive already streams the sector the pending
Setloc names is the game saying "keep going", not "start over". The
command is still ACKed with the same INT3; only the restart is skipped.

Two things fall out of it. The un-drained sector survives, so the game
receives the consecutive sectors it expects (109,110,111,112,113 rather
than 109,111,111,113,113). And each request stops paying
initial_read_delay_cycles() again: inter-sector gaps drop from ~456,000
cycles to 225,792 -- exactly the 2x cadence -- so this load takes 2 guest
frames instead of 13, matching DuckStation. int1_pended goes 10 -> 0.

Also included, developed alongside and verified in the same runs:

  - An 8-slot sector buffer ring, as the hardware has (nocash;
    DuckStation NUM_SECTOR_BUFFERS = 8). A delivered sector fills the
    NEXT slot and the INT1 that announces it latches the read slot, so a
    sector arriving mid-drain no longer clobbers the FIFO. The read
    pointer follows the slot each INT1 announces, captured when the INT1
    is RAISED: this drive free-runs, so latching "newest" at
    presentation time handed the guest a sector seven ahead of the one
    it asked for.
  - The seamless no-IRQ refill advances the read pointer, because it is
    a continuation of the drain already in progress. Without it a
    multi-sector DMA starves mid-transfer (measured: 1,128,960 starved
    drains).
  - A pended data-ready is released on a short schedule rather than
    inside the guest's ack store, matching DuckStation's
    QueueDeliverAsyncInterrupt.
  - Flow control for accelerated (non-1x) reads: never overwrite a
    sector the guest has not consumed while running faster than
    hardware, capped at the authentic sector period. Inert at authentic
    timing.

New cdrom_state counters ring_starved and ring_dropped, plus
cdrom_timing_dump for per-sector deadline/delivery records, are the
regression tripwires: ring_starved and int1_lost must stay 0.

Verified on Legend of Mana (SLUS-01013): the effect renders correctly,
the palette table holds 5 colour words, sectors arrive at the 2x
cadence, and no notification is lost.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alex Vanderveen
2026-08-24 12:19:56 -04:00
committed by Matthew Stanley
parent d1df20514f
commit ffa11fa2dc
3 changed files with 433 additions and 76 deletions
+66 -2
View File
@@ -5533,6 +5533,8 @@ static void handle_cdrom_state(int id, const char *json)
"\"read_cmd\":\"0x%02X\",\"read_delay\":%d,"
"\"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_starved\":%llu,\"ring_dropped\":%llu,"
"\"filter_file\":%u,\"filter_channel\":%u,\"muted\":%u,"
"\"seek_msf\":[%u,%u,%u],"
"\"pending\":{\"cmd\":\"0x%02X\",\"active\":%d,\"delay\":%d,\"phase\":%d},"
@@ -5551,6 +5553,10 @@ static void handle_cdrom_state(int id, const char *json)
(unsigned long long)s.int1_pended,
(unsigned long long)s.int1_lost,
s.int1_pending_now,
(unsigned long long)s.accel_consumer_waits,
(unsigned long long)s.accel_consumer_wait_cycles,
(unsigned long long)s.ring_starved,
(unsigned long long)s.ring_dropped,
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,
@@ -5726,10 +5732,11 @@ static void handle_cdrom_command_history(int id, const char *json)
if (frame_hi >= 0 && (int)e->frame > frame_hi) continue;
pos += snprintf(buf + pos, bufsz - pos,
"%s{\"seq\":%llu,\"frame\":%u,\"kind\":\"%s\","
"%s{\"seq\":%llu,\"cycle\":%llu,\"frame\":%u,\"kind\":\"%s\","
"\"cmd\":\"0x%02X\",\"param_count\":%u,\"params\":[",
emitted ? "," : "",
(unsigned long long)e->seq, e->frame,
(unsigned long long)e->seq,
(unsigned long long)e->cycle, e->frame,
cdrom_command_kind_name(e->kind),
e->cmd, e->param_count);
for (uint8_t i = 0; i < e->param_count && i < 16 && pos < bufsz - 96; i++) {
@@ -11553,6 +11560,12 @@ static void handle_cd_read_log(int id, const char *json)
uint32_t *dest, uint32_t *size);
int tail = json_get_int(json, "tail", 256);
/* Optional LBA window + output cap, matching the oracle's cd_read_log so
* one call shape returns the same span from both emulators. */
int lba_lo = json_get_int(json, "lba_lo", -1);
int lba_hi = json_get_int(json, "lba_hi", -1);
int max_entries = json_get_int(json, "max_entries", 65536);
int emitted_rows = 0;
uint32_t total = cd_dma_log_get_total();
uint32_t cap = 65536;
uint32_t avail = total < cap ? total : cap;
@@ -11567,6 +11580,10 @@ static void handle_cd_read_log(int id, const char *json)
int lba; uint32_t dest, size;
cd_dma_log_get_entry(i, &lba, &dest, &size);
if (lba < 0) continue;
if (emitted_rows >= max_entries) break;
if (lba_lo >= 0 && lba < lba_lo) continue;
if (lba_hi >= 0 && lba > lba_hi) continue;
emitted_rows++;
send_fmt("%s{\"lba\":%d,\"dest\":\"0x%08X\",\"size\":%u}",
first ? "" : ",", lba, dest, size);
first = 0;
@@ -11700,6 +11717,52 @@ static void handle_cdrom_timing(int id, const char *json)
send_fmt("{\"id\":%d,\"ok\":true,%s}\n", id, stats);
}
/* cdrom_timing_dump: per-sector records from the L1.5 timing ring.
* Parameters: tail (default 1024), lba_lo / lba_hi (optional filter).
* One row per physical sector deadline: when it was due, when it landed in
* the buffer, when its INT1 was armed and presented to INTC, and the
* data/dma/pended/lost flags -- enough to see a single skipped sector and
* whether the guest or the drive was late around it. */
static void handle_cdrom_timing_dump(int id, const char *json)
{
int tail = json_get_int(json, "tail", 1024);
if (tail < 1) tail = 1;
if (tail > 4096) tail = 4096;
int lba_lo = json_get_int(json, "lba_lo", -1);
int lba_hi = json_get_int(json, "lba_hi", -1);
uint64_t total = cdrom_timing_total();
uint64_t start = total > (uint64_t)tail ? total - (uint64_t)tail : 0;
send_fmt("{\"id\":%d,\"ok\":true,\"total\":%llu,\"entries\":[",
id, (unsigned long long)total);
int first = 1;
for (uint64_t seq = start; seq < total; seq++) {
CdTimingPub r;
if (!cdrom_timing_record(seq, &r)) continue;
if (lba_lo >= 0 && r.lba < lba_lo) continue;
if (lba_hi >= 0 && r.lba > lba_hi) continue;
send_fmt("%s{\"seq\":%llu,\"lba\":%d,\"frame\":%u,"
"\"due\":%llu,\"buffer\":%llu,\"irq_arm\":%llu,"
"\"intc\":%llu,\"data\":%u,\"dma\":%u,"
"\"pended\":%u,\"lost\":%u,\"irq_armed\":%u,"
"\"intc_seen\":%u}",
first ? "" : ",",
(unsigned long long)r.seq, r.lba, r.frame,
(unsigned long long)r.due_cycle,
(unsigned long long)r.buffer_cycle,
(unsigned long long)r.irq_arm_cycle,
(unsigned long long)r.intc_cycle,
(r.flags & 0x01u) ? 1 : 0,
(r.flags & 0x02u) ? 1 : 0,
(r.flags & 0x04u) ? 1 : 0,
(r.flags & 0x08u) ? 1 : 0,
(r.flags & 0x10u) ? 1 : 0,
(r.flags & 0x20u) ? 1 : 0);
first = 0;
}
send_fmt("]}\n");
}
/* autocompile_status: variant-capture automation state — autocapture
* enable/trigger counters + the background compile's state and output tail
* (in-memory ring; no log files). */
@@ -13470,6 +13533,7 @@ static const CmdEntry s_commands[] = {
{ "wtrace_reset", handle_wtrace_clear },
{ "wtrace_ranges", handle_wtrace_ranges },
{ "wtrace_dump", handle_wtrace_dump },
{ "cdrom_timing_dump", handle_cdrom_timing_dump },
{ "wtrace_stats", handle_wtrace_stats },
{ "wtrace_boot_dump", handle_wtrace_boot_dump },
{ "wtrace_boot_summary", handle_wtrace_boot_summary },