From 2e0f4af6ee693fb6f40183bc60ef16db826c8780 Mon Sep 17 00:00:00 2001
From: Mike <mike.harris@gmail.com>
Date: Wed, 19 Jun 2019 16:55:46 -0700
Subject: [PATCH] Pia6821 improvements (#5254)

* 6821pia: fix method names in comments

* zwackery: Remove PIA port A z_mask setting.

This would only be relevant if the PIA was read while in output mode.
These are inputs, and only read in input mode, so z_mask is not relevant.

* thomson: move pia pullup resistors from z_mask to read handler.

* 6821pia: remove unused set_port_a_z_mask calls.

* dgn_beta: move pia pullup resistors from z_mask to read handler.

* mpu3: Don't invert the VFD power on reset line.

It doesn't go through an inverter. I believe this was done to work
around a bug in the PIA port A implementation.

* pia6821: remove m_port_a_z_mask variable.

* 6821pia: For port A reads, only use the DDR for reading pins unless.

Provide an override option when a device depends on slightly undefined
behavior of an external device driving the pins and changing the value
read.

One board seems to need this (coinmstr). I don't have the schematics,
but it seems it uses this to check its meter operation.
---
 src/devices/bus/a2bus/mouse.cpp   |  2 +-
 src/devices/bus/spectrum/opus.cpp |  2 +-
 src/devices/bus/ss50/mpc.cpp      |  2 +-
 src/devices/machine/6821pia.cpp   | 32 +++++++++++++------------------
 src/devices/machine/6821pia.h     | 13 +++++--------
 src/mame/drivers/coinmstr.cpp     |  2 +-
 src/mame/drivers/mpu3.cpp         |  2 +-
 src/mame/drivers/zwackery.cpp     | 10 ++--------
 src/mame/includes/coco.h          |  2 +-
 src/mame/includes/coco3.h         |  2 +-
 src/mame/machine/coco.cpp         | 13 +++----------
 src/mame/machine/coco3.cpp        |  4 ++--
 src/mame/machine/dgn_beta.cpp     | 14 ++++++--------
 src/mame/machine/thomson.cpp      | 30 +++++++++++------------------
 14 files changed, 49 insertions(+), 81 deletions(-)

diff --git a/src/devices/bus/a2bus/mouse.cpp b/src/devices/bus/a2bus/mouse.cpp
index a960c74e52c..817cbd1fd85 100644
--- a/src/devices/bus/a2bus/mouse.cpp
+++ b/src/devices/bus/a2bus/mouse.cpp
@@ -251,7 +251,7 @@ READ8_MEMBER(a2bus_mouse_device::mcu_port_a_r)
 
 WRITE8_MEMBER(a2bus_mouse_device::mcu_port_a_w)
 {
-	m_pia->set_a_input(data, ~mem_mask);
+	m_pia->set_a_input(data);
 }
 
 READ8_MEMBER(a2bus_mouse_device::mcu_port_b_r)
diff --git a/src/devices/bus/spectrum/opus.cpp b/src/devices/bus/spectrum/opus.cpp
index be73ef01e92..239560cd512 100644
--- a/src/devices/bus/spectrum/opus.cpp
+++ b/src/devices/bus/spectrum/opus.cpp
@@ -285,5 +285,5 @@ WRITE8_MEMBER(spectrum_opus_device::pia_out_b)
 
 WRITE_LINE_MEMBER(spectrum_opus_device::busy_w)
 {
-	m_pia->set_a_input(state << 6, 0xbf);
+	m_pia->set_a_input(state << 6);
 }
diff --git a/src/devices/bus/ss50/mpc.cpp b/src/devices/bus/ss50/mpc.cpp
index ba05d80e8e3..8563a289202 100644
--- a/src/devices/bus/ss50/mpc.cpp
+++ b/src/devices/bus/ss50/mpc.cpp
@@ -136,7 +136,7 @@ void ss50_mpc_device::device_start()
 
 WRITE_LINE_MEMBER(ss50_mpc_device::serial_input_w)
 {
-	m_pia->set_a_input(state << 7, 0x7f);
+	m_pia->set_a_input(state << 7);
 	m_loopback->in_w<0>(state);
 }
 
diff --git a/src/devices/machine/6821pia.cpp b/src/devices/machine/6821pia.cpp
index 4171b9c6f82..bee109d4d34 100644
--- a/src/devices/machine/6821pia.cpp
+++ b/src/devices/machine/6821pia.cpp
@@ -52,7 +52,7 @@ pia6821_device::pia6821_device(const machine_config &mconfig, const char *tag, d
 		m_cb2_handler(*this),
 		m_irqa_handler(*this),
 		m_irqb_handler(*this), m_in_a(0),
-		m_in_ca1(0), m_in_ca2(0), m_out_a(0), m_out_ca2(0), m_port_a_z_mask(0), m_ddr_a(0),
+		m_in_ca1(0), m_in_ca2(0), m_out_a(0), m_out_ca2(0), m_ddr_a(0),
 		m_ctl_a(0), m_irq_a1(0), m_irq_a2(0),
 		m_irq_a_state(0), m_in_b(0),
 		m_in_cb1(0), m_in_cb2(0), m_out_b(0), m_out_cb2(0), m_last_out_cb2_z(0), m_ddr_b(0),
@@ -90,7 +90,6 @@ void pia6821_device::device_start()
 	save_item(NAME(m_in_ca2));
 	save_item(NAME(m_out_a));
 	save_item(NAME(m_out_ca2));
-	save_item(NAME(m_port_a_z_mask));
 	save_item(NAME(m_ddr_a));
 	save_item(NAME(m_ctl_a));
 	save_item(NAME(m_irq_a1));
@@ -137,7 +136,6 @@ void pia6821_device::device_reset()
 	m_in_ca2 = true;
 	m_out_a = 0;
 	m_out_ca2 = 0;
-	m_port_a_z_mask = 0;
 	m_ddr_a = 0;
 	m_ctl_a = 0;
 	m_irq_a1 = 0;
@@ -226,8 +224,8 @@ uint8_t pia6821_device::get_in_a_value()
 		}
 		else
 		{
-			// mark all pins disconnected
-			m_port_a_z_mask = 0xff;
+			// assume pins are disconnected and simulate the internal pullups.
+			port_a_data = 0xff;
 
 			if (!m_logged_port_a_not_connected && (m_ddr_a != 0xff))
 			{
@@ -237,12 +235,12 @@ uint8_t pia6821_device::get_in_a_value()
 		}
 	}
 
-	// - connected pins are always read
-	// - disconnected pins read the output buffer in output mode
-	// - disconnected pins are HI in input mode
-	ret = (~m_port_a_z_mask             & port_a_data) |
-			( m_port_a_z_mask &  m_ddr_a & m_out_a) |
-			( m_port_a_z_mask & ~m_ddr_a);
+	// For port A, when the port is in output mode other devices can drive the
+	// pins too. If the device pulls the voltage on the lines enough,a read of
+	// the output register will show the external device value on the driven pins.
+	ret = (~m_ddr_a & port_a_data)  // input pins
+		| (m_ddr_a & m_out_a & ~m_a_input_overrides_output_mask)  // normal output pins
+		| (m_ddr_a & port_a_data & m_a_input_overrides_output_mask);  // overridden output pins
 
 	return ret;
 }
@@ -888,14 +886,13 @@ void pia6821_device::write(offs_t offset, uint8_t data)
 //  set_a_input
 //-------------------------------------------------
 
-void pia6821_device::set_a_input(uint8_t data, uint8_t z_mask)
+void pia6821_device::set_a_input(uint8_t data)
 {
 	assert_always(m_in_a_handler.isnull(), "pia6821_device::set_a_input() called when m_in_a_handler set");
 
 	LOG("Set PIA input port A = %02X\n", data);
 
 	m_in_a = data;
-	m_port_a_z_mask = z_mask;
 	m_in_a_pushed = true;
 }
 
@@ -906,7 +903,7 @@ void pia6821_device::set_a_input(uint8_t data, uint8_t z_mask)
 
 void pia6821_device::write_porta(uint8_t data)
 {
-	set_a_input(data, 0);
+	set_a_input(data);
 }
 
 
@@ -916,14 +913,11 @@ void pia6821_device::write_porta(uint8_t data)
 
 void pia6821_device::write_porta_line(int line, bool state)
 {
-	if (!m_in_a_pushed)
-		m_port_a_z_mask = 0xff;
-
 	uint8_t mask = 1 << line;
 	if (state)
-		set_a_input(m_in_a | mask, m_port_a_z_mask & ~mask);
+		set_a_input(m_in_a | mask);
 	else
-		set_a_input(m_in_a & ~mask, m_port_a_z_mask & ~mask);
+		set_a_input(m_in_a & ~mask);
 }
 
 
diff --git a/src/devices/machine/6821pia.h b/src/devices/machine/6821pia.h
index 5682b43ecef..ad7d28f65d6 100644
--- a/src/devices/machine/6821pia.h
+++ b/src/devices/machine/6821pia.h
@@ -5,13 +5,10 @@
     Motorola 6821 PIA interface and emulation
 
     Notes:
-        * get_port_b_z_mask() gives the caller the bitmask that shows
+        * port_b_z_mask() gives the caller the bitmask that shows
           which bits are high-impedance when reading port B, and thus
-          neither 0 or 1. get_output_cb2_z() returns the same info
+          neither 0 or 1. cb2_output_z() returns the same info
           for the CB2 pin.
-        * set_port_a_z_mask allows the input callback to indicate
-          which port A bits are disconnected. For these bits, the
-          read operation will return the output buffer's contents.
         * The 'alt' interface functions are used when the A0 and A1
           address bits are swapped.
         * All 'int' data or return values are bool, and should be
@@ -61,13 +58,13 @@ public:
 	void write_alt(offs_t offset, uint8_t data) { write(((offset << 1) & 0x02) | ((offset >> 1) & 0x01), data); }
 
 	uint8_t port_b_z_mask() const { return ~m_ddr_b; }          // see first note in .c
-	void set_port_a_z_mask(uint8_t data) { m_port_a_z_mask = data; }// see second note in .c
 
 	DECLARE_WRITE8_MEMBER( porta_w ) { write_porta(data); }
 	void write_porta(uint8_t data);
 	void write_porta_line(int line, bool state);
-	void set_a_input(uint8_t data, uint8_t z_mask);
+	void set_a_input(uint8_t data);
 	uint8_t a_output();
+	void set_port_a_input_overrides_output_mask(uint8_t mask) { m_a_input_overrides_output_mask = mask; }
 
 	DECLARE_WRITE_LINE_MEMBER( pa0_w ) { write_porta_line(0, state); }
 	DECLARE_WRITE_LINE_MEMBER( pa1_w ) { write_porta_line(1, state); }
@@ -176,8 +173,8 @@ private:
 	uint8_t m_in_ca1;
 	uint8_t m_in_ca2;
 	uint8_t m_out_a;
+	uint8_t m_a_input_overrides_output_mask;
 	uint8_t m_out_ca2;
-	uint8_t m_port_a_z_mask;
 	uint8_t m_ddr_a;
 	uint8_t m_ctl_a;
 	uint8_t m_irq_a1;
diff --git a/src/mame/drivers/coinmstr.cpp b/src/mame/drivers/coinmstr.cpp
index 99008afa819..0461da5f541 100644
--- a/src/mame/drivers/coinmstr.cpp
+++ b/src/mame/drivers/coinmstr.cpp
@@ -1254,7 +1254,6 @@ uint32_t coinmstr_state::screen_update_coinmstr(screen_device &screen, bitmap_rg
 	return 0;
 }
 
-
 void coinmstr_state::coinmstr(machine_config &config)
 {
 	Z80(config, m_maincpu, CPU_CLOCK); // 7 MHz.
@@ -1267,6 +1266,7 @@ void coinmstr_state::coinmstr(machine_config &config)
 
 	pia6821_device &pia1(PIA6821(config, "pia1", 0));
 	pia1.readpa_handler().set_ioport("PIA1.A");
+	pia1.set_port_a_input_overrides_output_mask(0xff);
 	pia1.readpb_handler().set_ioport("PIA1.B");
 
 	pia6821_device &pia2(PIA6821(config, "pia2", 0));
diff --git a/src/mame/drivers/mpu3.cpp b/src/mame/drivers/mpu3.cpp
index 6e1c005ff64..75bb205e761 100644
--- a/src/mame/drivers/mpu3.cpp
+++ b/src/mame/drivers/mpu3.cpp
@@ -604,7 +604,7 @@ READ8_MEMBER(mpu3_state::pia_ic6_portb_r)
 WRITE8_MEMBER(mpu3_state::pia_ic6_porta_w)
 {
 	LOG(("%s: IC6 PIA Port A Set to %2x (Alpha)\n", machine().describe_context(),data));
-	m_vfd->por(!(data&0x08));
+	m_vfd->por((data & 0x08));
 	m_vfd->data((data & 0x20) >> 5);
 	m_vfd->sclk((data & 0x10) >>4);
 }
diff --git a/src/mame/drivers/zwackery.cpp b/src/mame/drivers/zwackery.cpp
index d186fae1ba3..d9d7a5f4cc3 100644
--- a/src/mame/drivers/zwackery.cpp
+++ b/src/mame/drivers/zwackery.cpp
@@ -441,10 +441,7 @@ WRITE_LINE_MEMBER(zwackery_state::pia0_irq_w)
 
 READ8_MEMBER( zwackery_state::pia1_porta_r )
 {
-	uint8_t data = ioport("IN1")->read();
-	m_pia1->set_port_a_z_mask(data);
-
-	return data;
+	return ioport("IN1")->read();
 }
 
 READ8_MEMBER( zwackery_state::pia1_portb_r )
@@ -457,10 +454,7 @@ READ8_MEMBER( zwackery_state::pia1_portb_r )
 
 READ8_MEMBER( zwackery_state::pia2_porta_r )
 {
-	uint8_t data = ioport("IN3")->read();
-	m_pia2->set_port_a_z_mask(data);
-
-	return data;
+	return ioport("IN3")->read();
 }
 
 
diff --git a/src/mame/includes/coco.h b/src/mame/includes/coco.h
index be0192b0f82..bf9db73747b 100644
--- a/src/mame/includes/coco.h
+++ b/src/mame/includes/coco.h
@@ -159,7 +159,7 @@ protected:
 	ram_device &ram() { return *m_ram; }
 
 	// miscellaneous
-	virtual void update_keyboard_input(uint8_t value, uint8_t z);
+	virtual void update_keyboard_input(uint8_t value);
 	virtual void cart_w(bool state);
 	virtual void update_cart_base(uint8_t *cart_base) { };
 
diff --git a/src/mame/includes/coco3.h b/src/mame/includes/coco3.h
index ffcb05f77a9..9551fafea7e 100644
--- a/src/mame/includes/coco3.h
+++ b/src/mame/includes/coco3.h
@@ -62,7 +62,7 @@ protected:
 	virtual bool irq_get_line(void) override;
 
 	// miscellaneous
-	virtual void update_keyboard_input(uint8_t value, uint8_t z) override;
+	virtual void update_keyboard_input(uint8_t value) override;
 	virtual void cart_w(bool line) override;
 
 private:
diff --git a/src/mame/machine/coco.cpp b/src/mame/machine/coco.cpp
index 71f26e266a9..fb1569828b5 100644
--- a/src/mame/machine/coco.cpp
+++ b/src/mame/machine/coco.cpp
@@ -874,10 +874,8 @@ void coco_state::poll_joystick(bool *joyin, uint8_t *buttons)
 void coco_state::poll_keyboard(void)
 {
 	uint8_t pia0_pb = pia_0().b_output();
-	uint8_t pia0_pb_z = pia_0().port_b_z_mask();
 
 	uint8_t pia0_pa = 0x7F;
-	uint8_t pia0_pa_z = 0x7F;
 
 	/* poll the keyboard, and update PA6-PA0 accordingly*/
 	for (unsigned i = 0; i < m_keyboard.size(); i++)
@@ -887,10 +885,6 @@ void coco_state::poll_keyboard(void)
 		{
 			pia0_pa &= ~(0x01 << i);
 		}
-		if ((value | pia0_pb_z) != 0xFF)
-		{
-			pia0_pa_z &= ~(0x01 << i);
-		}
 	}
 
 	/* hires joystick */
@@ -906,10 +900,9 @@ void coco_state::poll_keyboard(void)
 
 	/* mask out the buttons */
 	pia0_pa &= ~buttons;
-	pia0_pa_z &= ~buttons;
 
 	/* and write the result to PIA0 */
-	update_keyboard_input(pia0_pa, pia0_pa_z);
+	update_keyboard_input(pia0_pa);
 }
 
 
@@ -919,9 +912,9 @@ void coco_state::poll_keyboard(void)
 //  on the CoCo 3 controls a GIME input
 //-------------------------------------------------
 
-void coco_state::update_keyboard_input(uint8_t value, uint8_t z)
+void coco_state::update_keyboard_input(uint8_t value)
 {
-	pia_0().set_a_input(value, z);
+	pia_0().set_a_input(value);
 }
 
 
diff --git a/src/mame/machine/coco3.cpp b/src/mame/machine/coco3.cpp
index bf7dbf685b7..9ce7f324b95 100644
--- a/src/mame/machine/coco3.cpp
+++ b/src/mame/machine/coco3.cpp
@@ -112,9 +112,9 @@ bool coco3_state::irq_get_line(void)
 //  update_keyboard_input
 //-------------------------------------------------
 
-void coco3_state::update_keyboard_input(uint8_t value, uint8_t z)
+void coco3_state::update_keyboard_input(uint8_t value)
 {
-	coco_state::update_keyboard_input(value, z);
+	coco_state::update_keyboard_input(value);
 	m_gime->set_il1(value == 0xFF);
 }
 
diff --git a/src/mame/machine/dgn_beta.cpp b/src/mame/machine/dgn_beta.cpp
index 47dd832648d..adaa9da972d 100644
--- a/src/mame/machine/dgn_beta.cpp
+++ b/src/mame/machine/dgn_beta.cpp
@@ -438,7 +438,8 @@ int dgn_beta_state::GetKeyRow(dgn_beta_state *state, int RowNo)
 */
 READ8_MEMBER(dgn_beta_state::d_pia0_pa_r)
 {
-	return 0;
+	// The hardware has pullup resistors on port A.
+	return 0xff;
 }
 
 WRITE8_MEMBER(dgn_beta_state::d_pia0_pa_w)
@@ -558,7 +559,8 @@ WRITE_LINE_MEMBER(dgn_beta_state::d_pia0_irq_b)
 
 READ8_MEMBER(dgn_beta_state::d_pia1_pa_r)
 {
-	return 0;
+	// The hardware has pullup resistors on port A.
+	return 0xff;
 }
 
 WRITE8_MEMBER(dgn_beta_state::d_pia1_pa_w)
@@ -657,7 +659,8 @@ WRITE_LINE_MEMBER(dgn_beta_state::d_pia1_irq_b)
 */
 READ8_MEMBER(dgn_beta_state::d_pia2_pa_r)
 {
-	return 0;
+	// The hardware has pullup resistors on port A.
+	return 0xff;
 }
 
 WRITE8_MEMBER(dgn_beta_state::d_pia2_pa_w)
@@ -902,11 +905,6 @@ void dgn_beta_state::machine_reset()
 	memset(m_PageRegs, 0, sizeof(m_PageRegs));    /* Reset page registers to 0 */
 	SetDefaultTask();
 
-	/* Set pullups on all PIA port A, to match what hardware does */
-	m_pia_0->set_port_a_z_mask(0xFF);
-	m_pia_1->set_port_a_z_mask(0xFF);
-	m_pia_2->set_port_a_z_mask(0xFF);
-
 	m_d_pia1_pa_last = 0x00;
 	m_d_pia1_pb_last = 0x00;
 	m_RowShifter = 0x00;         /* shift register to select row */
diff --git a/src/mame/machine/thomson.cpp b/src/mame/machine/thomson.cpp
index 97867d1a6ed..bae9ce420b4 100644
--- a/src/mame/machine/thomson.cpp
+++ b/src/mame/machine/thomson.cpp
@@ -1308,9 +1308,9 @@ WRITE8_MEMBER( thomson_state::mo5_sys_porta_out )
 READ8_MEMBER( thomson_state::mo5_sys_porta_in )
 {
 	return
-		(mo5_get_cassette() ? 0x80 : 0) |     /* bit 7: cassette input */
-		((m_io_lightpen_button->read() & 1) ? 0x20 : 0)
-		/* bit 5: lightpen button */;
+		((m_io_lightpen_button->read() & 1) ? 0x20 : 0) | /* bit 5: lightpen button */
+		(mo5_get_cassette() ? 0x80 : 0) |                 /* bit 7: cassette input */
+		0x5f;                                             /* other bits are unconnected and pulled hi internally */
 }
 
 
@@ -1564,7 +1564,6 @@ MACHINE_RESET_MEMBER( thomson_state, mo5 )
 
 	/* subsystems */
 	thom_irq_reset();
-	m_pia_sys->set_port_a_z_mask(0x5f );
 	to7_game_reset();
 	to7_floppy_reset();
 	to7_modem_reset();
@@ -2445,16 +2444,14 @@ void thomson_state::to9_kbd_init()
 
 /* ------------ system PIA 6821 ------------ */
 
-/* afaik, P2-P7 are not connected, so, the warning about undefined 0xf0 can be safely ignored */
-
-
 READ8_MEMBER( thomson_state::to9_sys_porta_in )
 {
 	uint8_t ktest = to9_kbd_ktest();
 
 	LOG_KBD(( "to9_sys_porta_in: ktest=%i\n", ktest ));
 
-	return ktest;
+	// PB1-7 are not connected, and are pulled hi internally
+	return ktest | 0xfe;
 }
 
 
@@ -2507,7 +2504,6 @@ MACHINE_RESET_MEMBER( thomson_state, to9 )
 
 	/* subsystems */
 	thom_irq_reset();
-	m_pia_sys->set_port_a_z_mask( 0xfe );
 	to7_game_reset();
 	to9_floppy_reset();
 	to9_kbd_reset();
@@ -3412,14 +3408,14 @@ WRITE8_MEMBER( thomson_state::to8_vreg_w )
 /* ------------ system PIA 6821 ------------ */
 
 
-
 READ8_MEMBER( thomson_state::to8_sys_porta_in )
 {
 	int ktest = to8_kbd_ktest();
 
 	LOG_KBD(( "$%04x %f: to8_sys_porta_in ktest=%i\n", m_maincpu->pc(), machine().time().as_double(), ktest ));
 
-	return ktest;
+	// PB1-7 are not connected, and are pulled hi internally
+	return ktest | 0xfe;
 }
 
 
@@ -3504,7 +3500,6 @@ MACHINE_RESET_MEMBER( thomson_state, to8 )
 
 	/* subsystems */
 	thom_irq_reset();
-	m_pia_sys->set_port_a_z_mask( 0xfe );
 	to7_game_reset();
 	to8_floppy_reset();
 	to8_kbd_reset();
@@ -3655,7 +3650,6 @@ MACHINE_RESET_MEMBER( thomson_state, to9p )
 
 	/* subsystems */
 	thom_irq_reset();
-	m_pia_sys->set_port_a_z_mask( 0xfe );
 	to7_game_reset();
 	to8_floppy_reset();
 	to9_kbd_reset();
@@ -4116,10 +4110,10 @@ void thomson_state::mo6_game_reset()
 READ8_MEMBER( thomson_state::mo6_sys_porta_in )
 {
 	return
-		(mo5_get_cassette() ? 0x80 : 0) |     /* bit 7: cassette input */
-		8 |                                   /* bit 3: kbd-line float up to 1 */
-		((m_io_lightpen_button->read() & 1) ? 2 : 0);
-	/* bit 1: lightpen button */;
+		((m_io_lightpen_button->read() & 1) ? 2 : 0) | /* bit 1: lightpen button */
+		8 |                                            /* bit 3: kbd-line float up to 1 */
+		(mo5_get_cassette() ? 0x80 : 0) |              /* bit 7: cassette input */
+		0x75;                                          /* other bits are unconnected and pulled hi internally */
 }
 
 
@@ -4345,7 +4339,6 @@ MACHINE_RESET_MEMBER( thomson_state, mo6 )
 
 	/* subsystems */
 	thom_irq_reset();
-	m_pia_sys->set_port_a_z_mask( 0x75 );
 	mo6_game_reset();
 	to7_floppy_reset();
 	to7_modem_reset();
@@ -4568,7 +4561,6 @@ MACHINE_RESET_MEMBER( thomson_state, mo5nr )
 
 	/* subsystems */
 	thom_irq_reset();
-	m_pia_sys->set_port_a_z_mask( 0x65 );
 	mo5nr_game_reset();
 	to7_floppy_reset();
 	to7_modem_reset();