Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mattel/pixtermu.cpp: Add new not working system & software list #12013

Merged
merged 3 commits into from Feb 16, 2024

Conversation

qufb
Copy link
Contributor

@qufb qufb commented Feb 4, 2024

New software list items marked not working (pixter_cart.xml)

Alphabet Forest [QUFB]
Clifford - The Big Red Dog [QUFB]
Cyberchase [QUFB]
Mazes, Puzzles & Games [QUFB]
SpongeBob SquarePants - Math Adventure [QUFB]

@mamehaze
Copy link
Contributor

mamehaze commented Feb 5, 2024

ah, nice, I think we had one of these and a few games sent to Sean back in the day, but he didn't manage to dump the internal ROM.. looking over old emails

Fisher Price Pixter Color console
Fisher Price Pixter Color game cart: "Dino Draw"
Fisher Price Pixter Color game cart: "Mazes, Puzzles & Games"
Fisher Price Pixter Color game cart: "Creative Genius"
Fisher Price Pixter Color game cart: "Learning Starter"
Fisher Price Pixter game cart: "The Powerpuff Girls"
Fisher Price Pixter game cart: "Music Studio"
Fisher Price Pixter game cart: "Barbie Fashion Show"
Fisher Price Pixter game cart: "Learning Fun 2"
Fisher Price Pixter game cart: "Story Composer"

come up as 'secured and sent to Sean' at least, so if Sean didn't dump; those, maybe he still has them (assuming this is the same system we're talking about)

src/mame/mattel/pixtermu.cpp Outdated Show resolved Hide resolved
src/mame/mattel/pixtermu.cpp Outdated Show resolved Hide resolved
src/mame/mattel/pixtermu.cpp Outdated Show resolved Hide resolved
src/mame/mattel/pixtermu.cpp Outdated Show resolved Hide resolved
src/mame/mattel/pixtermu.cpp Outdated Show resolved Hide resolved
src/mame/mattel/pixtermu.cpp Outdated Show resolved Hide resolved
src/mame/mattel/pixtermu.cpp Outdated Show resolved Hide resolved
src/mame/mattel/pixtermu.cpp Show resolved Hide resolved
@qufb
Copy link
Contributor Author

qufb commented Feb 5, 2024

ah, nice, I think we had one of these and a few games sent to Sean back in the day, but he didn't manage to dump the internal ROM.. looking over old emails

Fisher Price Pixter Color console Fisher Price Pixter Color game cart: "Dino Draw" Fisher Price Pixter Color game cart: "Mazes, Puzzles & Games" Fisher Price Pixter Color game cart: "Creative Genius" Fisher Price Pixter Color game cart: "Learning Starter" Fisher Price Pixter game cart: "The Powerpuff Girls" Fisher Price Pixter game cart: "Music Studio" Fisher Price Pixter game cart: "Barbie Fashion Show" Fisher Price Pixter game cart: "Learning Fun 2" Fisher Price Pixter game cart: "Story Composer"

come up as 'secured and sent to Sean' at least, so if Sean didn't dump; those, maybe he still has them (assuming this is the same system we're talking about)

It's likely a very similar system. Pixter Multi-Media is backwards compatible with Pixter Color carts, and probably the original Pixter as well (there was an adapter made for Pixter Color, don't know if it works with Pixter Multi-Media).

I've written some dumping notes that might be helpful for dumping Pixter Color and those carts. It also has JTAG, although the pins will have to be traced for LH75411, as it seems that wiki didn't document the pinout.

hash/pixter_cart.xml Outdated Show resolved Hide resolved
Comment on lines 110 to 121
if (m_cart->exists()) {
std::string region_tag;
memory_region *cart_rom = m_cart->memregion("cart:rom");
uint32_t const size = m_cart->common_get_size("rom");
m_maincpu->space(AS_PROGRAM).install_rom(0x48000000, 0x48000000 + size - 1, size, cart_rom->base());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don’t need common_get_size here – you can use cart_rom->bytes() to get the size of the region in bytes directly.

I’m not sure the mirroring specified here is going to do exactly what you want:

  • For a 0x400000 byte cartridge, the mirror bits will be 0x400000, so it will be mapped at 0x48000000-0x483fffff and 0x48800000-0x487fffff.
  • For a 0x200000 byte cartridge, the mirror bits will be 0x200000, so it will be mapped at 0x48000000-0x481fffff and 0x48800000-0x483fffff.

I would expect you’d want it mirrored across the same area irrespective of the cartridge size.

The device_generic_cart_interface::install_non_power_of_two helper is designed to help deal with this (it works for powers of two as well). You can use it like:

		device_generic_cart_interface::install_non_power_of_two<0>(
				cart_rom->bytes(),
				0x007f'ffff,
				0,
				0x4800'0000,
				[this, cart_rom] (offs_t begin, offs_t end, offs_t mirror, offs_t src)
				{
					m_maincpu->space(AS_PROGRAM).install_rom(begin, end, mirror, cart_rom->base() + src);
				});

(I haven’t tested that with this driver, I’m going by memory.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for what I can’t remember, it’s whether the offsets need to be shifted for the address space width or only if the address space uses shifted addresses. If that installs at the wrong addresses, it probably needs to be:

		device_generic_cart_interface::install_non_power_of_two<2>(
				cart_rom->bytes() >> 2,
				0x007f'ffff >> 2,
				0,
				0x4800'0000,
				[this, cart_rom] (offs_t begin, offs_t end, offs_t mirror, offs_t src)
				{
					m_maincpu->space(AS_PROGRAM).install_rom(begin, end, mirror, &cart_rom->as_u32() + src);
				});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First suggestion works. I rechecked on hardware and the mirroring covers the whole range up to the start of the next nCS address map, so I just needed to adjust the mask.

Comment on lines 140 to 159
// nCS0 (Unused NAND Flash?)
map(0x40000000, 0x403fffff).share("ncs0");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need the .share(...) here? It’s creating a memory area that isn’t actually used.

Copy link
Contributor Author

@qufb qufb Feb 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it in since the firmware remap function supports it. It's just unconnected on this model (if any actually have a port for it).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but having the .share will allocate a block of RAM that isn’t being used for anything, but still gets put in save states, shows in debugger memory view source lists, etc. If there was actually Flash there, you’d need to use a NAND Flash device or something. It’s better to just leave a comment that the address range can be used for Flash.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough.

Comment on lines 185 to 235
GENERIC_CARTSLOT(config, m_cart, generic_plain_slot, "pixter_cart");
m_cart->set_endian(ENDIANNESS_LITTLE);
m_cart->set_width(GENERIC_ROM32_WIDTH);
m_cart->set_must_be_loaded(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You’re still going to need m_cart->set_device_load to deal with loose software (i.e. not loaded from the software list). It’s possible to just refuse to load it (in which case you need to actually check for that and return an error). It’s better to support it though, as it makes it easier to test new dumps, develop homebrew software, etc. without needing to edit the software list for every change.

Copy link
Contributor Author

@qufb qufb Feb 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I misunderstood your previous comment. I've added a guard if it was loaded via software list, to just return without doing any allocation, was that the intention?

@qufb qufb force-pushed the pixtermu branch 2 times, most recently from 9c2e7e7 to 4895acd Compare February 16, 2024 17:13
Copy link
Member

@cuavas cuavas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is almost there, just needs some more error checking in the image load callback.

Comment on lines 134 to 173
DEVICE_IMAGE_LOAD_MEMBER(pixter_multimedia_state::cart_load)
{
if (!image.loaded_through_softlist()) {
uint32_t const size = m_cart->common_get_size("rom");

m_cart->rom_alloc(size, GENERIC_ROM32_WIDTH, ENDIANNESS_LITTLE);
m_cart->common_load_rom(m_cart->get_rom_base(), size, "rom");
}

return std::make_pair(std::error_condition(), std::string());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, we’re almost there, but there are still a few issues:

  • No error checking to ensure that the software list entry is usable (people can edit software lists, and easily screw things up)
  • rom_alloc won’t give the same tag as the region loaded from the software list
  • common_load_rom doesn’t report errors
  • You need to remember to swap the data on big Endian hosts

Something like this should work, but I haven’t tested it, just coding from memory:

DEVICE_IMAGE_LOAD_MEMBER(pixter_multimedia_state::cart_load)
{
	uint64_t length;
	memory_region *cart_rom = nullptr;
	if (m_cart->loaded_through_softlist()) {
		cart_rom = m_cart->memregion("rom");
		if (!cart_rom)
			return std::make_pair(image_error::BADSOFTWARE, "Software list item has no 'rom' data area");
		length = cart_rom->bytes();
	} else {
		length = m_cart->length();
	}

	if (!length)
		return std::make_pair(image_error::INVALIDLENGTH, "Cartridges must not be empty");
	if (length & 3)
		return std::make_pair(image_error::INVALIDLENGTH, "Unsupported cartridge size (must be a multiple of 4 bytes)");

	if (!m_cart->loaded_through_softlist()) {
		cart_rom = machine().memory().region_alloc(m_cart->subtag("rom"), length, 4, ENDIANNESS_LITTLE);
		if (!cart_rom)
			return std::make_pair(std::errc::not_enough_memory, std::string());

		uint32_t *const base = reinterpret_cast<uint32_t *>(cart_rom->base());
		if (m_cart->fread(base, length) != length)
			return std::make_pair(std::errc::io_error, "Error reading cartridge file");

		if (ENDIANNESS_NATIVE != ENDIANNESS_LITTLE)
		{
			for (uint64_t i = 0; (length / 4) > i; ++i)
				base[i] = swapendian_int32(base[i]);
		}
	}

	return std::make_pair(std::error_condition(), std::string());
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I realise it would be useful to have a helper function to implement this pattern or something similar rather than the problematic common_load_rom. Replacing the current bus/generic stuff is a long-term goal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine, thanks.

Comment on lines 111 to 112
if (m_cart->exists()) {
memory_region *cart_rom = m_cart->memregion("cart:rom");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn’t this be m_cart->memregion("rom")? When software list items are loaded, the data area tags are interpreted relative to the slot. (The extra cart: part is only in the tag of redundant additional region that common_load_rom allocates.)

Copy link
Contributor Author

@qufb qufb Feb 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was missing the part of allocating with m_cart->subtag("rom") when loading manually, which makes this work, otherwise memregion wouldn't be able to find it. With the DEVICE_IMAGE_LOAD_MEMBER changes it works for both cases now.

@cuavas cuavas merged commit f5c4d82 into mamedev:master Feb 16, 2024
6 checks passed
Mokona pushed a commit to Mokona/mame that referenced this pull request Feb 28, 2024
…r and software list. (mamedev#12013)

New systems marked not working
-----------------------
Mattel Pixter Multi-Media [QUFB]

New software list items marked not working (pixter_cart.xml)
------------------------------------------------
Alphabet Forest [QUFB]
Clifford - The Big Red Dog [QUFB]
Cyberchase [QUFB]
Mazes, Puzzles & Games [QUFB]
SpongeBob SquarePants - Math Adventure [QUFB]
stonedDiscord pushed a commit to stonedDiscord/mame that referenced this pull request Apr 8, 2024
…r and software list. (mamedev#12013)

New systems marked not working
-----------------------
Mattel Pixter Multi-Media [QUFB]

New software list items marked not working (pixter_cart.xml)
------------------------------------------------
Alphabet Forest [QUFB]
Clifford - The Big Red Dog [QUFB]
Cyberchase [QUFB]
Mazes, Puzzles & Games [QUFB]
SpongeBob SquarePants - Math Adventure [QUFB]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants