CPUMSC reset test doesn't work

[orbea]

bsnes: https://github.com/bsnes-emu/bsnes/commit/6fc6bf14a39d32dab69c4f9687a81df26d412758

https://github.com/PeterLemon/SNES/blob/master/CPUTest/CPU/MSC/CPUMSC.sfc

This rom has a test for the snes reset feature which does not work, instead of resetting the system and showing that the test has passed it will get stuck displaying garbage graphics.

It works on the sd2snes, has been reported as working in bsnes sometime in the past and it still works in the mednafen bsnes fork. After some effort I was able to compile back to v070r1 (da5263bfc3088ded5e18eca11a2746aa91071415) without finding a good commit.

Can anyone find where this broke?

[Screwtapello]

How difficult is it to build v070r1 these days? What stopped you from going further?

[orbea]

I built a copy of gcc-4.9.4, SDL1, disabled most of the drivers and commented out problematic parts of the input code that were not relevant for the test. I also had to use the SDL1 video driver for the test rom to even start.

I could build v070, but it had no reset exposed in the phoenix ui. I think I would need an ancient Qt version to go farther which may be difficult.

There might be easier ways to narrow this down?

[Screwtapello]

Oh wow, that's intense. Thanks for doing that work!

The tukuyomi SNES archive includes pre-built Windows binaries for some versions of bsnes, but not all of them, and I think not even all the public releases. At the very least it might help narrow things down a bit?

[orbea]

That might indeed help, but unfortunately I can't do it since I have no windows install.

[Screwtapello]

They should work pretty well in Wine, if you have that.

[Max833]

Can anyone find where this broke?

It is working with v070, @orbea!

[orbea]

@Max833 Can you try find the first version that this broke? I haven't tried wine yet.

As it turns out a friend already tried these windows binaries and came up with different results than I did on linux where v070r16a worked, but not v071. It was also not at all clear what the difference between those two versions were that resulted in the test breaking.

[Max833]

I'm sorry. I can't build a different version. But yes, indeed, v070r16a worked, but not v071.

The only thing I can give you is the v070r17 changelog. Maybe @jbo-85 can help us with this.

https://github.com/bsnes-emu/bsnes/commit/4016ae1d432e41e15e6f1e7fa045612f338f62af

[orbea]

Thanks for trying! Given we are clearly getting different results on linux and windows there are a few hypotheses.

• This is related to the frontend?
• There was some kind of UB or compiler bug that changed which this test depended on?
• The executables do not actually correspond to the labeled version number?

[Max833]

[orbea]

Doesn't seem to be the case.

https://github.com/bsnes-emu/bsnes/blob/6fc6bf14a39d32dab69c4f9687a81df26d412758/bsnes/sfc/cpu/cpu.cpp#L95

My guess is that the value is not being set correctly or is being lost somewhere else.

[jbo-85]

I could reproduce it with higan 094 on Linux, it works with profile=balanced and breaks with profile=accuracy I also tested it with higan 097, but it seemed to break even on balanced

[orbea]

The default profile was changed for v071 from performance to accuracy which is where it stopped working with the windows binaries as reproduced by @Max833.

https://github.com/bsnes-emu/bsnes/commit/6c3aec7dc96affaf7ef7d1ea4b293407f2309f91#diff-e8793b471442b1f470d0cabc5566c3ecf8bfb15069e0d8c748f4b613422fd920L3

Of course v070r17 doesn't have an executable to confirm it wasn't broken earlier.

[jbo-85]

v070r17 is basically the same as v070r16a, qoute from the old bboard: "No binaries, basically the same as v070r16a on Google Code, only this one has source code" the default profile was changed from compatibility to performance in v070r1 and from performance to accurary in v071, so it only works with the compatibility profile

[orbea]

@jbo-85 Are you able of finding which commit broke it for the balanced profile? I tried and was unable to because of unrelated issues.

v095r10 the test passes with the balanced profile v095r11 - r16 I get a build error related to nall::function and the balanced ppu which is not obvious how to resolve. v095r17 - v097 I get a black screen when starting the test rom.

[orbea]

Using v095r10 with the accuracy profile will cause the test to fail, but when using the balanced ppu instead of the accurate ppu will allow the test to pass.

Maybe its not that the test fails, but its that the graphics are breaking?

Edit: Likewise the balanced profile will fail if the accurate ppu is used.

[jbo-85]

v098b (commit aff00506c5b2a988ea8f119b17ed27e38cc23334) was the last commit to work with the balanced profile, after that the balanced/performance profiles were removed :) My test with v097 was flawed, since the executable was named tomoko, at that time, and not higan

[orbea]

Thanks for finding that out, I suppose someone will need to figure out how to make this work with the accuracy profile the hard way. At least it seems like the issue can be narrowed down to the ppu.

[jbo-85]

The problem can also be reproduced in bsnes-plus when you build with "profile=accuracy"

[Max833]

And it doesn't work with the bsnes v115 balanced build as well (enhancements -> fast mode).

[jbo-85]

The problem is that more registers are resetted than on real hardware The problematic line for ppu-fast is https://github.com/bsnes-emu/bsnes/blob/master/bsnes/sfc/ppu-fast/ppu.cpp#L193 It's spread out in more files for the accuracy ppu. I'm not sure which registers shouldn't be resetted though

[orbea]

I found a working solution by wrapping these lines with if(!reset).

https://github.com/bsnes-emu/bsnes/blob/6fc6bf14a39d32dab69c4f9687a81df26d412758/bsnes/sfc/ppu/ppu.cpp#L167

https://github.com/bsnes-emu/bsnes/blob/6fc6bf14a39d32dab69c4f9687a81df26d412758/bsnes/sfc/ppu/ppu.cpp#L177-L184

To be exact only bg1.power(), io.pseudoHires and the following lines are needing and it will work maybe 99% of the time...

https://github.com/bsnes-emu/bsnes/blob/6fc6bf14a39d32dab69c4f9687a81df26d412758/bsnes/sfc/ppu/object.cpp#L167-L199

https://github.com/bsnes-emu/bsnes/blob/6fc6bf14a39d32dab69c4f9687a81df26d412758/bsnes/sfc/ppu/screen.cpp#L166

https://github.com/bsnes-emu/bsnes/blob/6fc6bf14a39d32dab69c4f9687a81df26d412758/bsnes/sfc/ppu/window.cpp#L50-L56

https://github.com/bsnes-emu/bsnes/blob/6fc6bf14a39d32dab69c4f9687a81df26d412758/bsnes/sfc/ppu/window.cpp#L82-L101

Hiding the entire block makes it seem to work all of the time, but this really needs confirmation with real hardware to be sure. I tested a few real games too without issues, but it would at least need better testing.

[orbea]

Its fixed in the jgemu bsnes fork.

https://gitlab.com/jgemu/bsnes/-/commit/3227e0102f1453b8ff2e9217c7a826d7fb38874a

I am not sure if @Screwtapello would prefer to proceed with this solution now or wait for further hardware confirmation?

[Max833]

The problem is that not many people are capable doing hardware confirmation test roms. @undisbeliever and @PeterLemon are the only one I know.

[Max833]

It's fixed on ares. https://github.com/ares-emulator/ares/commit/0a6147c177e504eca78db7b248fbb7a2aade45b9