PSX Memory Cards: Sourcing SIO Data Values
#1
I apologize in advance if this isn't a fitting place for this discussion; I feel like Github isn't the place for casual discussion. Let me know if this needs moved.

Why I started this nonsense:
Savestate stability problems after many many cumulative hours played (suggests bad game design causing memory leaks or gradual destabilization of savestates for PSX).

The short of what I've already done:
Modified GUI to allow PSX memory card creation. The PSX memory card code already existed, all I had to do was make a way to create them and plug in a few numbers for the card size. BIOS recognizes it, allows formatting, no problems in the memory cards GUI either.

The problem:
The one PSX game I have been able to boot without too much fuss, Digimon World 3, still will not detect PSX memory cards. Upon investigation in Sio.cpp (debug messages more plentiful than nitrogen in our atmosphere), PSX load/save operations seem to only fire the various SIO functions with a data value of 129 in decimal, or 0x81 hexadecimal. From what I can tell, the proper PSX data values should be handled if encountered (taken from "SIO_WRITE sioWriteMemcard(u8 data)"):
Code:
// If the PS2 commands fail, it falls back into PSX mode
case 0x52: // PSX 'R'ead
case 0x53: // PSX 'S'tate
case 0x57: // PSX 'W'rite
siomode = SIO_MEMCARD_PSX;
break;
Notable, the "SIO_WRITE sioWriteMemcard(u8 data)" function contains a case for this data value (0x81), but it has no break, and overflows down to share a result with several other cases:
Code:
case 0x81: // Checked right after copy/delete
case 0xBF: // Wtf?? On game booting?
case 0xF3: // Reset?
case 0xF7: // No idea
MemcardResponse();
siomode = SIO_DUMMY;
break;
Another notable thing is that there is also a "SIO_WRITE sioWriteMemcardPSX(u8 data)", however I do not see calls to this while running a PSX game. Funny enough, I saw a few stray calls tossed in while running a PS2 game, though.

The last note, and a bit of an offhand one, is that the data value 66 (0x42) is CONSTANTLY, regardless of whether a load/save is being attempted, being plugged into "static void sioWrite8inl(u8 data)". The only time I see this hex as a case is in "SIO_WRITE memcardWrite(u8 data)":
Code:
case 0x42: // Write
memcpy(sio.buf, header, 4);
once = true;
break;
I wonder, in the back of my mind, if this means anything.

Some attempted solutions:
Forcefully changing the siomode variable to try and steer control into "sioWriteMemcardPSX", away from "sioWriteMemcard". Sure enough, the function calls changed, but no other behavior changes. After all, the data being passed in wasn't changing.

Breakpointing and following the call stack. However, the call stack falls out of scope rather quickly.
[Image: YOoAeyy.png]
Debug messages on both of these functions indicates that 129 (0x81) is being passed in to both; this number is originating from something deeper and passing through these unmodified. While I did find instances in PCSX2 code where "sioWrite8inl" was called, attempting to breakpoint these yielded no result, and debug messages had no console output.

The question:
Does anyone have any insight as to what the hell could be making these calls? I'm led to believe something "down in the metal" is not working as intended and only making these calls with 129 as the data. I would like to find out why but if the call stack is going to drop off like this and only leave me with assembly code, I don't think that's feasible. 

The other thing is, remember that offhand comment about 66 (0x42)? Could that be more important than I think? The rapid fire calls makes me think perhaps it is, but I have no idea why it would be called outside of a save/load operation, then.

edit: swapped screenshot with one that isn't murdered by display scaling
Reply

Sponsored links

#2
Before PSX mode was implemented, there was nothing to get SIO working with it.
All you see now is from quick hack attempts to make games happy and boot further.
So as soon as you're in PSX mode, you should expect all SIO code to be wrong.
If you want to work on it, try it with the game Saga Frontier (NTSC-U). I had it saving and loading games at one point.
Find out what it's doing 'right' and how other titles differ from it, then fix all the bugs ;p

Another hint:
SIO has interrupt delays defined somewhere. Every PSX title required the delay to be set, iirc.
Check "#define SIO_INLINE_IRQS" and comment it out. That should do it.
Reply
#3
(08-27-2017, 01:29 PM)rama Wrote: Before PSX mode was implemented, there was nothing to get SIO working with it.
All you see now is from quick hack attempts to make games happy and boot further.
So as soon as you're in PSX mode, you should expect all SIO code to be wrong.
If you want to work on it, try it with the game Saga Frontier (NTSC-U). I had it saving and loading games at one point.
Find out what it's doing 'right' and how other titles differ from it, then fix all the bugs ;p

Another hint:
SIO has interrupt delays defined somewhere. Every PSX title required the delay to be set, iirc.
Check "#define SIO_INLINE_IRQS" and comment it out. That should do it.

So, it turns out your "Another Hint" was "The Solution". Sure enough, comment out the define, and magically the card starts reading and writing. Save and load both worked (though slow because of the sheer volume of console messages I have bogging down the thread). Will have to cut those back out.

Will be doing some more experimentation to verify data integrity but... if this works out, is this worth a PR to Github?

Edit: Actually, I want to take into consideration your comment in here first.
Code:
// Let's enable this to free the IOP event handler of some considerable load.
// Games are highly unlikely to need timed IRQ's for PAD and MemoryCard handling anyway (rama).
//#define SIO_INLINE_IRQS
Now, I'm not seeing any adverse effects so far in PS2 titles, but this could be a regression for a lot of games if this is just left as is without any sort of PSX/PS2 checking, right?
Reply
#4
One problem is that there's still this wrong SIO implementation. Even with SIO delays enabled, some PSX games can't access Memory Cards (FF8) and some games now don't start anymore (Saga Frontier 2).
Also, SIO is combined with SIO2 and the delays are somehow required for PSX mode, yet not required (bad even?) for PS2 mode.
We can't just enable the delays again and hope for the best. Some PS2 game is surely going to break.

But it would be nice if you shared a patch or opened a PR on GitHub for that switch-case bug you found.
Next, we need some data on what PSX games expect from SIO. Maybe capture the communication from FF8 on a PSX and then a PS2.
And I think Wisi is also still working on reversing the hardware. His PGIF fixes would be very welcome as well.
Reply
#5
(08-28-2017, 10:24 AM)rama Wrote: One problem is that there's still this wrong SIO implementation. Even with SIO delays enabled, some PSX games can't access Memory Cards (FF8) and some games now don't start anymore (Saga Frontier 2).

So I got lucky that the game I am testing with was one which could even run, let alone save and load. Fair enough. Could, for these other games you mention, the core problem be related to the data values that are being plugged into the SIO functions? Digimon World 3 was, without delays enabled, only calling SIO functions with 129 (decimal), but suddenly with delays enabled was throwing out the right numbers to get the memory card to respond, though I need to re-add my debug messages to check what exact numbers are throwing. After getting those messages back, I'll have a better idea what numbers actually matter. If other games make SIO calls with other numbers, then the scope of the PSX cases in the switch just needs enlarged. Cue research.

(08-28-2017, 10:24 AM)rama Wrote: Also, SIO is combined with SIO2 and the delays are somehow required for PSX mode, yet not required (bad even?) for PS2 mode.
We can't just enable the delays again and hope for the best. Some PS2 game is surely going to break.

So this is a regression. More of a C++ question as I haven't really worked with preprocessor directives before; is there any reason I couldn't make the #define conditional, say...
Code:
if (isPS2Game) {
   #define
}

as a baseline? Then going forward if more and more games show up needing special treatment, add a function that looks up the title and determine whether to enable delays?

(08-28-2017, 10:24 AM)rama Wrote: But it would be nice if you shared a patch or opened a PR on GitHub for that switch-case bug you found.

I can certainly open an issue discussing the switch values, but as far as having a PR, everything we've discussed to this point says that should wait. In my case with one game, the bug fix was removing the #define. With the #define commented out the whole thing is basically one big hack right now, so a little more "infrastructure buildup" needs to happen before any PRs start flying. But yes, I can dump all the info I have on data values into a Github issue once I figure out which ones DMW3 is using, and maybe submit a PR to the issue once it is cleaned up.

(08-28-2017, 10:24 AM)rama Wrote: Next, we need some data on what PSX games expect from SIO. Maybe capture the communication from FF8 on a PSX and then a PS2.
And I think Wisi is also still working on reversing the hardware. His PGIF fixes would be very welcome as well.

While I don't own FF8, I can keep poking DMW3 around in my VS debugger. The only other PSX title I have that boots (console output on the others implies the rips missed a few sectors) is, hilariously enough DMW2, but I haven't seriously played that one to the point of saving being an option. That said, I will try, and document it as well in the issue.

Edit: Upon further evaluation of the curious case of DMW3, I made a startling discovery that completely broke my understanding of how these SIO functions are being called. It turns out, depending on which of three save files I choose to load, the starting data value pushed into SIO code changes, and as the operation runs, it increases, looping back after "overflowing" at 255. I was interpreting these as "commands", with how the switches were laid out, but here it is just chugging through them as if they are memory addresses... More to come as I keep messing around.
Reply
#6
(08-29-2017, 12:52 AM)pandubz Wrote: So I got lucky that the game I am testing with was one which could even run, let alone save and load. Fair enough. Could, for these other games you mention, the core problem be related to the data values that are being plugged into the SIO functions? Digimon World 3 was, without delays enabled, only calling SIO functions with 129 (decimal), but suddenly with delays enabled was throwing out the right numbers to get the memory card to respond, though I need to re-add my debug messages to check what exact numbers are throwing. After getting those messages back, I'll have a better idea what numbers actually matter. If other games make SIO calls with other numbers, then the scope of the PSX cases in the switch just needs enlarged. Cue research.


So this is a regression. More of a C++ question as I haven't really worked with preprocessor directives before; is there any reason I couldn't make the #define conditional, say...
Code:
if (isPS2Game) {
   #define
}

as a baseline? Then going forward if more and more games show up needing special treatment, add a function that looks up the title and determine whether to enable delays?


I can certainly open an issue discussing the switch values, but as far as having a PR, everything we've discussed to this point says that should wait. In my case with one game, the bug fix was removing the #define. With the #define commented out the whole thing is basically one big hack right now, so a little more "infrastructure buildup" needs to happen before any PRs start flying. But yes, I can dump all the info I have on data values into a Github issue once I figure out which ones DMW3 is using, and maybe submit a PR to the issue once it is cleaned up.


While I don't own FF8, I can keep poking DMW3 around in my VS debugger. The only other PSX title I have that boots (console output on the others implies the rips missed a few sectors) is, hilariously enough DMW2, but I haven't seriously played that one to the point of saving being an option. That said, I will try, and document it as well in the issue.

To answer your c++ question yes that would compile but isn't great practice, if what you're needing is numerical, consider instead a constant int or double.
Reply
#7
(08-29-2017, 02:55 AM)kenshen Wrote: To answer your c++ question yes that would compile but isn't great practice, if what you're needing is numerical, consider instead a constant int or double.

It is not numerical; it is actually quite simply "#define SIO_INLINE_IRQS", checked later with an #ifdef.

Counter argument:

In another file I spotted "#define IS_LAST_BYTE_IN_PACKET ((sio.bufCount >= 3) ? 1 : 0)"

I thought preprocessors were parsed before actual C++ compilation started, so is C++ code being thrown in to a preprocessor define like this going to work like I would want it to? If so, I may be able to surround the #define with an #if, and problem solved.
Reply
#8
This is probably a macro. The preprocessor replaces all occurences of IS_LAST_BYTE_IN_PACKET with the code that follows.

For SIO, you should expect commands to be mixed with data. That interleaving should follow a pattern and that pattern has to be implemented correctly.
There's a problem in the details though:
We know pretty accurately what a PSX would use. We also know fairly well what a PS2 in PS2 mode would use.
What we (I) don't know is what a PS2 in PSX mode does. How is it adressing SIO(1) instead of SIO2? Are these even different devices?
Are they one device with 2 operation modes? If so, how is it implemented?
Reply
#9
(08-29-2017, 03:02 PM)rama Wrote: This is probably a macro. The preprocessor replaces all occurences of IS_LAST_BYTE_IN_PACKET with the code that follows.

For SIO, you should expect commands to be mixed with data. That interleaving should follow a pattern and that pattern has to be implemented correctly.
There's a problem in the details though:
We know pretty accurately what a PSX would use. We also know fairly well what a PS2 in PS2 mode would use.
What we (I) don't know is what a PS2 in PSX mode does. How is it adressing SIO(1) instead of SIO2? Are these even different devices?
Are they one device with 2 operation modes? If so, how is it implemented?

This is probably not the answer you want to hear, but for that question I am at a loss other than my own inference based on what I've seen. That inference being that SIO1 and 2 are different modes in one system. While the deeper PSX and PS2 SIO code is separated, the calls still filter through the sioWriteStart and sioWrite8inl functions before all else. They at least have that in common. And from what I can tell with my limited game library, they seem to play nicely when set up like this.

The trials that I have done and the lack of any major problems (exception being the define for the delays) tells me the current implementation is accurate enough that simpler games can save and load fine. And the reality is, I'm at a dead end here because the limited game sample I have is working without issue. So, for my own efforts I am shifting gears.

I made a proper fork today and pushed a commit with my changes to it. In case you're interested, it is here: https://github.com/RedPanda4552/pcsx2

The TL;DR of the changes:

- Modify the GUI to allow PSX card creation. A quick solution, not the most visually pleasing thing.
- Comment out #define SIO_INLINE_IRQS. Seems to be the magic switch for my two working PSX games, but as discussed this could adversely affect many other games.
- Add debug messages such that:
  1. Each call to sioWriteStart with data = 129 (0x81, the code to indicate memory card IO) sends a console message. These calls indicate a new "cycle"; each time the game wants to do some sort of IO, this function is at the front of it all.
  2. In sioWrite8inl, each case in the switch related to memory cards has a debug message on it that will output the siomode of that case, the current data parameter, and the buffer count. Buffer count is weird; I do not know if I made a mistake with my choice of variable but I am seeing a lot of scientific notation and floating points where I was only expecting to see integers.
A word of warning if you compile and run: as you would expect, the debug messages slow down the emulator substantially, and absolutely flood the console. Pausing the emulator is a must in some cases if you want a chance at reading output.

The current changes I have are functioning and the limited library of games I have are all handling any abuse I can think to throw at them. Currently I think priority one for myself is how to replace "#define SIO_INLINE_IRQS" with a more adaptive option; ideally it should adjust based on whether a PSX or PS2 game is running. That way I can in good conscience submit a PR, have a base line going and make it so others can reliably get on board with this. The problem is, being preprocessor directives, I will need to rewrite it such that it is able to enable/disable delays at runtime based on a flag. Which, I am not sure if I can or how to do so. Cue experimentation.
Reply
#10
(08-30-2017, 03:21 AM)pandubz Wrote: This is probably not the answer you want to hear, but for that question I am at a loss other than my own inference based on what I've seen. That inference being that SIO1 and 2 are different modes in one system. While the deeper PSX and PS2 SIO code is separated, the calls still filter through the sioWriteStart and sioWrite8inl functions before all else. They at least have that in common. And from what I can tell with my limited game library, they seem to play nicely when set up like this.

The trials that I have done and the lack of any major problems (exception being the define for the delays) tells me the current implementation is accurate enough that simpler games can save and load fine. And the reality is, I'm at a dead end here because the limited game sample I have is working without issue. So, for my own efforts I am shifting gears.

I made a proper fork today and pushed a commit with my changes to it. In case you're interested, it is here: https://github.com/RedPanda4552/pcsx2

The TL;DR of the changes:

- Modify the GUI to allow PSX card creation. A quick solution, not the most visually pleasing thing.
- Comment out #define SIO_INLINE_IRQS. Seems to be the magic switch for my two working PSX games, but as discussed this could adversely affect many other games.
- Add debug messages such that:
  1. Each call to sioWriteStart with data = 129 (0x81, the code to indicate memory card IO) sends a console message. These calls indicate a new "cycle"; each time the game wants to do some sort of IO, this function is at the front of it all.
  2. In sioWrite8inl, each case in the switch related to memory cards has a debug message on it that will output the siomode of that case, the current data parameter, and the buffer count. Buffer count is weird; I do not know if I made a mistake with my choice of variable but I am seeing a lot of scientific notation and floating points where I was only expecting to see integers.
A word of warning if you compile and run: as you would expect, the debug messages slow down the emulator substantially, and absolutely flood the console. Pausing the emulator is a must in some cases if you want a chance at reading output.

The current changes I have are functioning and the limited library of games I have are all handling any abuse I can think to throw at them. Currently I think priority one for myself is how to replace "#define SIO_INLINE_IRQS" with a more adaptive option; ideally it should adjust based on whether a PSX or PS2 game is running. That way I can in good conscience submit a PR, have a base line going and make it so others can reliably get on board with this. The problem is, being preprocessor directives, I will need to rewrite it such that it is able to enable/disable delays at runtime based on a flag. Which, I am not sure if I can or how to do so. Cue experimentation.
with that define option, why don't you make a separate class with the specific define and any other unique psx code then wrap that into the if statement? in the meantime I have several games in my psx library namely most of the spyro games and medieval and metal gear solid that not only boot but are perfectly playable in pcsx2 as it is right now so I'll test your build.
Reply




Users browsing this thread: 1 Guest(s)