Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apple/apple2e.cpp: update LC memory map when LC state changes #11996

Merged

Conversation

xotmatrix
Copy link
Contributor

It appears that during a reset, the expected and executed language card state changes do not also update the system memory map. This can cause a reset vector to fail when the language card RAM has been banked over ROM. The following software hangs when reset is asserted:

Merlin running from 32 Meg Hard Drive Image

BBC Basic running from Applecorn

A reset hang can be demonstrated from a clean boot by entering the following code into the monitor. It banks in the LC over ROM and waits for the user to press reset. If the LC bank is uninitialized and reset is pressed, it will hang in a cascade of BRK instructions.

*300: 2C 80 C0 4C 03 03 N 300G

The Apple //c will hang a little differently since it copies an IRQ/BRK vector into the LC RAM area when it boots. The interrupt service routine will attempt to jump to a ROM location but it isn't banked in, a BRK will be hit, and the routine will be triggered again.

I must stress that I don't fully understand how memory mapping works. I copied the pattern seen in parts of the code, consolidated it into a function lcrom_update(), and inserted calls to it wherever the language card state changed. These changes resolve the reset vector problems but someone wiser than me should review what I've done carefully. The ALTZP softswitch affects the language card area but I gather changes to m_altzp followed by a call to auxbank_update() bank AUX memory into the LC RAM area automatically (though it is not explicitly stated).

A true slot 0 language card in an early model Apple II does not change its state when reset so this has been left alone. According to Jeff Mazur there is no hardware reset of m_romswitch and the extended ROM code supports that so that has not been changed. I don't know enough about the CEC computers to comment and have tried to leave its LC mapping undisturbed.

Comment on lines +1611 to +1617
#if 0
printf("LC: new state %c%c dxxx=%04x altzp=%d (PC=%x)\n",
m_lcram ? 'R' : 'x',
m_lcwriteenable ? 'W' : 'x',
m_lcram2 ? 0x1000 : 0x0000,
m_altzp, m_maincpu->pc());
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid printf – it isn’t type-safe. Also try to avoid using the preprocessor to disable logging code like this as it will rot when it isn’t enabled. The utilities from "logmacro.h" should be used for compile-time configurable logging.

Copy link
Contributor Author

@xotmatrix xotmatrix Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not my code but good to know. I'll remove the logging in another PR coming in soon.

EDIT:

On second thought, there is a ton of such logging throughout /apple. Excising it all probably deserves its own PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modernizing the logging definitely should be a separate PR. You can see how the modern logging with the different channels works in e.g. apple/dafb.cpp. I'll check the rest of this PR in a moment.

@rb6502 rb6502 merged commit 534af53 into mamedev:master Feb 2, 2024
5 checks passed
Mokona pushed a commit to Mokona/mame that referenced this pull request Feb 28, 2024
stonedDiscord pushed a commit to stonedDiscord/mame that referenced this pull request Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants