nb1414m4.cpp : Fix "INSERT COIN" blinking start timing.

[sasuke-arcade]

ninjaemak and clones/kozure

• Correction of blinking timing of "INSERT COIN" (See https://imgur.com/a/i3FjjoB)

[angelosa]

Sounds unlikely that game has own internal frame counter, it's in my humble opinion more possible that the machine_config (again) has naive screen parameters hooked up that causes wrong timings.

[sasuke-arcade]

I guessed from "kozure" and "ninjemak" PCBs blinking timing, and when the "INSERT COIN" display message was called, I thought it was counting using the internal work memory of the MCU.
At least, when use frame counter of since starting emulation, blinking started was not the same as PCBs.
And, can you tell me what screen parameters of machine_config is?

[sasuke-arcade]

This counter is only for blinking "INSERT COIN", "ONE PLAYER ONLY" and "ONE OR TWO PLAYERS". The variable name may not be good. Should I change it?

[sasuke-arcade]

To make it easier to understand, I changed the name of m_frame_count to m_flickering_count.

[angelosa]

Blinking is usually derived from screen vblank timing, that's why I'm dubious about this internal state variable. Wrong timings are likely caused by incorrect machine_config parameters for screen:

void bigfghtr_state::bigfghtr(machine_config &config)
{
        [...]
	/* video hardware */
	screen_device &screen(SCREEN(config, "screen", SCREEN_TYPE_RASTER));
	screen.set_raw(XTAL(16'000'000)/2,531,12*8,(64-12)*8, 254, 1*8, 31*8); // guess, matches 59.3 Hz from reference - measured at 59.1358Hz

This is the better implementation of CRTC parameters, inputs are pixel_clock, htotal, hdisplay_start, hdisplay_end, vtotal, vdisplay_start, vdisplay_end.

void armedf_state::cclimbr2(machine_config &config)
{
       [...]
	/* video hardware */
	screen_device &screen(SCREEN(config, "screen", SCREEN_TYPE_RASTER));
	screen.set_refresh_hz(60);
	screen.set_vblank_time(ATTOSECONDS_IN_USEC(2500) /* not accurate */);
	screen.set_size(64*8, 32*8);
	screen.set_visarea(14*8, (64-14)*8-1, 2*8, 30*8-1 );

This instead is the legacy format. It doesn't really honor the screen timing at all and if SW relies on somehow tight vblank/vdisplay timings it causes all kinds of video and even input lag artifacts. When a driver sports this it is really a cargo cult caused by lack of info about h/vsync timings and imho should be marked as deprecated function off the bat.

So, in order to fix this for armed.cpp you can sub-routine the screen creation with the display resolution as parameters (because I truly guess all of these Nichibutsu games shares same CRTC discrete device).
For deriving the screen timings the canonical approximation is:

htotal = pixel_clock / hsync
vtotal = hsync / vsync

Scaled to Hz unit and rounded up to next integer. pixel_clock must be derived from available xtals from PCB, deriving the pixel_clock divider is a matter of common sense (i.e. htotal mustn't be smaller than hdisplay and it is unlikely that a xtal divider isn't a multiple of 2).
For armedf.cpp this translates to:

// pixel_clock cannot be /4 because 265,36 falls into the hdisplay area. Could be /3 but 353,82 hotal sounds too short.
htotal = (16/2 MHz) / (15,0735 kHz) = 8000000 / 15073,5 = 530,73 = 531
vtotal = (15,0735 kHz) / (59,1358 Hz) = 15073,5 / 59,1358 = 254,89 = 255

A good reference about how analog CRT actually works out is here, notice that h/vtotals are actually a collection of blanking/sync/overscan events that MAME doesn't currently take into account.

[sasuke-arcade]

Thank you for telling me so much in detail.
I understood what didn't know.

I thought that this blinking by NB1414M4 was expressed by writing a blank in VRAM, just like when displaying characters. The one of reason is that the display command is called every frame while it needs to blink.
And, all blinks that do not pass through the NB1414M4 appear to be represented by a VRAM rewrite by the software. (For example, "CONTINUE PLAY?" in kozure. The blinking timing of "1-UP" also controlled by software.)
For these reason, this hardware seems does not have a blinking function, and it seems to be an operation that simply writes characters or blanks alternately into VRAM every time it is called command 16 times.

This was the reason for using the counter, not the screen hardware timing.
Do you think there is a possibility?

[sasuke-arcade]

Another reason. At least, “INSERT COIN” blinking timing after power-on is completely the same between kozure and youma. This starts at the first (0 frame) of 32 frame period.
As in the current code, if it counts from 0 frame only when the command is called, timing will match exactly.
The NB1414M4 is probably only connected to VRAM simply, and I don't think it has any unique hardware to blink INSERT COIN. (I think it has not synchronization of any hardware timing.)

Comparison video is here :
https://imgur.com/a/i3FjjoB

[cuavas]

I'm not so sure that these fixes are correct, either. We're still relying on simulation for things that are not well-understood, and as far as I know it's all based on observing known games rather than actually testing hardware.

[sasuke-arcade]

I fixed the reviewed points and additional bugs.
There is nothing to commit now.

[sasuke-arcade]

Following are code change details.

galivan.cpp / armedf.cpp

• Tuned DAC gain compared to PCB.

armedf.cpp

• TILE_GET_INFO_MEMBER(ninjemak_get_tx_tile_info) : Fixed dusts in lower left (This is for hide custom chip communication).

galivan.h

• Fixed code indentation.

nb1414m4.cpp

• dma() : Fixed text attribute reference when erasing of blinking.
• kozure_score_msg() : When score is 0, the tens place is blank.
• insert_coin_msg(), credit_msg() : Changed the management of character blinking cycle.
• insert_coin_msg(), credit_msg() : If the in-game flag is on, "INSERT COIN" etc. is not displayed.
• _0200() : Get the in-game flag bit from sent at command.
• _0200() : If the same command is received continuously within one frame, it is not processed.
• _0200() : Removed duplicate bit operations to make code easier to read.
• _0e00() : Draw the score when the game over display bit is set.
• _0e00() : The flashing bit is no longer applied to game over.
• Added necessary member variables to the above.

[angelosa]

Again the flickering condition cannot be internal to the device, not even the tilemap VRAM is.
No matter the proof about being "more accurate" to the PCB ref, as I've mentioned before if the video params gets changed with the more accurate set_raw() function it's going to break again.

[sasuke-arcade]

The reason why the blinking timing is different in the previous source is because "screen().frame_number()" is used. I think the origin author needed only 32 frame cycle (It is comletely accurate) and this was used, but the start timing is not taken into account, and the blinking pattern changes with each reset. I think this should not be used. This cause is the use of parameters unique to MAME, so I can't solve this by changing machine_config.

The game main loop calls the NB1414M4 command every 1 vblank. I think that NB1414M4 is MCU (as this source code argument names indicate). MCU can count the times of command is called. And I think this counter is used to control blinking. I don't know why this is not possible. Also I don't have a way to improveme with machine_config without doing this. I think the incorrect parameter of machine_config is a completely different problem. I don't know what to do.

[angelosa]

If you need a latch you can synchronize with the "irq acknowledge" write handler and add a device setter that increments there.
That's for $7c00c / $7c00e in the common_map for armedf.cpp and i/o $86 / $87 for ninjemak_io_map in galivan.cpp. Note: other games may differ, also ofc you must be careful in not calling nb1414m4 device if it doesn't exist.
It's a wild guess, but both ports definitely have a video meaning since they are tied to the vblank handlers (at start and end), and nb1414m4 definitely sits there. Whatever is the actual "blitter trigger" and even if the port is really for irq ack is something that only a real PCB test can tell.

PS: the notes on top of nb1414m4.cpp are terribly outdated, and by current knowledge I'm pretty sure that's not a MCU.

1. Nihon Bussan wasn't known for having state-of-the-art (and expensive) protection chips, for example several of their jong PCBs have out of place random ROMs from other games where apparently they just hash the content out.
2. The protection isn't creative enough for needing a MCU in the first place, and indeed the only game that really uses one (Tatakae Big Fighter) never got bootlegged.

[cuavas]

To move things forward, can the less contentious parts of the change be moved to a separate pull request? I expect that if the sound balance and duplicate clear command check were moved to a separate pull request that could be merged.

[sasuke-arcade]

OK, I has separated code that was not pointed out in the reviews to #6016 #6017.

[angelosa]

This PR can now be reworked honoring the vblank_trigger() function.
Usage: just increment and pad an internal frame variable in there. Bonus points for using bool condition precalculated for a minor optimization.

[sasuke-arcade]

Sorry for the very slow response.
The fix for blinking seems like I need to learn more. Based on what you have taught me, I think carefully about the contents of the correction.
Also, is duplicate clear commands haves any problems? This is not discussing now, but the review is not resolved. Do I need to split it to another pull request?

[angelosa]

The duplicate is ok from our current understading of the chip, it is the frame stepping that needs to be done in the newly created setter.

[sasuke-arcade]

Thanks!
I has separated code to #6070.

[sasuke-arcade]

Thank you for merging.

[sasuke-arcade]

I am closing this ticket for reconsideration.