diff --git a/harness/mame/vidstream.lua b/harness/mame/vidstream.lua index dc0f1df..a9e66bf 100644 --- a/harness/mame/vidstream.lua +++ b/harness/mame/vidstream.lua @@ -254,11 +254,11 @@ local DICT_BAD = MARK_PAGE + 0x58 * 8 local TILES_DROP = MARK_PAGE + 0x5c * 8 -- Must match vidstream.c. -local SLOT_BYTES = 24576 +local SLOT_BYTES = 49152 -- The RX ring (Task 7), from vidstream.c: RX_DESC descriptors of 8 bytes each, -- one RXBUFLEN-byte buffer per descriptor inside the armed slot. -local RX_DESC = 16 +local RX_DESC = 32 local RXBUFLEN = 1526 local ETH_HDR = 14 local ETH_FCS = 4 @@ -1045,6 +1045,32 @@ local function ring_armed_for(base, what) what, want, RXBUFLEN, badown, badaddr, badlen, badstride)) end + +-- How many descriptors point into slot A, slot B, and neither? +-- The payload now releases descriptors one at a time as their data is consumed +-- (rx_release in vidstream.c), so after a frame the consumed ones have moved to +-- the other slot while a descriptor the consumer has not reached yet stays +-- where it was. Asserting "all 16 in slot B" was the old all-at-once flip, and +-- that flip is what silently discarded frames the chip had already delivered. +local function ring_slot_counts(a_base, b_base) + local want_a, want_b = a_base // 8, b_base // 8 + local i, n_a, n_b, bad = 0, 0, 0, 0 + local in_a, in_b, in_bad = {}, {}, {} + for i = 0, RX_DESC - 1 do + local addr = buf_addr(ring_ringb, i) + if addr == want_a + i * RXBUFLEN then + n_a = n_a + 1; in_a[#in_a + 1] = i + elseif addr == want_b + i * RXBUFLEN then + n_b = n_b + 1; in_b[#in_b + 1] = i + else + bad = bad + 1; in_bad = in_bad or {} + in_bad[#in_bad + 1] = string.format("%d:%06X", i, addr) + end + end + return n_a, n_b, bad, table.concat(in_a, ","), table.concat(in_b, ","), + table.concat(in_bad, ",") +end + -- The 32 bytes at the start of screen row 0. Enough to see a blit: the frame -- ring mode delivers paints row 0 with 0xF0 bytes, so any paint changes them. local function read_vram_head(n) @@ -1186,9 +1212,12 @@ local function ring_step() ring_check(prog:read_u32(RX_RING_ACC) == 0, "the frame left no bytes in progress") ring_check(ring_dgrams >= 15, string.format("the consumer took all 15 datagrams (%d)", ring_dgrams)) - ring_check(prog:read_u32(RX_RING_ARMED) == 1, - "the completion moved the write pointer to slot B") - ring_armed_for(b, "slot B after the frame") + local n_a, n_b, bad, ia, ib, ibad = ring_slot_counts(a, b) + ring_check(n_b == 15 and n_a == RX_DESC - 15 and bad == 0, + string.format("the 15 consumed descriptors moved to slot B and the %d untouched ones kept slot A (A=%d[%s] B=%d[%s] bad=%d[%s])", RX_DESC - 15, + n_a, ia, n_b, ib, bad, ibad)) + ring_check(prog:read_u32(RX_RING_ARMED) == 0, + "the next descriptor is still the one in slot A") ring_vram0 = read_vram_head(32) -- Now the impossible frame, into the slot the ring is armed on (B). local big, blen = oversize_frame() @@ -1214,9 +1243,12 @@ local function ring_step() local after = read_vram_head(32) ring_check(ring_vram0 ~= nil and after == ring_vram0, "the screen did not change: the oversize frame was not blitted") + local r_a, r_b, r_bad = ring_slot_counts(a, b) + ring_check(r_b == 16 and r_a == RX_DESC - 16 and r_bad == 0, + string.format("the rejected descriptor moved to slot B with the consumed ones (A=%d B=%d bad=%d)", + r_a, r_b, r_bad)) ring_check(prog:read_u32(RX_RING_ARMED) == 0, - "the rejection re-armed the ring on slot A") - ring_armed_for(a, "slot A after the rejection") + "the next descriptor is still the untouched one in slot A") ring_check(prog:read_u32(RX_RING_DROPS) == 0, "no frame was dropped for want of descriptors") -- Phase 3: one datagram the chip flagged bad (FCS). The chip gives @@ -1254,9 +1286,10 @@ local function ring_step() "the FCS error abandoned its frame: nothing is left in progress") ring_check(prog:read_u32(RX_OK) == 1, "RX_OK did not move: the errored datagram painted nothing") - ring_check(prog:read_u32(RX_RING_ARMED) == 1, - "the FCS error re-armed the ring on slot B") - ring_armed_for(b, "slot B after the FCS error") + local f_a, f_b, f_bad = ring_slot_counts(a, b) + ring_check(f_b == 17 and f_a == RX_DESC - 17 and f_bad == 0, + string.format("the errored datagram's descriptor was released too (A=%d B=%d bad=%d)", + f_a, f_b, f_bad)) local after = read_vram_head(32) ring_check(ring_vram0 ~= nil and after == ring_vram0, "the screen did not change: the errored datagram was not blitted") diff --git a/harness/udpstream/build-stream.sh b/harness/udpstream/build-stream.sh index 50cdbc0..41ee8b6 100755 --- a/harness/udpstream/build-stream.sh +++ b/harness/udpstream/build-stream.sh @@ -9,8 +9,8 @@ # run -- see harness/c-hello/README.md). # # POOL_BYTES is compile-time: the script writes poolcfg.h from -# STREAM_POOL_BYTES (default 249856 = 244 KB: two 24 KB slots + 160 KB -# framebuffer + 32 KB dictionary + 4 KB slack). +# STREAM_POOL_BYTES (default 249856 = 244 KB: two 48 KB slots, 32 KB +# dictionary, and the LANCE block's fixed 4 KB tail). # # Outputs: build/vidstream.bin (raw .text, what vidstream.lua loads), # build/vidstream.coff, vidstream.map, vidstream.lst, entry.lst, diff --git a/harness/udpstream/vidstream.c b/harness/udpstream/vidstream.c index 847b505..830eca4 100644 --- a/harness/udpstream/vidstream.c +++ b/harness/udpstream/vidstream.c @@ -24,10 +24,11 @@ * * Pool layout inside the single allocation (POOL_BYTES = 249856 = 244 KB): * - * +0x00000 frame slot A 24576 bytes (16 x 1526 B RX buffers) - * +0x06000 frame slot B 24576 bytes (16 x 1526 B RX buffers) - * +0x0C000 dictionary 32768 bytes (256 x 128 B, Task 3) - * +0x14000 framebuffer 163840 bytes (scheme A reserve; see below) + * +0x00000 frame slot A 49152 bytes (32 x 1526 B RX buffers) + * +x0C000 frame slot B 49152 bytes (32 x 1526 B RX buffers) + * +0x18000 dictionary 32768 bytes (256 x 128 B, Task 3) + * +0x20000 slack 114688 bytes (unused; the LANCE block's + * fixed 0x3C000 tail follows) * +0x3C000 LANCE block 4096 bytes (init block, TX ring, status * frame, RX ring) * @@ -35,8 +36,10 @@ * plan fixes and plan 2's Task 4 consumes. The 160 KB framebuffer reserve * follows it: this decoder paints straight from the RX fragments (there is no * per-frame CPU copy, spec 8.1) and never touches that region, so it is slack - * today, but the layout adds up to exactly POOL_BYTES -- 24576 + 24576 + 32768 - * + 163840 + 4096 = 249856. dict_slack_check below pins the part Task 3 + * today, but the layout adds up to exactly POOL_BYTES -- 49152 + 49152 + 32768 + * + 114688 + 4096 = 249856. The pool size did NOT have to grow for the deeper + * ring: the 160 KB framebuffer reserve was unused slack, and 114,688 B of it + * is still free between the dictionary and the LANCE block. dict_slack_check below pins the part Task 3 * depends on (the slot pair and the 32 KB dictionary); the LANCE block's * offsets at 0x3C000 were fixed before it and are not re-checked here. Had * the dictionary gone after the framebuffer, the plan's base would not be @@ -46,8 +49,8 @@ * (lance_start() explains why the offset alone is not enough), and the status * frame sits right after the 8 TX descriptors, the RX ring after that. * - * A slot is 16 packet buffers of RXBUFLEN = 1526 bytes (16 x 1526 = 24416, so - * it fits 24576 with 160 bytes to spare). The chip writes ONE Ethernet frame + * A slot is RX_DESC packet buffers of RXBUFLEN = 1526 bytes (32 x 1526 = + * 48832, so it fits 49152 with 320 bytes to spare). The chip writes ONE Ethernet frame * per buffer -- 14-byte header, the VS frame's chunk, and the FCS it appends -- * so a VS frame arrives as a series of datagrams in ring order, and the frame's * bytes are NOT contiguous in the slot: they are 14 bytes into each buffer, @@ -152,13 +155,19 @@ static unsigned int decode_worst; /* worst since the last status sent */ #define SENT_VALUE 0xA5C3 /* - * The two frame slots. SLOT_BYTES is the plan's 24 KB: a 128-row strip frame - * (128 x 160 B + 16 B header = 20,496 B = 15 packets) fits, and so does any - * smaller strip. Slot addresses are bit addresses, because that is what a + * The two frame slots. SLOT_BYTES holds a whole RX ring: 32 descriptors x + * RXBUFLEN (1526) = 48,832 B, rounded up. It also still holds a 128-row strip + * frame (128 x 160 B + 16 B header = 20,496 B = 15 packets). + * + * 32, not 16, because 16 could not absorb the tile dictionary: it arrives as + * 23 back-to-back datagrams (one TILES_DICT_CHUNK each), and with a 16-deep + * ring the chip dropped the tail while the consumer was still copying -- the + * measured zero-gap upload stalled at 24,616 of 32,768 B. The heap has + * megabytes free; the ring, not the memory, was the limit. Slot addresses are bit addresses, because that is what a * pointer holds in this toolchain (see harness/c-hello/README.md: "pointers * are bit addresses"). */ -#define SLOT_BYTES 24576 +#define SLOT_BYTES 49152 /* * Plan 2's Task 3: the tile dictionary. @@ -270,9 +279,16 @@ char tile_grid_check[(TILES_SCREEN == FP_TILES_SCREEN) ? 1 : -1]; * 1514-byte Ethernet frame plus margin, the figure lance_cmd.c uses on the * unit: a datagram from vidsrc.py is 14 + 1450 = 1464 bytes, the chip appends * the 4-byte FCS, writes it after the frame and reports the total in RMD3. - * RX_DESC * RXBUFLEN = 24416 bytes, which fits a 24576-byte slot. + * RX_DESC * RXBUFLEN = 48832 bytes, which fits a 49152-byte slot. + * + * The depth is the burst size the protocol actually uses: a whole 32 KB + * dictionary upload is 23 datagrams (MAX_BODY 1448), so 32 descriptors let the + * chip take the entire upload before the consumer has copied the first chunk. + * It is not a substitute for the sender pacing -- any ring can be outrun by a + * longer burst -- but it removes the accidental failure of the one burst the + * protocol defines. */ -#define RX_DESC 16 +#define RX_DESC 32 #define RXBUFLEN 1526 #define RMD1_OWN 0x8000 /* the chip owns the descriptor */ @@ -352,9 +368,10 @@ static unsigned int drops_total; /* CSR0 MISS windows, lower bound on misses */ /* * Task 7 receive state. * - * rx_armed the slot the ring's 16 descriptors point into (0 = A, 1 = B). - * Only ever written together with a full re-arm, so a descriptor - * never points at a slot the payload is not expecting. + * rx_slot_of[] which slot each descriptor points into (0 = A, 1 = B). + * Per descriptor, so descriptors can be recycled one at a time + * and nothing the chip has already delivered is re-pointed + * unread (see rx_release()). * rx_pos the next descriptor the consumer expects. The chip fills * descriptors strictly in ring order (am79c90.cpp's * recv_complete_cb advances m_rx_ring_pos by one per packet and @@ -379,10 +396,21 @@ static unsigned int drops_total; /* CSR0 MISS windows, lower bound on misses */ * for them -- so without this cell a body-corrupting packet would be invisible * to both crc_err and drops. Each one abandons the frame it belongs to. */ -static unsigned int rx_armed; static unsigned int rx_pos; static unsigned int rx_acc; static unsigned int rx_nfrag; +/* Which slot each descriptor currently points into. Per descriptor, not one + * global "armed" slot: a burst is handled by releasing descriptors one at a + * time as their data is consumed, so a descriptor the chip has already filled + * is never re-pointed behind the consumer's back. */ +static unsigned int rx_slot_of[RX_DESC]; +/* Descriptors consumed for the frame being assembled -- their data must stay + * put until the frame is parsed or abandoned, so they are released together + * at the end. */ +static unsigned int rx_hold[RX_DESC]; +static unsigned int rx_nhold; +/* The slot the first fragment of the current frame landed in (RX_SLOT). */ +static unsigned int rx_frame_slot; static unsigned int rx_frag_off[RX_DESC]; static unsigned int rx_frag_ptr[RX_DESC]; static unsigned int rx_frag_len[RX_DESC]; @@ -1058,34 +1086,54 @@ static unsigned int slot_base[2]; * descriptor completed by anything but the chip reads as a zero-length packet * rather than as the previous frame's length. */ +static void rx_arm_one(i, slot) +unsigned int i, slot; +{ + unsigned int cb; + + cb = (slot_base[slot] >> 3) + i * RXBUFLEN; + /* OWN clear, with the new high byte: a fetch here is a MISS. */ + w16(lance_rxring + i * 64 + 16, (cb >> 16) & 0xFF); + w16(lance_rxring + i * 64 + 0, cb & 0xFFFF); + w16(lance_rxring + i * 64 + 32, (0x10000 - RXBUFLEN) & 0xFFFF); + w16(lance_rxring + i * 64 + 48, 0); + /* Ownership last, over a complete descriptor. */ + w16(lance_rxring + i * 64 + 16, RMD1_OWN | ((cb >> 16) & 0xFF)); + rx_slot_of[i] = slot; +} + static void rx_arm(slot) unsigned int slot; { - unsigned int i, cb, base; + unsigned int i; - base = slot_base[slot]; for (i = 0; i < RX_DESC; i++) { - cb = (base >> 3) + i * RXBUFLEN; - /* OWN clear, with the new high byte: a fetch here is a MISS. */ - w16(lance_rxring + i * 64 + 16, (cb >> 16) & 0xFF); - w16(lance_rxring + i * 64 + 0, cb & 0xFFFF); - w16(lance_rxring + i * 64 + 32, (0x10000 - RXBUFLEN) & 0xFFFF); - w16(lance_rxring + i * 64 + 48, 0); - /* Ownership last, over a complete descriptor. */ - w16(lance_rxring + i * 64 + 16, - RMD1_OWN | ((cb >> 16) & 0xFF)); + rx_arm_one(i, slot); } } /* - * rx_switch() -- arm the other slot. Called at the end of a frame, before the - * picture is drawn, so the chip can fill N+1 while we paint N and can never - * write into the slot being read (spec 8.1's overlap). + * rx_release() -- give the frame's descriptors back, pointing each at the OTHER + * slot (spec 8.1's overlap: the buffer the chip next writes must not be one the + * consumer is still reading). + * + * This replaces a per-frame "switch slots and re-arm all 16". That flip was + * the burst bug: an all-16 re-arm re-points descriptors the chip had ALREADY + * completed but the consumer had not yet read, so their frames were silently + * lost -- measured on a 23-datagram dictionary upload: the chip delivered all + * 23 (LANCE DMA log, 1.13 ms apart) and only 4 reached these counters. Holding + * and releasing each descriptor individually means nothing the chip delivered + * is ever re-pointed unread, and the ring recycles as fast as the consumer. */ -static void rx_switch() +static void rx_release() { - rx_armed = 1 - rx_armed; - rx_arm(rx_armed); + unsigned int k, i; + + for (k = 0; k < rx_nhold; k++) { + i = rx_hold[k]; + rx_arm_one(i, 1 - rx_slot_of[i]); + } + rx_nhold = 0; } /* @@ -1108,11 +1156,14 @@ static void rx_dgram(i) unsigned int i; { unsigned int rmd1, mlen, len, src, plen, total, lim, slot; + unsigned int accepted, refused; fp_header h; + accepted = 0; + refused = 0; rmd1 = r16(lance_rxring + i * 64 + 16); mlen = r16(lance_rxring + i * 64 + 48) & 0x0FFF; - src = slot_base[rx_armed] + i * RXBUFLEN * 8 + ETH_HDR_LEN * 8; + src = slot_base[rx_slot_of[i]] + i * RXBUFLEN * 8 + ETH_HDR_LEN * 8; len = 0; if (rmd1 & RMD1_ERR) { /* @@ -1145,7 +1196,7 @@ unsigned int i; *(unsigned int *)RX_FCS_DROP + 1; rx_acc = 0; rx_nfrag = 0; - rx_switch(); + rx_release(); } return; } @@ -1169,6 +1220,9 @@ unsigned int i; } } + if (rx_nfrag == 0) { + rx_frame_slot = rx_slot_of[i]; /* what RX_SLOT reports */ + } rx_frag_off[rx_nfrag] = rx_acc; rx_frag_ptr[rx_nfrag] = src; rx_frag_len[rx_nfrag] = len; @@ -1192,17 +1246,17 @@ unsigned int i; *(unsigned int *)RX_BAD = *(unsigned int *)RX_BAD + 1; rx_acc = 0; rx_nfrag = 0; - rx_switch(); + rx_release(); return; } if (rx_acc < total) { return; /* still arriving */ } - /* The frame is complete. Hand the OTHER slot to the chip first, so - * nothing it receives from now on lands in the picture below. */ - slot = rx_armed; - rx_switch(); + /* The frame is complete. Its descriptors stay held -- so the chip + * cannot overwrite the picture while it is parsed and painted -- and are + * released just below, each into the other slot. */ + slot = rx_frame_slot; /* * Task 4 finding (b): the length handed to fp_parse() is clamped to the @@ -1219,9 +1273,7 @@ unsigned int i; *(unsigned int *)RX_LAST_LEN = h.payload_len; *(unsigned int *)RX_SLOT = slot; paint(&h); - /* Published after the paint: a reader that sees RX_OK move - * sees a picture that is already on the screen. */ - *(unsigned int *)RX_OK = *(unsigned int *)RX_OK + 1; + accepted = 1; } else if (h.kind == FP_KIND_DICT_CHUNK) { /* A chunk is not a picture: it is counted by DICT_BYTES * and DICT_BAD, never by RX_OK. */ @@ -1239,11 +1291,9 @@ unsigned int i; if (paint_tiles(&h)) { *(unsigned int *)RX_LAST_LEN = h.payload_len; *(unsigned int *)RX_SLOT = slot; - *(unsigned int *)RX_OK = - *(unsigned int *)RX_OK + 1; + accepted = 1; } else { - tiles_drop = tiles_drop + 1; - *(unsigned int *)TILES_DROP = tiles_drop; + refused = 1; } } else if (h.kind == FP_KIND_TILES_DELTA) { /* @@ -1255,11 +1305,9 @@ unsigned int i; if (paint_delta(&h)) { *(unsigned int *)RX_LAST_LEN = h.payload_len; *(unsigned int *)RX_SLOT = slot; - *(unsigned int *)RX_OK = - *(unsigned int *)RX_OK + 1; + accepted = 1; } else { - tiles_drop = tiles_drop + 1; - *(unsigned int *)TILES_DROP = tiles_drop; + refused = 1; } } else { /* A well-formed header whose kind nothing here dispatches @@ -1272,6 +1320,23 @@ unsigned int i; } rx_acc = 0; rx_nfrag = 0; + /* Released only now: every byte this frame needed has been read. */ + rx_release(); + /* + * Published after the release, not inside the dispatch. RX_OK is the + * "frame done" signal a reader polls, and it must not become visible + * while rx_release() is still rewriting descriptors: a sample taken then + * catches a descriptor mid-update (measured: slot B's high byte with + * slot A's low word). A reader that sees RX_OK move now sees a painted + * picture AND a consistent ring. + */ + if (refused) { + tiles_drop = tiles_drop + 1; + *(unsigned int *)TILES_DROP = tiles_drop; + } + if (accepted) { + *(unsigned int *)RX_OK = *(unsigned int *)RX_OK + 1; + } } /* @@ -1300,18 +1365,20 @@ static void rx_poll() } rx_pos = (i + 1) & (RX_DESC - 1); *(unsigned int *)RX_DGRAM = *(unsigned int *)RX_DGRAM + 1; + rx_hold[rx_nhold] = i; + rx_nhold = rx_nhold + 1; rx_dgram(i); if (rx_nfrag == RX_DESC && rx_acc != 0) { *(unsigned int *)RX_RING_DROPS = *(unsigned int *)RX_RING_DROPS + 1; rx_acc = 0; rx_nfrag = 0; - rx_switch(); + rx_release(); } } *(unsigned int *)RX_RING_POS = rx_pos; *(unsigned int *)RX_RING_ACC = rx_acc; - *(unsigned int *)RX_RING_ARMED = rx_armed; + *(unsigned int *)RX_RING_ARMED = rx_slot_of[rx_pos]; } /* @@ -1375,7 +1442,7 @@ unsigned int ptr; w16(init + 7 * 16, 0); cb = rxring >> 3; /* BYTE address of the RX ring */ w16(init + 8 * 16, cb & 0xFFFF); - w16(init + 9 * 16, ((cb >> 16) & 0xFF) | (4 << 13)); /* 16 desc */ + w16(init + 9 * 16, ((cb >> 16) & 0xFF) | (5 << 13)); /* 32 desc */ cb = ring >> 3; /* BYTE address of the ring */ w16(init + 10 * 16, cb & 0xFFFF); w16(init + 11 * 16, ((cb >> 16) & 0xFF) | (3 << 13)); @@ -1396,8 +1463,8 @@ unsigned int ptr; /* The RX ring, armed for slot A: this is the state the first packet * finds, and it is what the MAME ring check asserts. */ - rx_armed = 0; - rx_arm(rx_armed); + rx_nhold = 0; + rx_arm(0); *(unsigned int *)RX_RING_BASE = rxring; lance_csr_w(1, (init >> 3) & 0xFFFF); @@ -1702,10 +1769,11 @@ int main() status_seq = 0; status_seen = 0; drops_total = 0; - rx_armed = 0; rx_pos = 0; rx_acc = 0; rx_nfrag = 0; + rx_nhold = 0; + rx_frame_slot = 0; vid_dint_(); lance_ok = lance_start(ptr);