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

misc/sttechno: Implement driver for Shamisen Brothers and promote to working #11929

Merged
merged 10 commits into from
Jan 15, 2024

Conversation

987123879113
Copy link
Contributor

Old driver misc/shambros.cpp has been renamed to misc/sttechno.cpp. It's unknown if/what other games run on this PCB still, but the logo "ST-Techno" is visible in 3 different places on the PCB and variations of the "ST" prefix are used in multiple places (STV01 MainPCB, STT-GA1, STT-SA1) so it seems like a better driver name.

@987123879113 987123879113 force-pushed the shambros branch 2 times, most recently from afc0eab to 23d858b Compare January 14, 2024 11:10
-----------------------------
Shamisen Brothers Vol 1 (V1.01K) [windyfairy, Taro, angeryer]
src/devices/bus/ata/fx5400w.cpp Outdated Show resolved Hide resolved
src/devices/sound/stt_sa1.cpp Outdated Show resolved Hide resolved
src/devices/sound/stt_sa1.cpp Outdated Show resolved Hide resolved
Comment on lines 145 to 146
m_stream = stream_alloc(0, 2, clock() / 448);
m_ram = make_unique_clear<uint8_t[]>(0x600000); // 12 512k x 8 SRAM chips
Copy link
Member

Choose a reason for hiding this comment

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

Is the RAM logically words or bytes?

Alternatively, you could store the RAM as a std::unique_ptr<uint16_t []>:

  • No need to double the offset and use get_u16be/put_u16be when reading/writing.
  • Use util::big_endian_cast<uint8_t>(m_ram.get())[offset] (from endianness.h) in read_sample.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The registers are 16-bit but the RAM chips themselves are 8-bit (U31, U32, U33, U37, U38, U39, U40, U41, U44, U45, U47, U48 (HY628400): Toshiba TC554001, 512k x 8 SRAM) which is why I implemented the RAM as uint8_t in the device here just to be consistent with the real hardware.

Would you consider using uint16_t for the RAM the better choice here, performance and such considered? It's all implemented in the FPGA and I can't verify anything beyond how the code uses the chip so I don't have a preference here as long as it's works.

Copy link
Member

Choose a reason for hiding this comment

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

Oh dammit. I see what I’m missing.

It isn’t that the RAM and Flash is behind the “sound chip” per se. It’s just that it has an external 16-bit memory bus that’s attached to the RAM and Flash, and the the FPGA also allows the CPU to access a window into this space.

The “sound chip” device should only be handling its own register reads and writes, and it should derive from device_rom_interface to access its sample storage. The driver can install the RAM and Flash in its address space.

Are you comfortable looking for examples of device_rom_interface and doing it, or do I need to have a crack at it in the next few days?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, and just to be explicit, the 8-bit RAMs can easily be wired onto a 16-bit bus, you just wire 6 of them to the upper byte and the other 6 to the lower byte. Lane select is easy for byte writes because you can enable one or both of the chips.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I gave it a shot. Everything seems to be working still. Checked endianness of both the RAM and data being read from flash and everything is looking how it should still.

e142229

Copy link
Member

Choose a reason for hiding this comment

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

I’ll do a bit of cleanup and push to this PR branch so you can re-test it.

src/devices/sound/stt_sa1.cpp Outdated Show resolved Hide resolved
src/devices/sound/stt_sa1.cpp Show resolved Hide resolved
src/devices/sound/stt_sa1.h Outdated Show resolved Hide resolved
src/mame/misc/sttechno.cpp Outdated Show resolved Hide resolved
src/mame/misc/sttechno.cpp Outdated Show resolved Hide resolved
987123879113 and others added 3 commits January 15, 2024 03:30
* bus/ata/atapihle.cpp: Add check for changing configuration after
  system begins to start.
* sound/stt_sa1.cpp: Use 16-bit sample space, mask read/write offsets.
* misc/sttechno.cpp: Use individual Flash write enables.
@cuavas
Copy link
Member

cuavas commented Jan 15, 2024

@987123879113 can you please pull my changes and give it a bit of a test? I did run it and it seemed to work, but I don’t know how to play and I can’t be bothered learning right now.

Also, given this is a rhythm game, do the guessed screen parameters and periodic interrupt warrant flagging it as imperfect timing, or are we good with just imperfect sound?

cuavas and others added 2 commits January 16, 2024 05:02
- misc/sttechno.cpp: Added MACHINE_IMPERFECT_TIMING due to using guessed timings, removed unused parameter and fixed endianness of data written to sound RAM
@987123879113
Copy link
Contributor Author

@cuavas I fixed some small things. The endianness of the data for the sound RAM is a little tricky. One of them needs to be swapped and I think the stuff being written into m_sound_ram is easier to implement unless there's an easy way to change endianness of the intelfsh16_device::read_raw mappings in sound_map.

As for MACHINE_IMPERFECT_TIMING, sure why not. The timings are best effort guesses right now and I think it's pretty close right now but there was still a lot of guessing involved there. If someone can ever figure out the real raw screen parameters and figure out exactly how IRQ6 is supposed to actually work then I think it can be removed.

Just to be completely transparent about how I'm verifying the endianness here:
In :pcm_sound memory space...
At 0x751b70 is the data that corresponds to SE/a440_11.sdx on the CD
image

At 0x800 is the data that corresponds to the start of the BGM samples for one of the channels (assumed L, doesn't matter for verification). You can go into the song selection screen and hover over Diamonds to make it load MUSIC/diamonds.sdx from the CD.
image

The SE files get written to the flash during the first boot. The BGMs are always read in from the CD.

Here's how it looks in the MAME debugger with this latest change.
image

@cuavas
Copy link
Member

cuavas commented Jan 15, 2024

Oh, weird – opposite endianness on Flash and RAM from the CPU’s point of view is not something I would have expected.

@cuavas cuavas merged commit 32e13b0 into mamedev:master Jan 15, 2024
5 checks passed
@987123879113 987123879113 deleted the shambros branch February 17, 2024 18:04
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

2 participants