bsnes icon indicating copy to clipboard operation
bsnes copied to clipboard

CPUMSC reset test doesn't work

Open orbea opened this issue 3 years ago • 25 comments

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?

orbea avatar Jan 22 '22 02:01 orbea

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

Screwtapello avatar Jan 22 '22 04:01 Screwtapello

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?

orbea avatar Jan 22 '22 04:01 orbea

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?

Screwtapello avatar Jan 22 '22 05:01 Screwtapello

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

orbea avatar Jan 22 '22 12:01 orbea

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

Screwtapello avatar Jan 22 '22 13:01 Screwtapello

Can anyone find where this broke?

It is working with v070, @orbea!

image

Max833 avatar Jan 23 '22 14:01 Max833

@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.

orbea avatar Jan 23 '22 14:01 orbea

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

Max833 avatar Jan 23 '22 15:01 Max833

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?

orbea avatar Jan 23 '22 15:01 orbea

image

Max833 avatar Jan 23 '22 19:01 Max833

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.

orbea avatar Jan 23 '22 19:01 orbea

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

jbo-85 avatar Jan 26 '22 04:01 jbo-85

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.

orbea avatar Jan 26 '22 15:01 orbea

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

jbo-85 avatar Jan 26 '22 16:01 jbo-85

@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 avatar Jan 26 '22 17:01 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.

orbea avatar Jan 26 '22 18:01 orbea

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

jbo-85 avatar Jan 26 '22 21:01 jbo-85

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.

orbea avatar Jan 26 '22 22:01 orbea

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

jbo-85 avatar Jan 26 '22 23:01 jbo-85

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

Max833 avatar Jan 27 '22 17:01 Max833

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

jbo-85 avatar Jan 28 '22 01:01 jbo-85

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 avatar Jan 28 '22 15:01 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?

orbea avatar Jan 28 '22 16:01 orbea

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

Max833 avatar Jan 28 '22 16:01 Max833

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

Max833 avatar Mar 04 '23 17:03 Max833