From d6d67b2a441d490fddaa459efbc73e74dd7a4a5f Mon Sep 17 00:00:00 2001 From: Ian Flanigan Date: Sun, 25 Jun 2023 12:57:03 +0200 Subject: [PATCH] Fix issue #187 were bank writing was not working correctly Before, `mmu.ts` would deactivate writing to the language card if `prewrite` was reset. However, it is totally possible to reset `prewrite` and leave writing enabled, as shown my Sather in _Understanding the Apple IIe_, table 5.5, p. 5-24. For example: ```assembly_x86 sta $c08a ; WRITE DISABLE; READ DISABLE lda $c08b ; PRE-WRITE set lda $c08b ; WRITE enabled sta $c08a ; PRE-WRITE reset; WRITE still enabled! lda $c08b ; PRE-WRITE set; WRITE still enabled! ``` would not work correctly because the last line would clear `_writebsr` before setting `_prewrite`, which is incorrect. Now, `_writebsr` is only set when `_prewrite` is set and thus only cleared when `writeSwitch` is false. This matches Table 5.5. --- js/mmu.ts | 4 +- test/js/mmu.test.ts | 90 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 test/js/mmu.test.ts diff --git a/js/mmu.ts b/js/mmu.ts index edc0791..6ac3454 100644 --- a/js/mmu.ts +++ b/js/mmu.ts @@ -684,7 +684,9 @@ export default class MMU implements Memory, Restorable { if (writeSwitch) { // 0xC081, 0xC083 if (readMode) { - this._writebsr = this._prewrite; + if (this._prewrite) { + this._writebsr = true; + } } this._prewrite = readMode; diff --git a/test/js/mmu.test.ts b/test/js/mmu.test.ts new file mode 100644 index 0000000..0fd2658 --- /dev/null +++ b/test/js/mmu.test.ts @@ -0,0 +1,90 @@ +import Apple2IO from 'js/apple2io'; +import MMU from '../../js/mmu'; +import CPU6502 from 'js/cpu6502'; +import { HiresPage, LoresPage, VideoModes, VideoPage } from 'js/videomodes'; +import Apple2eROM from '../../js/roms/system/apple2e'; +import { MemoryPages } from 'js/types'; + +function newFakeMemoryPages() { + return {} as unknown as MemoryPages; +} + +function newFakeVideoPage(): VideoPage { + const bank0Pages = newFakeMemoryPages(); + const bank1Pages = newFakeMemoryPages(); + return { + bank0() { + return bank0Pages; + }, + bank1() { + return bank1Pages; + }, + } as unknown as VideoPage; +} + +function newFakeLoresPage(): LoresPage { + return newFakeVideoPage() as unknown as LoresPage; +} + +function newFakeHiresPage(): HiresPage { + return newFakeVideoPage() as unknown as HiresPage; +} + +describe('MMU', () => { + const fakeVideoModes = {} as unknown as VideoModes; + const fakeCPU = {} as unknown as CPU6502; + const fakeLoResPage1 = newFakeLoresPage(); + const fakeLoResPage2 = newFakeLoresPage(); + const fakeHiResPage1 = newFakeHiresPage(); + const fakeHiResPage2 = newFakeHiresPage(); + const fakeApple2IO = {} as unknown as Apple2IO; + + it('is constructable', () => { + const mmu = new MMU(fakeCPU, fakeVideoModes, fakeLoResPage1, fakeLoResPage2, + fakeHiResPage1, fakeHiResPage2, fakeApple2IO, new Apple2eROM()); + expect(mmu).not.toBeNull(); + }); + + it('requires prewrite to write to bank1', () => { + const mmu = new MMU(fakeCPU, fakeVideoModes, fakeLoResPage1, fakeLoResPage2, + fakeHiResPage1, fakeHiResPage2, fakeApple2IO, new Apple2eROM()); + + // From https://github.com/whscullin/apple2js/issues/187 + // Action descriptions from Sather, Table 5.5, p. 5-24, UtAIIe: + mmu._access(0x8b); // WRTCOUNT = WRTCOUNT + 1, READ ENABLE + mmu._access(0x8b); // WRTCOUNT = WRTCOUNT + 1, READ ENABLE (write enabled) + mmu._access(0x89, 0x00); // WRTCOUNT = 0, READ DISABLE (write still enabled) + mmu._access(0x89); // WRTCOUNT = WRITCOUNT + 1, READ DISABLE (write still enabled) + mmu.write(0xd0, 0x00, 0xa1); + mmu._access(0x8b); // WRTCOUNT = WRTCOUNT + 1, READ ENABLE (write still enabled) + expect(mmu.read(0xd0, 0x00)).toBe(0xa1); + }); + + it('prewrite is reset on write access before write', () => { + const mmu = new MMU(fakeCPU, fakeVideoModes, fakeLoResPage1, fakeLoResPage2, + fakeHiResPage1, fakeHiResPage2, fakeApple2IO, new Apple2eROM()); + + // Action descriptions from Sather, Table 5.5, p. 5-24, UtAIIe: + mmu._access(0x89, 0x00); // WRTCOUNT = 0, READ DISABLE + mmu._access(0x8b); // WRTCOUNT = WRTCOUNT + 1, READ ENABLE (write not enabled yet) + mmu._access(0x8b, 0x00); // WRTCOUNT = 0, READ ENABLE (write still not enabled) + const oldValue = mmu.read(0xd0, 0x00); + mmu.write(0xd0, 0x00, 0xa1); // writes to the void + expect(mmu.read(0xd0, 0x00)).toBe(oldValue); // reads old value + }); + + + it('write stays active with overzealous switching', () => { + const mmu = new MMU(fakeCPU, fakeVideoModes, fakeLoResPage1, fakeLoResPage2, + fakeHiResPage1, fakeHiResPage2, fakeApple2IO, new Apple2eROM()); + + // Action descriptions from Sather, Table 5.5, p. 5-24, UtAIIe: + mmu._access(0x8b); // WRTCOUNT = WRTCOUNT + 1, READ ENABLE + mmu._access(0x8b); // WRTCOUNT = WRTCOUNT + 1, READ ENABLE (write enabled) + mmu._access(0x8b); // WRTCOUNT = WRTCOUNT + 1, READ ENABLE (write enabled) + mmu._access(0x8b); // WRTCOUNT = WRTCOUNT + 1, READ ENABLE (write enabled) + mmu.write(0xd0, 0x00, 0xa1); + mmu._access(0x8b); // WRTCOUNT = WRTCOUNT + 1, READ ENABLE (write still enabled) + expect(mmu.read(0xd0, 0x00)).toBe(0xa1); + }); +}); \ No newline at end of file