cpu/upd7725.cpp: Improved host interface, suppress side effects for debugger reads. (#13530)

* Split host interface into separate data_r, data_w and status_r.
* Added access mask for µPD96050 data RAM write, simplified downstream code that uses it.
* bus/snes/event.cpp, bus/snes/upd.cpp: Added logging for writes to DSP status register address.

src/devices/bus/snes/event.cpp

@@ -113,8 +113,10 @@ uint8_t sns_pfest94_device::chip_read(offs_t offset)
 	else
 	{
 		// DSP access
-		offset &= 0x1fff;
+		if (BIT(offset, 12))
-		return m_upd7725->snesdsp_read(offset < 0x1000);
+			return m_upd7725->status_r();
+		else
+			return m_upd7725->data_r();
 	}
 }
 
@@ -151,8 +153,10 @@ void sns_pfest94_device::chip_write(offs_t offset, uint8_t data)
 	else
 	{
 		// DSP access
-		offset &= 0x1fff;
+		if (BIT(~offset, 12))
-		m_upd7725->snesdsp_write(offset < 0x1000, data);
+			m_upd7725->data_w(data);
+		else
+			logerror("%s: Writing DSP status to %02x, ignored", machine().describe_context(), data);
 	}
 }
 

src/devices/bus/snes/upd.cpp

@@ -13,13 +13,13 @@
 
 
 // helpers
-inline uint32_t get_prg(uint8_t *CPU, uint32_t addr)
+inline uint32_t get_prg(uint8_t const *CPU, uint32_t addr)
 {
-	return ((CPU[addr * 4] << 24) | (CPU[addr * 4 + 1] << 16) | (CPU[addr * 4 + 2] << 8) | 0x00);
+	return (CPU[addr * 4] << 24) | (CPU[addr * 4 + 1] << 16) | (CPU[addr * 4 + 2] << 8) | 0x00;
 }
-inline uint16_t get_data(uint8_t *CPU, uint32_t addr)
+inline uint16_t get_data(uint8_t const *CPU, uint32_t addr)
 {
-	return ((CPU[addr * 2] << 8) | CPU[addr * 2 + 1]);
+	return (CPU[addr * 2] << 8) | CPU[addr * 2 + 1];
 }
 
 //-------------------------------------------------
@@ -141,15 +141,19 @@ void sns_rom20_necdsp_device::device_add_mconfig(machine_config &config)
 
 uint8_t sns_rom20_necdsp_device::chip_read(offs_t offset)
 {
-	offset &= 0x7fff;
+	if (BIT(offset, 14))
-	return m_upd7725->snesdsp_read(offset < 0x4000);
+		return m_upd7725->status_r();
+	else
+		return m_upd7725->data_r();
 }
 
 
 void sns_rom20_necdsp_device::chip_write(offs_t offset, uint8_t data)
 {
-	offset &= 0x7fff;
+	if (BIT(~offset, 14))
-	m_upd7725->snesdsp_write(offset < 0x4000, data);
+		m_upd7725->data_w(data);
+	else
+		logerror("%s: Writing DSP status to %02x, ignored", machine().describe_context(), data);
 }
 
 
@@ -199,15 +203,19 @@ void sns_rom21_necdsp_device::device_add_mconfig(machine_config &config)
 
 uint8_t sns_rom21_necdsp_device::chip_read(offs_t offset)
 {
-	offset &= 0x1fff;
+	if (BIT(offset, 12))
-	return m_upd7725->snesdsp_read(offset < 0x1000);
+		return m_upd7725->status_r();
+	else
+		return m_upd7725->data_r();
 }
 
 
 void sns_rom21_necdsp_device::chip_write(offs_t offset, uint8_t data)
 {
-	offset &= 0x1fff;
+	if (BIT(~offset, 12))
-	m_upd7725->snesdsp_write(offset < 0x1000, data);
+		m_upd7725->data_w(data);
+	else
+		logerror("%s: Writing DSP status to %02x, ignored", machine().describe_context(), data);
 }
 
 
@@ -220,13 +228,18 @@ void sns_rom21_necdsp_device::chip_write(offs_t offset, uint8_t data)
 uint8_t sns_rom_setadsp_device::chip_read(offs_t offset)
 {
 	if (offset >= 0x600000 && offset < 0x680000 && (offset & 0xffff) < 0x4000)
-		m_upd96050->snesdsp_read((offset & 0x01) ? false : true);
+	{
+		if (BIT(offset, 0))
+			return m_upd96050->status_r();
+		else
+			return m_upd96050->data_r();
+	}
 
 	if (offset >= 0x680000 && offset < 0x700000 && (offset & 0xffff) < 0x8000)
 	{
-		uint16_t address = offset & 0xffff;
+		uint16_t const address = offset & 0xffff;
-		uint16_t temp = m_upd96050->dataram_r(address/2);
+		uint16_t const temp = m_upd96050->dataram_r(address >> 1);
-		if (offset & 1)
+		if (BIT(offset, 0))
 			return temp >> 8;
 		else
 			return temp & 0xff;
@@ -240,27 +253,18 @@ void sns_rom_setadsp_device::chip_write(offs_t offset, uint8_t data)
 {
 	if (offset >= 0x600000 && offset < 0x680000 && (offset & 0xffff) < 0x4000)
 	{
-		m_upd96050->snesdsp_write((offset & 0x01) ? false : true, data);
+		if (BIT(~offset, 0))
+			m_upd96050->data_w(data);
+		else
+			logerror("%s: Writing DSP status to %02x, ignored", machine().describe_context(), data);
 		return;
 	}
 
 	if (offset >= 0x680000 && offset < 0x700000 && (offset & 0xffff) < 0x8000)
 	{
-		uint16_t address = offset & 0xffff;
+		uint16_t const address = (offset & 0xffff) >> 1;
-		uint16_t temp = m_upd96050->dataram_r(address/2);
+		uint8_t const shift = BIT(offset, 0) << 3;
-
+		m_upd96050->dataram_w(address, (uint16_t(data) << 8) | data, uint16_t(0xff) << shift);
-		if (offset & 1)
-		{
-			temp &= 0xff;
-			temp |= data << 8;
-		}
-		else
-		{
-			temp &= 0xff00;
-			temp |= data;
-		}
-
-		m_upd96050->dataram_w(address/2, temp);
 		return;
 	}
 }

src/devices/cpu/upd7725/upd7725.cpp

@@ -569,38 +569,44 @@ void necdsp_device::exec_ld(uint32_t opcode) {
 	}
 }
 
-uint8_t necdsp_device::snesdsp_read(bool mode) {
+uint8_t necdsp_device::status_r()
-	if (!mode)
+{
-	{
+	return regs.sr >> 8;
-		return regs.sr >> 8;
+}
-	}
 
+
+uint8_t necdsp_device::data_r()
+{
 	if (regs.sr.drc == 0)
 	{
 		//16-bit
 		if(regs.sr.drs == 0)
 		{
-			regs.sr.drs = 1;
+			if (!machine().side_effects_disabled())
+				regs.sr.drs = 1;
 			return regs.dr >> 0;
 		}
 		else
 		{
-			regs.sr.rqm = 0;
+			if (!machine().side_effects_disabled())
-			regs.sr.drs = 0;
+			{
+				regs.sr.rqm = 0;
+				regs.sr.drs = 0;
+			}
 			return regs.dr >> 8;
 		}
 	}
 	else
 	{
 		//8-bit
-		regs.sr.rqm = 0;
+		if (!machine().side_effects_disabled())
+			regs.sr.rqm = 0;
 		return regs.dr >> 0;
 	}
 }
 
-void necdsp_device::snesdsp_write(bool mode, uint8_t data) {
+void necdsp_device::data_w(uint8_t data)
-	if (!mode) return;
+{
-
 	if (regs.sr.drc == 0)
 	{
 		//16-bit

src/devices/cpu/upd7725/upd7725.h

@@ -36,8 +36,9 @@ public:
 	auto p0() { return m_out_p0_cb.bind(); }
 	auto p1() { return m_out_p1_cb.bind(); }
 
-	uint8_t snesdsp_read(bool mode);
+	uint8_t status_r();
-	void snesdsp_write(bool mode, uint8_t data);
+	uint8_t data_r();
+	void data_w(uint8_t data);
 
 protected:
 	// construction/destruction
@@ -175,7 +176,10 @@ public:
 	upd96050_device(const machine_config &mconfig, const char *tag, device_t *owner, uint32_t clock);
 
 	uint16_t dataram_r(uint16_t addr) { return dataRAM[addr & 0x07ff]; }
-	void dataram_w(uint16_t addr, uint16_t data) { dataRAM[addr & 0x07ff] = data; }
+	void dataram_w(uint16_t addr, uint16_t data, uint16_t mem_mask = uint16_t(~0))
+	{
+		COMBINE_DATA(&dataRAM[addr & 0x07ff]);
+	}
 };
 
 // device type definition

src/mame/seta/ssv.cpp

@@ -347,49 +347,17 @@ void drifto94_state::dsp_data_map(address_map &map)
 	map(0x0000, 0x07ff).rom().region("dspdata", 0);
 }
 
-uint16_t drifto94_state::dsp_dr_r()
-{
-	return m_dsp->snesdsp_read(true);
-}
-
-void drifto94_state::dsp_dr_w(uint16_t data)
-{
-	m_dsp->snesdsp_write(true, data);
-}
-
 uint16_t drifto94_state::dsp_r(offs_t offset)
 {
 	const uint16_t temp = m_dsp->dataram_r(offset / 2);
-	uint16_t res;
+	const uint8_t shift = BIT(offset, 0) << 3;
-
+	return (temp >> shift) & 0xff;
-	if (BIT(offset, 0))
-	{
-		res = temp >> 8;
-	}
-	else
-	{
-		res = temp & 0xff;
-	}
-
-	return res;
 }
 
 void drifto94_state::dsp_w(offs_t offset, uint16_t data)
 {
-	uint16_t temp = m_dsp->dataram_r(offset / 2);
+	const uint8_t shift = BIT(offset, 0) << 3;
-
+	m_dsp->dataram_w(offset / 2, (data & 0xff) << shift, 0xff << shift);
-	if (BIT(offset, 0))
-	{
-		temp &= 0xff;
-		temp |= data << 8;
-	}
-	else
-	{
-		temp &= 0xff00;
-		temp |= data;
-	}
-
-	m_dsp->dataram_w(offset / 2, temp);
 }
 
 /***************************************************************************
@@ -438,8 +406,8 @@ void drifto94_state::drifto94_map(address_map &map)
 	ssv_map(map, 0xc00000);
 //  map(0x210002, 0x210003).nopw();                                      // ? 1 at the start
 	map(0x400000, 0x47ffff).nopw();                                       // ?
-	map(0x480000, 0x480001).rw(FUNC(drifto94_state::dsp_dr_r), FUNC(drifto94_state::dsp_dr_w));
+	map(0x480000, 0x480001).rw(m_dsp, FUNC(upd96050_device::data_r), FUNC(upd96050_device::data_w));
-	map(0x482000, 0x482fff).rw(FUNC(drifto94_state::dsp_r), FUNC(drifto94_state::dsp_w));
+	map(0x482000, 0x482fff).rw(FUNC(drifto94_state::dsp_r), FUNC(drifto94_state::dsp_w)).umask16(0x00ff);
 	map(0x483000, 0x485fff).nopw();                                        // ?
 	map(0x500000, 0x500001).nopw();                                        // ??
 	map(0x510000, 0x510001).r(FUNC(drifto94_state::drifto94_unknown_r));                       // ??
@@ -797,8 +765,8 @@ void drifto94_state::twineag2_map(address_map &map)
 	ssv_map(map, 0xe00000);
 	map(0x010000, 0x03ffff).ram();                         // More RAM
 	map(0x210000, 0x210001).r("watchdog", FUNC(watchdog_timer_device::reset16_r)); // Watchdog (also value is cmp.b with mem 8)
-	map(0x480000, 0x480001).rw(FUNC(drifto94_state::dsp_dr_r), FUNC(drifto94_state::dsp_dr_w));
+	map(0x480000, 0x480001).rw(m_dsp, FUNC(upd96050_device::data_r), FUNC(upd96050_device::data_w));
-	map(0x482000, 0x482fff).rw(FUNC(drifto94_state::dsp_r), FUNC(drifto94_state::dsp_w));
+	map(0x482000, 0x482fff).rw(FUNC(drifto94_state::dsp_r), FUNC(drifto94_state::dsp_w)).umask16(0x00ff);
 }
 
 

src/mame/seta/ssv.h

@@ -162,8 +162,6 @@ public:
 	void twineag2(machine_config &config);
 
 private:
-	uint16_t dsp_dr_r();
-	void dsp_dr_w(uint16_t data);
 	uint16_t dsp_r(offs_t offset);
 	void dsp_w(offs_t offset, uint16_t data);
 	uint16_t drifto94_unknown_r();

src/mame/skeleton/tsispch.cpp

@@ -155,9 +155,6 @@ public:
 private:
 	uint8_t dsw_r();
 	void peripheral_w(uint8_t data);
-	uint16_t dsp_data_r();
-	void dsp_data_w(uint16_t data);
-	uint16_t dsp_status_r();
 	void dsp_status_w(uint16_t data);
 	void dsp_to_8086_p0_w(int state);
 	void dsp_to_8086_p1_w(int state);
@@ -205,7 +202,7 @@ void tsispch_state::peripheral_w(uint8_t data)
 	see the top of the file for more info.
 	*/
 	m_paramReg = data;
-	m_dsp->set_input_line(INPUT_LINE_RESET, BIT(data,6)?CLEAR_LINE:ASSERT_LINE);
+	m_dsp->set_input_line(INPUT_LINE_RESET, BIT(data, 6) ? CLEAR_LINE : ASSERT_LINE);
 	//LOGPRM("8086: Parameter Reg written: UNK7: %d, DSPRST6: %d; UNK5: %d; LED4: %d; LED3: %d; LED2: %d; LED1: %d; DSPIRQMASK: %d\n", BIT(data,7), BIT(data,6), BIT(data,5), BIT(data,4), BIT(data,3), BIT(data,2), BIT(data,1), BIT(data,0));
 	LOGPRM("8086: Parameter Reg written: UNK7: %d, DSPRST6: %d; UNK5: %d; LED4: %d; LED3: %d; LED2: %d; LED1: %d; DSPIRQMASK: %d\n", BIT(data,7), BIT(data,6), BIT(data,5), BIT(data,4), BIT(data,3), BIT(data,2), BIT(data,1), BIT(data,0));
 	popmessage("LEDS: 6/Talking:%d 5:%d 4:%d 3:%d\n", 1-BIT(data,1), 1-BIT(data,2), 1-BIT(data,3), 1-BIT(data,4));
@@ -214,30 +211,10 @@ void tsispch_state::peripheral_w(uint8_t data)
 /*****************************************************************************
  UPD77P20 stuff
 *****************************************************************************/
-uint16_t tsispch_state::dsp_data_r()
-{
-	uint8_t r = m_dsp->snesdsp_read(true);
-	LOGDSP("dsp data read: %02x\n", r);
-	return r;
-}
-
-void tsispch_state::dsp_data_w(uint16_t data)
-{
-	LOGDSP("dsp data write: %02x\n", data);
-	m_dsp->snesdsp_write(true, data);
-}
-
-uint16_t tsispch_state::dsp_status_r()
-{
-	uint8_t r = m_dsp->snesdsp_read(false);
-	LOGDSP("dsp status read: %02x\n", r);
-	return r;
-}
 
 void tsispch_state::dsp_status_w(uint16_t data)
 {
 	LOG("warning: upd772x status register should never be written to!\n");
-	m_dsp->snesdsp_write(false, data);
 }
 
 void tsispch_state::dsp_to_8086_p0_w(int state)
@@ -334,8 +311,8 @@ void tsispch_state::i8086_mem(address_map &map)
 	map(0x03200, 0x03203).mirror(0x341fc).rw(m_pic, FUNC(pic8259_device::read), FUNC(pic8259_device::write)).umask16(0x00ff); // AMD P8259 PIC @ U5 (reads as 04 and 7c, upper byte is open bus)
 	map(0x03400, 0x03400).mirror(0x341fe).r(FUNC(tsispch_state::dsw_r)); // verified, read from dipswitch s4
 	map(0x03401, 0x03401).mirror(0x341fe).w(FUNC(tsispch_state::peripheral_w)); // verified, write to the 4 leds, plus 4 control bits
-	map(0x03600, 0x03601).mirror(0x341fc).rw(FUNC(tsispch_state::dsp_data_r), FUNC(tsispch_state::dsp_data_w)); // verified; UPD77P20 data reg r/w
+	map(0x03600, 0x03601).mirror(0x341fc).rw(m_dsp, FUNC(upd7725_device::data_r), FUNC(upd7725_device::data_w)); // verified; UPD77P20 data reg r/w
-	map(0x03602, 0x03603).mirror(0x341fc).rw(FUNC(tsispch_state::dsp_status_r), FUNC(tsispch_state::dsp_status_w)); // verified; UPD77P20 status reg r
+	map(0x03602, 0x03603).mirror(0x341fc).r(m_dsp, FUNC(upd7725_device::status_r)).w(FUNC(tsispch_state::dsp_status_w)); // verified; UPD77P20 status reg r
 	map(0xc0000, 0xfffff).rom(); // verified
 }