search

mrehkopf / sd2snes

SD card based multi-purpose cartridge for the SNES
http://sd2snes.de
GNU General Public License v2.0
590 stars 115 forks source link

MSU-1 Check State bug (affects both PRO and OG models) #138

Closed ghost closed 3 years ago

ghost commented 4 years ago

Hi,

just wanted to know whether this bug may already been addressed? http://krikzz.com/forum/index.php?topic=9737.0

The author says

It was also fine in 0.1.7d, but was broken again with the release of 0.1.7e

So I looked into msu.h and found this difference between these versions:

define MSU_INT_STATUS_SET_CTRL_PENDING (0x0001)

define MSU_SNES_STATUS_SET_AUDIO_PLAY (0x0002)

define MSU_SNES_STATUS_SET_AUDIO_REPEAT (0x0004)

define MSU_SNES_STATUS_SET_AUDIO_ERROR (0x0008)

define MSU_SNES_STATUS_SET_DATA_BUSY (0x0010)

define MSU_SNES_STATUS_SET_AUDIO_BUSY (0x0020)

define MSU_INT_STATUS_CLEAR_CTRL_PENDING (0x0100)

define MSU_SNES_STATUS_CLEAR_AUDIO_PLAY (0x0200)

define MSU_SNES_STATUS_CLEAR_AUDIO_REPEAT (0x0400)

define MSU_SNES_STATUS_CLEAR_AUDIO_ERROR (0x0800)

define MSU_SNES_STATUS_CLEAR_DATA_BUSY (0x1000)

define MSU_SNES_STATUS_CLEAR_AUDIO_BUSY (0x2000)

Maybe here was mixed something up and lead to that issue? I do not know what this code exactly does but if audio error refers to bit 8, audio playing should be bit 10 and not bit 2 (that is however wild guessing. If I look into snes9x git https://github.com/snes9xgit/snes9x/blob/master/msu1.h , the entries are the following:

enum SMSU1_FLAG { Revision = 0x07, // bitmask, not the actual version number AudioError = 0x08, AudioPlaying = 0x10, AudioRepeating = 0x20, AudioBusy = 0x40, DataBusy = 0x80 };

which corresponds to the flags in https://helmet.kafuka.org/msu1.htm

7 6 5 4 3 2 1 0
Data busy Audio busy Audio repeat Audio playing Track missing Revision

This could be however something completely else...

gizaha commented 4 years ago

I guess these bits have internal usage. That's why it has SET_DATA_BUSY and CLEAR_DATA_BUSY. There unrelated with the usual msu status bits. Also, msu status bits are 8bit while these are 16bit.

mrehkopf commented 4 years ago

@gizaha 's assumption is correct, the MSU<->SNES interface bits are not mapped the same way as the Microcontroller<->FPGA interface bits. How much time passes after sending the PLAY command before the AudioPlaying bit is tested? There is a certain round trip time before the AudioPlaying bit is set after receiving a PLAY command (because the FPGA signals it to the MCU which needs to take note of it, start audio playback, and set the corresponding status bit in the FPGA again). So if it's tested immediately (less than say 20µs) it might not yet be set.

mrehkopf commented 4 years ago

(Right now I don't see any harm in setting the playback flags directly by the FPGA after the command has been written, without the round trip through the MCU. Error checking should have taken place already by the track request itself...)

mrehkopf commented 3 years ago

@Conn79 the relevant change is probably this one: 88d876d Track change is supposed to clear the play/repeat status flags which is fixed by the above commit. As I mentioned the PLAY flag takes a while (probably a couple of µs) to be set after the PLAY command is sent. If you check the PLAY flag too soon after sending the PLAY command it might not yet be set, so a distinction would have to be made between "not playing yet" and "not playing anymore".