playmark.cpp: Dumped and added sound PIC for "Hard Times" [Caps0ff, Tailsnic Retroworks]

[clawgrip]

Removed the MACHINE_NO_SOUND flag on both "Hard Times" sets, as they sound right now

[cuavas]

As with the other PR, the size of the dump doesn’t seem right at all. Program memory can only be whole number of words, and a PIC16C57 has 2048 words of program memory.

[Quench0]

The "overdump" half contains the PICs configuration words though it's one redundant byte short - this binary format is somewhat standard with some universal programmers but not all per my comment in the previous playmark PR.

[ghost]

yeah, there's very little in terms of standard with this kind of thing, we've had it with other Caps0ff dumps as well as those from other people.

Every programmer seems to have developed it's own 'standard' on where to store the config bytes etc. It's a mess.

Given the lack of true standard I think we'd be better off just using the binary data and including the extra info in the source where we have it.

[clawgrip]

I've updated the PR to load just the real internal ROM (trimming the ROM file).
If someone can make sense of the additional data present on the original dump, please edit this PR or open another PR after it (the programmer is a BP Micro).

[cuavas]

Is this just based on what you got from Caps0ff? It won’t be any use if it was read after deprotecting the chip. The configuration word is necessarily erased to reset the code protect bits. In the erased state, the watchdog timer will be enabled and the oscillator will be set to RC mode.

Do you have a board? Can you see what the OSC1/OSC2 pins are actually connected to?

[Quench0]

I've run Capsoff's PIC code, it frequently resets the watchdog timer so the watchdog timer should be enabled in the configuration word.

I actually have a Hard Times board here (running Hot Mind code). The PIC configuration word should be:
WDT: Enabled
Oscillator Mode: HS (High Speed since it's the only mode supporting the 24MHz/2 clock hooked up).

The remarked line at 1261 should remain remarked. PIC port A is not wired for OKI banking on this board, it's already hooked up through PIC port C in the driver as it is on the board.

[cuavas]

I've run Capsoff's PIC code, it frequently resets the watchdog timer so the watchdog timer should be enabled in the configuration word.

That doesn’t actually guarantee that the watchdog timer is enabled. It’s likely it’s enabled, but it could have been disabled either due to a bug where the watchdog timer won’t be reset soon enough in a corner case, or just by omission. You can only guarantee the watchdog timer was enabled if it depends on the watchdog timer to exit sleep.

Oscillator Mode: HS (High Speed since it's the only mode supporting the 24MHz/2 clock hooked up).

That’s not true – HS is the only mode that would support driving a crystal at 12 MHz, but a logic level clock input at any supported frequency will work in any of the three crystal modes (LP, XT or HS), as they only affect the crystal drive output. The Playmark PCBs supply a buffered logic level clock to the PIC’s clock input and leave the output disconnected. You can only guess which of the three crystal clock modes is used.

[clawgrip]

The remarked line at 1261 should remain remarked. PIC port A is not wired for OKI banking on
this board, it's already hooked up through PIC port C in the driver as it is on the board.

Readded, thanks!!!!

[Quench0]

That doesn’t actually guarantee that the watchdog timer is enabled. It’s likely it’s enabled, but it could have been disabled

Yes and what about the situation where the PIC might glitch/hang say from a power event and they wanted the watchdog timer to make sure the PIC resets back into operation?
We have two unprotected HEX dumps of Playmark PICs from "Big Twin" and "Excelsior". The configuration word for both of these has the watchdog timer enabled. These and the other decapped sets dumped in binary format clear the watchdog in their execution loop.

That’s not true – HS is the only mode that would support driving a crystal at 12 MHz, but a logic level clock input at any supported frequency will work in any of the three crystal modes (LP, XT or HS)

Again, "Big Twin" and "Excelsior" are set for HS mode. Under what circumstance is it advantageous for Playmark or anybody to select LP or XT when the clock is externally driven by a buffered 12MHz signal? If they change the board design to use a 12MHz crystal for the PIC oscillator, any part in stock set to LP or XT becomes unusable.

Now imagine an interstate friend of yours needs to program one of these PIC16C57 for this board. They only have one blank chip and it's OTP. How are you telling them to set the configuration word on what we know?

[ghost]

I tend to agree with @Quench0 here, a decision needs to be made though, it would be nice to have this in the next release.

[cuavas]

Yes and what about the situation where the PIC might glitch/hang say from a power event and they wanted the watchdog timer to make sure the PIC resets back into operation?

My point is, we’re still guessing. It’s likely that the watchdog timer was enabled, especially given it’s enabled for the other games, but there could have been some issue with this particular game that required the watchdog timer to be disabled.

Under what circumstance is it advantageous for Playmark or anybody to select LP or XT when the clock is externally driven by a buffered 12MHz signal?

Switching transients from the crystal driver are more severe in HS mode even if no crystal is connected. If you aren’t actually driving a crystal/resonator, avoiding HS mode will induce less noise on the power rails.

Now imagine an interstate friend of yours needs to program one of these PIC16C57 for this board. They only have one blank chip and it's OTP. How are you telling them to set the configuration word on what we know?

Well, considering the unprogrammed state for the watchdog timer enable bit enables the watchdog timer, the safe choice is to say to leave it enabled since it can be disabled later if an issue is discovered. For the oscillator mode, it doesn’t really matter as long as they choose one of the crystal modes.

[Quench0]

The change to line 1260 in the driver needs to be reverted. PIC port A is not wired anywhere on this board. I thought Clawgrip already reinstated the double slash comment prefix or was that in the other PR for this that he opened?
See line 1296 as a reference.

Switching transients from the crystal driver are more severe in HS mode even if no crystal is connected.

By how much? You don't know how well the circuitry is designed to cope with such conditions.

My point is, we’re still guessing.

A great deal of MAME driver code over the years was/is based on guesswork until proven otherwise. Somehow this point has become irrelevant here.

You've now sanctioned a binary file format for this code which serves no advantage to anyone that actually needs this code to maintain/repair their boards when HEX format is the standard file format for PIC code (Microchip dev and universal programmers).
HEX format being problematic/inconvenient to MAME is not a reason not to use it. Fix MAME, not force a file format that isn't standard.
Who's the primary target audience for these dumps anyway?
Preservationists not familiar with MAME let alone how to dig deep into the code are burdened with this binary file format that's now missing the ID bytes and a best guess config word.

[cuavas]

By how much? You don't know how well the circuitry is designed to cope with such conditions.

You keep moving the goal posts. You asked what the benefit of avoiding HS mode is, and I told you.

Every other driver in MAME uses binary dumps for PIC microcontrollers (e.g. handheld/hh_pic16.cpp, tumbleb.cpp, midway/vegas.cpp). This driver is the odd one out. MAME has always gone with binary formats.

Are you proposing putting another layer in front of ROM loading? How would you deal with canonicalising all the possible formats? How would you deal with checksumming sparse formats? A dodgy parser in the driver itself that makes big assumptions about what’s actually a flexible format is fragile.

[Quench0]

Every other driver in MAME uses binary dumps for PIC microcontrollers (e.g. handheld/hh_pic16.cpp, tumbleb.cpp, midway/vegas.cpp). This driver is the odd one out. MAME has always gone with binary formats.

That's because this driver was the very first to use the PIC16C5x micro-controller core. The other drivers using PIC binary file format came later and I had no involvement with them when the files were added as decaps.

Simply these files should be in HEX format so all preservationists can use them as is. And MAME should support HEX however the team sees fit. Mistakes of the past don't make them right now. We don't know how many boards have been trashed by people trying to repair them with faulty PLDs, PICs that couldn't work out how to program their replacements chips with MAME sanctioned binary file formats that go against industry standard formats for these devices.

We already discussed canonicalising in the other PR with @smf using sanitised files. It only take us a few minutes to check if someones new HEX/PLD dump is indeed different.

A dodgy parser in the driver itself that makes big assumptions about what’s actually a flexible format is fragile.

The converter does its basic job for the HEX dumps in this driver but I agree it's nowhere near good enough as a HEX parser.

The ID bytes and config word are part of the devices fuse map - config word critical to controlling how the device operates. Since these are now missing from the file dump, it's incomplete and it should be marked bad.
Right now I would just convert this dump to HEX like the others in this file, put the ID bytes back in and use a best case config word until proven otherwise marking it as a bad dump. We both know we'll likely never see an unprotected read of the chip.
I hand crafted the "luckboomh" and "hotmind" PIC HEX code files in this driver and ultimately that's how I set the config word noting my Hotmind game running on a Hard Times PCB was working perfectly fine with this hand crafted code file.