gridcomp/gridcomp.cpp: implement application ROMs#15193
gridcomp/gridcomp.cpp: implement application ROMs#15193vklachkov wants to merge 1 commit intomamedev:masterfrom
Conversation
| namespace { | ||
|
|
||
| #define I80130_TAG "osp" | ||
| #define NUMBER_OF_ROMS 4 |
There was a problem hiding this comment.
I think a constexpr within gridcomp_state would be more C++-y.
|
|
||
| // TODO: Handle 32KB flag at offset 2. | ||
|
|
||
| if (offset == 8) |
There was a problem hiding this comment.
Why didn't you put a specific handler at dfea8?
There was a problem hiding this comment.
It seems that all of this is implemented in a single PAL. That's why I decided to combine both the activator and the 32-kilobyte flag into a single function.
| m_active_slots[offset / 2] = data; | ||
| } | ||
|
|
||
| uint8_t gridcomp_state::app_rom_r(offs_t offset) |
There was a problem hiding this comment.
Is app_rom_r called often when running? Because there's a way to make it significantly more efficient (using banks and views) but it's a little more complicated. So does the system run from there or just cupy stuff to ram?
There was a problem hiding this comment.
This function is called very often. Depending on the ROM, the laptop may either execute the code directly or copy the contents into memory. But judging by my logs, this is a pretty "hot" function.
I don't like this implementation, it smells of some kind of reinventing the wheel. But I didn't have the energy to figure out how to do it more cleanly, so I decided to make the PR as is. If you think such code has no place in MAME -- please point me to some machines I could look at for inspiration, and I'll be ready to redo app_rom_r...
|
|
||
| // 4 slot selection registers are located at addresses: | ||
| // DFE0:8, DFE0:A, DFE0:C, and DFE0:E. | ||
| m_active_slots[offset / 2] = data; |
There was a problem hiding this comment.
What happens when writing at odd addresses? Is A0 ignored (what you implemented) or is the write ignored?
There was a problem hiding this comment.
I don't know. There's no mention of this in the CCPROM code, and not a single program I know of writes to odd addresses. I decided to ignore writes.
There was a problem hiding this comment.
I would suggest a logerror() or similar if it happens, so that you know if it ever comes up.
| if (!m_roms_enabled) | ||
| return 0xFF; | ||
|
|
||
| // The address space of the ROM is divided into 4 "windows". | ||
| const uint32_t window_size = 0x8000; | ||
|
|
||
| // Calculate the quarter number from which we are reading. | ||
| const size_t quarter = offset / window_size; | ||
|
|
||
| // And look up which ROM slot is mapped to this window. | ||
| const size_t slot = m_active_slots[quarter]; | ||
| const auto& rom = m_app_roms[slot]; | ||
|
|
||
| // It does not matter what is returned if the ROM is not present because | ||
| // CCPROM checks the 0xBB66 marker at offset 9FF0:0, | ||
| // and then also verifies the checksum and other things. | ||
| if (!rom->exists()) | ||
| return 0xFF; |
There was a problem hiding this comment.
Kind of trivial compared to fixing the performance issues, but please use lowercase hex digits in MAME code.
4 participants
Continuation of PR #15159.
Adds Application ROM support to the GRiD Compass II. Implemented based on the CCPROM code. In the code, I tried to explain the logic of this mechanism in detail.
What remains unresolved is a TODO for a flag when installing 32KiB ROMs. I don't know exactly what it is for. But everything seems to work, here are the screenshot:
Works on both 1129 and 1139.