bus/spectrum/zxbus.cpp: Refactored shadow IO handeling

[holub]

as suggested in #13202

[cuavas]

This has multiple issues with the implementation, sorry. I really don’t want it going in a release like this, and we’re less than a week from freeze. Should I just revert it?

Specifically:

• Installing to the I/O space will install over the top of the view. A card can’t install for non-shadow only with this setup.
• You shouldn’t be passing around memory_view_entry references specifically, just use address_space_installer

I’d do something like:

void zxbus_device::set_io_space(address_space_installer &io, address_space_installer &shadow_io)
{
	m_io = &io;
	m_shadow_io = &shadow_io;
}

...

void zxbus_device::add_slot(zxbus_slot_device &slot)
{
	m_slot_list.push_front(&slot);
	device_zxbus_card_interface *const card = slot.get_card_device();
	if (card)
	{
		if (m_io)
			m_io->install_device(0x0000, 0xffff, *card, &device_zxbus_card_interface::io_map);
		if (m_shadow_io)
			m_shadow_ioio->install_device(0x0000, 0xffff, *card, &device_zxbus_card_interface::shadow_io_map);
	}
}

In a card:

void some_card::io_map(address_map &map)
{
	// install non-shadow stuff here
}

void some_card::shadow_io_map(address_map &map)
{
	// install shadow stuff here, or call io_map(map) if the card ignores the 17th bit
}

(Cards won’t need to install themselves on start if it’s done for them from add_slot.)

Then when using it:

void scorpion_state::scorpion_io(address_map &map)
{
	map.unmap_value_high();

	map(0x0022, 0x0022).select(0xffdc) // FE | xxxxxxxxxx1xxx10
		.rw(FUNC(scorpion_state::spectrum_ula_r), FUNC(scorpion_state::spectrum_ula_w));
	map(0x0023, 0x0023).mirror(0xffdc) // FF | xxxxxxxxxx1xxx11
		.r(FUNC(scorpion_state::port_ff_r));
	map(0x0021, 0x0021).mirror(0x3fdc) // 1FFD | 00xxxxxxxx1xxx01
		.w(FUNC(scorpion_state::port_1ffd_w));
	map(0x4021, 0x4021).mirror(0x3fdc) // 7FFD | 01xxxxxxxx1xxx01
		.w(FUNC(scorpion_state::port_7ffd_w));

	map(0xa021, 0xa021).mirror(0x1fdc) // BFFD | 101xxxxxxx1xxx01
		.lw8(NAME([this](u8 data) { m_ay[m_ay_selected]->data_w(data); }));
	map(0xe021, 0xe021).mirror(0x1fdc) // FFFD | 111xxxxxxx1xxx01
		.lr8(NAME([this]() { return m_ay[m_ay_selected]->data_r(); })).w(FUNC(scorpion_state::ay_address_w));

	// Mouse
	map(0xfadf, 0xfadf).lr8(NAME([this]() -> u8 { return 0x80 | (m_io_mouse[2]->read() & 0x07); }));
	map(0xfbdf, 0xfbdf).lr8(NAME([this]() -> u8 { return m_io_mouse[0]->read(); }));
	map(0xffdf, 0xffdf).lr8(NAME([this]() -> u8 { return ~m_io_mouse[1]->read(); }));
	map(0x0003, 0x0003) // 1F | xxxxxxxx0x0xxx11
		.select(0xff5c).lr8(NAME([this]() -> u8 { return (m_beta->state_r() & 0xc0) | 0x00; })); // TODO Kepmston Joystick

	// DOS + xxxxxxxx0nnxxx11
	map(0x0000, 0xffff).view(m_io_shadow_view);

	m_io_shadow_view[0];

	// Shadow
	m_io_shadow_view[1](0x0003, 0x0003).mirror(0xff1c).rw(m_beta, FUNC(beta_disk_device::status_r), FUNC(beta_disk_device::command_w));
	m_io_shadow_view[1](0x0023, 0x0023).mirror(0xff1c).rw(m_beta, FUNC(beta_disk_device::track_r), FUNC(beta_disk_device::track_w));
	m_io_shadow_view[1](0x0043, 0x0043).mirror(0xff1c).rw(m_beta, FUNC(beta_disk_device::sector_r), FUNC(beta_disk_device::sector_w));
	m_io_shadow_view[1](0x0063, 0x0063).mirror(0xff1c).rw(m_beta, FUNC(beta_disk_device::data_r), FUNC(beta_disk_device::data_w));
	m_io_shadow_view[](0x00e3, 0x00e3).mirror(0xff1c).rw(m_beta, FUNC(beta_disk_device::state_r), FUNC(beta_disk_device::param_w));

	subdevice<zxbus_device>("zxbus")->set_io_space(m_io_shadow_view[0], m_io_shadow_view[1]);
}

...

void scorpion_state::update_io(bool dos_enable)
{
	if (dos_enable)
		m_beta->enable();
	else
		m_beta->disable();

	scorpion_update_memory();

	m_io_shadow_view.select(dos() ? 1 : 0);
}

[holub]

@cuavas Yes, feel free to revert it. I'll open new one with the fixes you suggested.

[holub]

void scorpion_state::update_io(bool dos_enable)
{
 	...
	m_io_shadow_view.select(dos() ? 1 : 0);
}

I love this!!! I was looking how to manage such setup nicely for a long time and didn't realize it can be select() all the time without disabled()