devices/cpu/powerpc/ppccom.cpp: (PPC4xx) Add configurable external serial clock and use actual system clock value for serial timer #10588
Conversation
987123879113
changed the title
src/devices/cpu/powerpc/ppccom.cpp
Outdated
| m_serial_clock = std::min(c_serial_clock != 0 ? c_serial_clock : 3686400, m_system_clock / 2); // serial clock can be at most half the system clock | ||
|
|
There was a problem hiding this comment.
Would it be better to throw a fatal error or something if the serial clock frequency is too high? In reality, if you feed it a frequency too high you’ll get aliasing from the synchronisation circuit so your clock intervals will be uneven – you won’t get a neat half system clock rate.
There was a problem hiding this comment.
I considered a fatal error here initially but I wasn't sure if it'd be ok. I'll change it. It'd be easier and safer to catch and fix bad clocks during testing.
There was a problem hiding this comment.
Pushed the change.
Tested it with a large clock and acceptable clock and it's working as expected, but throwing a fatal error in device_start throws a stack trace in addition to the fatal error message fyi. I tested another device's device_start and it does the same thing there, and tested in another place that wasn't a device_start and it errors without the stack trace. I don't know if this is expected behavior or not but just wanted to let you know.
Fatal error: PPC: serial clock (117964800) must not be more than half of the system clock (32000000)
-----------------------------------------------------
Exception at EIP=00007ff788b741c0 (not found): ACCESS VIOLATION
While attempting to read memory at 0000000000000090
-----------------------------------------------------
RAX=00007ff78bf47f50 RBX=0000000000000000 RCX=0000000000000000 RDX=000001eed8c10000
RSI=000001eedb2a5b30 RDI=000001eedd7742f0 RBP=000000e164dfa0c0 RSP=000000e164df9b40
R8=000001eed8ba2280 R9=0000000000000001 R10=0000000000000003 R11=000000e164df9bb0
R12=000000e164df9e70 R13=00007ff78be02b50 R14=000000e164df9f60 R15=000000e164df9f80
-----------------------------------------------------
Stack crawl:
000000e164df9bc0: 00007ff788b741c0 (not found)
000000e164df9c00: 00007ff788fccc43 (not found)
000000e164df9c40: 00007ff78909a636 (not found)
000000e164df9dc0: 00007ff7890a356a (not found)
000000e164dff250: 00007ff78963ab96 (not found)
000000e164dff640: 00007ff7896d4c76 (not found)
000000e164dff950: 00007ff7896d528a (not found)
000000e164dff9b0: 00007ff7896352f7 (not found)
000000e164dffd80: 00007ff78b88ae3e (not found)
000000e164dffe50: 00007ff7886513c1 (not found)
000000e164dffe80: 00007ff7886514f6 (not found)
000000e164dffeb0: 00007fff9c84244d (BaseThreadInitThunk+0x001d)
000000e164dfff30: 00007fff9ea8dfb8 (RtlUserThreadStart+0x0028)
There was a problem hiding this comment.
Fatal error handler is probably making an assumption about the running_machine being set up. I don’t care too much – anything that throws a fatal error during start should be fixed before it ends up in a release.
…rial clock and use actual system clock value for serial timer
987123879113
force-pushed
the
ppc4xx_fix_serial_clock
branch
from
7fdc22b
to
572c583
Compare
As per the PPC403 CPU manual, SerClk is supposed to be an external clock and can only be up to half of SysClk, and SysClk should be the actual system clock speed.
The hardcoded 33333333 (system clock) and 3686400 (serial clock) constants have existed for longer than I can see in the Git history (pre-0.121) so I don't have more context for those specific values.
https://github.com/mamedev/mame/blame/7b77f1218624ea26dbb2efd85a19f795f5d4e02e/src/emu/cpu/powerpc/ppc403.c#L625-L629
I tested with a few random Konami Hornet, Konami Cobra, and Konami Firebeat games and everything seems to be working as well as before, and a fatal error with Hornet games when frequently polling for inputs on the JVS I/O has been fixed (commented in code). The fatal error does not affect any games currently but will occur when a JVS I/O device is hooked up as required for 4 player inputs in NBA Play By Play.