vsmjtria.cpp: Allow comms to actually work so that game can be coined up properly from either side. (#8245)

* Added NMI acknowledge, fixed coin input polarity, fixed gal B display, fixed flip screen behaviour.
* Added NVRAM clear using SW2.

Machines promoted to working
-------------------------
VS Mahjong Triangle [Angelo Salese, Vas Crabb]

src/mame/drivers/rmhaihai.cpp

@@ -21,6 +21,7 @@ TODO:
   protection (in rmhaisei the failure is more explicit, in rmhaijin it's
   deviously delayed to a later part of the game).
   In themj the checks are patched out, maybe it's a bootleg?
+  ETA: it uses IOX, which is shared with speedatk.cpp and srmp2.cpp as well.
 
 - some unknown reads and writes.
 

src/mame/drivers/vsmjtria.cpp

@@ -1,5 +1,5 @@
 // license:BSD-3-Clause
-// copyright-holders:
+// copyright-holders:Vas Crabb, Angelo Salese
 
 /*
 VS Mahjong Triangle (c) 1986 Dyna
@@ -10,13 +10,28 @@ Unmarked board with everything doubled up, to provide a versus experience with a
 CPU:    2x Z80
 RAM:    6x M5M5117P
 Sound:  2x AY-3-8910
-I/O:    2x 8255, 4x 8-dip banks, 1x single dip
+I/O:    2x 8255, 4x 8-dip banks, 1x single dip switch (at position 11m)
 OSC:    20MHz
 
+Notes:
+- Loosely based off rmhaihai.cpp.
+  Changes needed for merging both implementations seems too trivial to warrant a
+  driver merge, basically just the video HW, the CRTC (448x224 clocked at 20 MHz?)
+  and a few I/O bits looks very similar, the odd screen size is also a thing in srmp2.cpp
+  and probably other Seta HWs.
+- CPU seems a bit too slow when deciding about what tile to discard,
+  it also takes a bit too much to sync when a player takes a tile from the pond in
+  a VS play. It's most likely btanb unless a gameplay video proves otherwise;
+- Position 11m on PCB has a single slider switch.
+  It is not read by the CPUs at all, but instead zaps the nearby NVRAM contents if active.
+
 TODO:
-- coins don't work (set to free play to test for now)
+- Upper byte of I/O access seems to be some extra comms state machine
-- VS. feature communications aren't implemented
+  (i.e. it more or less mimic the commands that CPUs send between each other).
-- the hardware is based on the one in rmhaihai.cpp. Derive, merge or neither?
+  Maybe just a debug port that is read thru external HW?
+- Pinpoint how HW reads the three lead-filled marked as SW1 (position 1b).
+  It's located next to the key matrix, maybe just ext debugging?
+
 */
 
 #include "emu.h"
@@ -36,13 +51,16 @@ namespace {
 class vsmjtria_state : public driver_device
 {
 public:
-	vsmjtria_state(const machine_config &mconfig, device_type type, const char *tag) :
+	vsmjtria_state(const machine_config &mconfig, device_type type, const char *tag)
-		driver_device(mconfig, type, tag),
+		: driver_device(mconfig, type, tag)
-		m_cpu(*this, "cpu%u", 0U),
+		, m_cpu(*this, "cpu%u", 0U)
-		m_gfxdecode(*this, "gfxdecode%u", 0U),
+		, m_gfxdecode(*this, "gfxdecode%u", 0U)
-		m_colorram(*this, "colorram%u", 0U),
+		, m_colorram(*this, "colorram%u", 0U)
-		m_videoram(*this, "videoram%u", 0U),
+		, m_videoram(*this, "videoram%u", 0U)
-		m_key{ { *this, "P1_KEY%u", 0U }, { *this, "P2_KEY%u", 0U } }
+		, m_key{ { *this, "P1_KEY%u", 0U }, { *this, "P2_KEY%u", 0U } }
+		, m_nvram(*this, "nvram%u", 0U)
+		, m_nvram_data(*this, "nvram%u", 0U)
+		, m_sw2(*this, "SW2")
 	{ }
 
 	void vsmjtria(machine_config &config);
@@ -51,6 +69,7 @@ public:
 
 protected:
 	virtual void machine_start() override;
+	virtual void machine_reset() override;
 	virtual void video_start() override;
 
 private:
@@ -59,11 +78,15 @@ private:
 	required_shared_ptr_array<uint8_t, 2> m_colorram;
 	required_shared_ptr_array<uint8_t, 2> m_videoram;
 	required_ioport_array<5> m_key[2];
+	required_device_array<nvram_device, 2> m_nvram;
+	required_shared_ptr_array<u8, 2> m_nvram_data;
+	required_ioport m_sw2;
 
 	tilemap_t *m_bg_tilemap[2];
 	uint8_t m_keyboard_cmd[2];
 
-	template <uint8_t Which> void nmi_w(uint8_t data);
+	template <uint8_t Which> void nmi_set_w(uint8_t data);
+	template <uint8_t Which> void nmi_ack_w(uint8_t data);
 	template <uint8_t Which> void ctrl_w(uint8_t data);
 	template <uint8_t Which> uint8_t keyboard_r();
 	template <uint8_t Which> void keyboard_w(uint8_t data);
@@ -79,51 +102,9 @@ private:
 	void sub_io_map(address_map &map);
 };
 
-
+/*
-void vsmjtria_state::machine_start()
+ * Video section
-{
+ */
-	m_keyboard_cmd[0] = m_keyboard_cmd[1] = 0;
-
-	save_item(NAME(m_keyboard_cmd));
-}
-
-template <uint8_t Which>
-void vsmjtria_state::nmi_w(uint8_t data)
-{
-	m_cpu[Which]->pulse_input_line(INPUT_LINE_NMI, m_cpu[Which]->minimum_quantum_time());
-}
-
-template <uint8_t Which>
-void vsmjtria_state::ctrl_w(uint8_t data)
-{
-	flip_screen_set(data & 0x01);
-
-	// (data & 0x02) is switched on and off in service mode
-
-	machine().bookkeeping().coin_lockout_w(0, ~data & 0x04);
-	machine().bookkeeping().coin_counter_w(0, data & 0x08);
-}
-
-template <uint8_t Which>
-uint8_t vsmjtria_state::keyboard_r()
-{
-	uint8_t data = 0x00;
-
-	if (BIT(m_keyboard_cmd[Which], 0)) data |= m_key[Which][0]->read();
-	if (BIT(m_keyboard_cmd[Which], 1)) data |= m_key[Which][1]->read();
-	if (BIT(m_keyboard_cmd[Which], 2)) data |= m_key[Which][2]->read();
-	if (BIT(m_keyboard_cmd[Which], 3)) data |= m_key[Which][3]->read();
-	if (BIT(m_keyboard_cmd[Which], 4)) data |= m_key[Which][4]->read();
-
-	return data;
-}
-
-template <uint8_t Which>
-void vsmjtria_state::keyboard_w(uint8_t data)
-{
-	m_keyboard_cmd[Which] = data;
-}
-
 
 void vsmjtria_state::video_start()
 {
@@ -162,6 +143,57 @@ uint32_t vsmjtria_state::screen_update(screen_device &screen, bitmap_ind16 &bitm
 	return 0;
 }
 
+/*
+ * I/O
+ */
+
+template <uint8_t Which>
+void vsmjtria_state::nmi_set_w(uint8_t data)
+{
+	m_cpu[Which]->set_input_line(INPUT_LINE_NMI, ASSERT_LINE);
+}
+
+template <uint8_t Which>
+void vsmjtria_state::nmi_ack_w(uint8_t data)
+{
+	m_cpu[Which]->set_input_line(INPUT_LINE_NMI, CLEAR_LINE);
+}
+
+// TODO: flip screen and coin counter are actually never triggered, are they really connected for this HW?
+// (ctrl_w fn comes from rmhaihai.cpp assumption of being identical, it's most likely not present here)
+template <uint8_t Which>
+void vsmjtria_state::ctrl_w(uint8_t data)
+{
+//	flip_screen_set(data & 0x01);
+	m_bg_tilemap[Which]->set_flip((data & 1) ? (TILEMAP_FLIPY | TILEMAP_FLIPX) : 0);
+
+	// (data & 0x02) is switched on and off in service mode
+
+	machine().bookkeeping().coin_lockout_w(Which, ~data & 0x04);
+	machine().bookkeeping().coin_counter_w(Which, data & 0x08);
+}
+
+template <uint8_t Which>
+uint8_t vsmjtria_state::keyboard_r()
+{
+	uint8_t data = 0x00;
+
+	for (int key_n = 0; key_n < 5; key_n ++ )
+	{
+		if (BIT(m_keyboard_cmd[Which], key_n))
+			data |= m_key[Which][key_n]->read();
+	}
+
+	return data;
+}
+
+template <uint8_t Which>
+void vsmjtria_state::keyboard_w(uint8_t data)
+{
+	// bits 6-5 are always set when accessed, unknown purpose
+	// 0x60 (at boot only, not read back), then 0x61 -> 0x62 -> 0x64 -> 0x68 -> 0x70 and back to 0x61
+	m_keyboard_cmd[Which] = data;
+}
 
 void vsmjtria_state::main_prg_map(address_map &map)
 {
@@ -169,18 +201,20 @@ void vsmjtria_state::main_prg_map(address_map &map)
 	map(0xa000, 0xa7ff).ram().share("nvram0");
 	map(0xa800, 0xafff).ram().w(FUNC(vsmjtria_state::colorram_w<0>)).share(m_colorram[0]);
 	map(0xb000, 0xb7ff).ram().w(FUNC(vsmjtria_state::videoram_w<0>)).share(m_videoram[0]);
-	map(0xc000, 0xdfff).rom();
+	map(0xc000, 0xdfff).rom().region("cpu0", 0xa000);
 	map(0xe000, 0xffff).rom();
 }
 
 void vsmjtria_state::main_io_map(address_map &map)
 {
-	map(0x8000, 0x8003).rw("ppi0", FUNC(i8255_device::read), FUNC(i8255_device::write));
+	map.global_mask(0xff);
-	map(0x8020, 0x8020).r("ay0", FUNC(ay8910_device::data_r));
+	map(0x00, 0x03).rw("ppi0", FUNC(i8255_device::read), FUNC(i8255_device::write));
-	map(0x8020, 0x8021).w("ay0", FUNC(ay8910_device::address_data_w));
+	map(0x20, 0x20).r("ay0", FUNC(ay8910_device::data_r));
-	map(0x8060, 0x8060).w(FUNC(vsmjtria_state::ctrl_w<0>));
+	map(0x20, 0x21).w("ay0", FUNC(ay8910_device::address_data_w));
-	map(0x80c0, 0x80c0).w(FUNC(vsmjtria_state::nmi_w<1>));
+	map(0x60, 0x60).w(FUNC(vsmjtria_state::ctrl_w<0>));
-	map(0x80e0, 0x80e0).r("latch1", FUNC(generic_latch_8_device::read)).w("latch0", FUNC(generic_latch_8_device::write));
+	map(0xa0, 0xa0).w(FUNC(vsmjtria_state::nmi_ack_w<0>));
+	map(0xc0, 0xc0).w(FUNC(vsmjtria_state::nmi_set_w<1>));
+	map(0xe0, 0xe0).r("latch1", FUNC(generic_latch_8_device::read)).w("latch0", FUNC(generic_latch_8_device::write));
 }
 
 void vsmjtria_state::sub_prg_map(address_map &map)
@@ -189,31 +223,35 @@ void vsmjtria_state::sub_prg_map(address_map &map)
 	map(0xa000, 0xa7ff).ram().share("nvram1");
 	map(0xa800, 0xafff).ram().w(FUNC(vsmjtria_state::colorram_w<1>)).share(m_colorram[1]);
 	map(0xb000, 0xb7ff).ram().w(FUNC(vsmjtria_state::videoram_w<1>)).share(m_videoram[1]);
-	map(0xc000, 0xdfff).rom();
+	map(0xc000, 0xdfff).rom().region("cpu1", 0xa000);
 	map(0xe000, 0xffff).rom();
 }
 
 void vsmjtria_state::sub_io_map(address_map &map)
 {
-	map(0x8000, 0x8003).rw("ppi1", FUNC(i8255_device::read), FUNC(i8255_device::write));
+	map.global_mask(0xff);
-	map(0x8020, 0x8020).r("ay1", FUNC(ay8910_device::data_r));
+	map(0x00, 0x03).rw("ppi1", FUNC(i8255_device::read), FUNC(i8255_device::write));
-	map(0x8020, 0x8021).w("ay1", FUNC(ay8910_device::address_data_w));
+	map(0x20, 0x20).r("ay1", FUNC(ay8910_device::data_r));
-	map(0x8060, 0x8060).w(FUNC(vsmjtria_state::ctrl_w<1>));
+	map(0x20, 0x21).w("ay1", FUNC(ay8910_device::address_data_w));
-	map(0x80c0, 0x80c0).w(FUNC(vsmjtria_state::nmi_w<0>));
+	map(0x60, 0x60).w(FUNC(vsmjtria_state::ctrl_w<1>));
-	map(0x80e0, 0x80e0).r("latch0", FUNC(generic_latch_8_device::read)).w("latch1", FUNC(generic_latch_8_device::write));
+	map(0xa0, 0xa0).w(FUNC(vsmjtria_state::nmi_ack_w<1>));
+	map(0xc0, 0xc0).w(FUNC(vsmjtria_state::nmi_set_w<0>));
+	map(0xe0, 0xe0).r("latch0", FUNC(generic_latch_8_device::read)).w("latch1", FUNC(generic_latch_8_device::write));
 }
 
 
 static INPUT_PORTS_START( vsmjtria )
+	// TODO: deduplicate all inputs here
+	// either use a C-style macro or device-ify
 	PORT_START("P1_COIN")
-	PORT_BIT( 0x01, IP_ACTIVE_HIGH, IPT_COIN1 ) PORT_IMPULSE(1)
+	PORT_BIT( 0x01, IP_ACTIVE_LOW, IPT_COIN1 )
-	PORT_BIT( 0x1e, IP_ACTIVE_HIGH, IPT_UNUSED ) // stored at A38F along with bit 0 - seems to be vestigial, bit 2 tested at 02D1, but result discarded
+	PORT_BIT( 0x1e, IP_ACTIVE_LOW, IPT_UNKNOWN ) // stored at A38F along with bit 0 - seems to be vestigial, bit 2 tested at 02D1, but result discarded
-	PORT_BIT( 0xe0, IP_ACTIVE_HIGH, IPT_UNUSED ) // masked out after being read
+	PORT_BIT( 0xe0, IP_ACTIVE_LOW, IPT_UNUSED ) // masked out after being read
 
 	PORT_START("P2_COIN")
-	PORT_BIT( 0x01, IP_ACTIVE_HIGH, IPT_COIN2 ) PORT_IMPULSE(1)
+	PORT_BIT( 0x01, IP_ACTIVE_LOW, IPT_COIN2 )
-	PORT_BIT( 0x1e, IP_ACTIVE_HIGH, IPT_UNUSED ) // stored at A38F along with bit 0 - seems to be vestigial, bit 2 tested at 02D1, but result discarded
+	PORT_BIT( 0x1e, IP_ACTIVE_LOW, IPT_UNKNOWN ) // stored at A38F along with bit 0 - seems to be vestigial, bit 2 tested at 02D1, but result discarded
-	PORT_BIT( 0xe0, IP_ACTIVE_HIGH, IPT_UNUSED ) // masked out after being read
+	PORT_BIT( 0xe0, IP_ACTIVE_LOW, IPT_UNUSED ) // masked out after being read
 
 	PORT_START("P1_KEY0")
 	PORT_BIT( 0x01, IP_ACTIVE_HIGH, IPT_START1 ) PORT_NAME("P1 Single Start")
@@ -416,6 +454,13 @@ static INPUT_PORTS_START( vsmjtria )
 	PORT_DIPNAME( 0x80, 0x80, "Unknown 4-7" )
 	PORT_DIPSETTING(    0x80, DEF_STR( Off ) )
 	PORT_DIPSETTING(    0x00, DEF_STR( On ) )
+
+	// single slider switch, not read by CPU
+	// grounds NVRAM contents, simulated in machine_reset fn
+	PORT_START("SW2")
+	PORT_DIPNAME( 0x01, 0x00, "Clear NVRAM at boot" )
+	PORT_DIPSETTING(    0x00, DEF_STR( No ) )
+	PORT_DIPSETTING(    0x01, DEF_STR( Yes ) )
 INPUT_PORTS_END
 
 
@@ -438,10 +483,31 @@ static GFXDECODE_START( gfx_vsmjtria_sub )
 	GFXDECODE_ENTRY( "subgfx", 0, charlayout, 0, 32 )
 GFXDECODE_END
 
+void vsmjtria_state::machine_start()
+{
+	m_keyboard_cmd[0] = m_keyboard_cmd[1] = 0;
+
+	save_item(NAME(m_keyboard_cmd));
+}
+
+void vsmjtria_state::machine_reset()
+{
+	if (m_sw2->read() & 1)
+	{
+		// Note: there's no direct setter in nvram_device that directly flushes contents for this case scenario
+	    for (auto &nvram : m_nvram_data)
+			std::fill_n(&nvram[0], nvram.length(), 0);
+
+		logerror("machine_reset: flush NVRAM contents\n");
+	}
+}
+
 void vsmjtria_state::vsmjtria(machine_config &config)
 {
 	config.set_perfect_quantum("cpu0"); // crude way to make comms work - does it have FIFOs rather than latches?
 
+	// Both CPUs are LH0080A -> Z80A, nominally 4 MHz
+	// Clock also controls sound tempo, which at /4 sounds too fast already.
 	Z80(config, m_cpu[0], 20_MHz_XTAL / 5); // divider guessed
 	m_cpu[0]->set_addrmap(AS_PROGRAM, &vsmjtria_state::main_prg_map);
 	m_cpu[0]->set_addrmap(AS_IO, &vsmjtria_state::main_io_map);
@@ -452,8 +518,8 @@ void vsmjtria_state::vsmjtria(machine_config &config)
 	m_cpu[1]->set_addrmap(AS_IO, &vsmjtria_state::sub_io_map);
 	m_cpu[1]->set_vblank_int("rscreen", FUNC(vsmjtria_state::irq0_line_hold));
 
-	NVRAM(config, "nvram0", nvram_device::DEFAULT_ALL_0);
+	NVRAM(config, m_nvram[0], nvram_device::DEFAULT_ALL_0);
-	NVRAM(config, "nvram1", nvram_device::DEFAULT_ALL_0);
+	NVRAM(config, m_nvram[1], nvram_device::DEFAULT_ALL_0);
 
 	GENERIC_LATCH_8(config, "latch0");
 	GENERIC_LATCH_8(config, "latch1");
@@ -574,4 +640,4 @@ void vsmjtria_state::init_vsmjtria()
 } // Anonymous namespace
 
 
-GAME( 1986, vsmjtria, 0, vsmjtria, vsmjtria, vsmjtria_state, init_vsmjtria, ROT0, "Dyna", "VS Mahjong Triangle", MACHINE_NOT_WORKING | MACHINE_SUPPORTS_SAVE ) // puts Home Data in RAM? coins and CPU comms not working
+GAME( 1986, vsmjtria, 0, vsmjtria, vsmjtria, vsmjtria_state, init_vsmjtria, ROT0, "Dyna", "VS Mahjong Triangle", MACHINE_SUPPORTS_SAVE ) // puts Home Data in RAM?