hrydgard / ppsspp

A PSP emulator for Android, Windows, Mac and Linux, written in C++. Want to contribute? Join us on Discord at https://discord.gg/5NJB6dD or just send pull requests / issues. For discussion use the forums at forums.ppsspp.org.
https://www.ppsspp.org
Other
11.04k stars 2.15k forks source link

CPU bug: Corrupted video in games with software codec #464

Closed hrydgard closed 10 years ago

hrydgard commented 11 years ago

(Known games with this issue: Lego Harry Potter (both), Phineas & Ferb, Army of Two)

A few games don't use the PSP hardware decoder for video but instead supply their own software decoder which runs on the main CPU/VFPU.

These videos currently look corrupt in PPSSPP and the only cause I can imagine is bugs in the CPU emulation.

The inner DCT uses the following VFPU instructions:

lv.q vs2i.p vi2f.q vmul.q vs2i.p vsub.q vadd.q vscl.q vpfxd 0-1 x 4 (before vscl.q) vscl.q vf2iz.q vi2uc.q sv.s

However, all of these are fairly well tested or trivial...

An additional issue is that I can't seem to detect any cache clearing operation which can be used to zap the video texture... the videos also have issues with frame flicker due to really weak texture hashing so they repeat frames etc.

unknownbrackets commented 11 years ago

There is a "cache" instruction which appears to writeback the cache etc. (it has different modes or something, though.) Currently it's a noop. But it's not used in there?

Does JPCSP render the video right? Does it read/write to VRAM directly?

Also, I assume it's the same in interpreter vs. jit?

-[Unknown]

cloud1250x4 commented 11 years ago

Army of two have a problem with video too

hrydgard commented 11 years ago

It's wrong in both JIT and interpreter. It writes the video image to RAM. The corruption symptoms are somewhat similar to the issues we had when "paired single" fp instructions had bugs in Dolphin so CPU bugs seems very likely. Added some more cpu tests to pspautotests but failed to unearth any bugs.

The cache instruction is used there but only in a mode that does not writeback (!).

Haven't tried them in JPCSP yet.

unknownbrackets commented 11 years ago

Hmm. If it's only a few different instructions, maybe dump the ops the interpreter runs to an .S file (replacing memory reads/writes with immediates) and see if you can find where it deviates from the PSP? Sounds like a pain.

-[Unknown]

hrydgard commented 11 years ago

Yup, that's on the todo list.

cloud1250x4 commented 11 years ago

just to be sure that army of two do have the same problem...

here's a video: http://www.youtube.com/watch?v=4ODiYdbhEYo

here we go ;) I've tested the game with jpcsp and no problem at all ;)

unknownbrackets commented 11 years ago

It doesn't use vcmp, right? Just those? I suspect vcmp has some bugs, with NaN and some of the e.g. VC_NN (which I assume is !isnan, and that's what jpcsp does)... but not sure.

-[Unknown]

unknownbrackets commented 11 years ago

Looks like LittleBigPlanet may have this same problem (or a similar one, I haven't checked the instructions but it has a video it plays which has these problems.) However, it needs some hackery to get there.

-[Unknown]

cloud1250x4 commented 11 years ago

http://www.youtube.com/watch?v=5fP7AQKTdA8&feature=youtu.be

for lbp, I didn't use the cso format and didn't remove anything in the game

unknownbrackets commented 11 years ago

Messed with this a bit in LittleBigPlanet. Some notes:

-[Unknown]

hrydgard commented 11 years ago

Nice work, too bad about the results :)

Got any more examples of differences to JPCSP? Should add vscl with prefix to the test (which needs to be expanded a lot in various other ways too, to be more exhaustive).

Could very well be a MIPSTables bug. Those should be detectable by simply expanding the autotest to cover all of these well.

PowerPC has the ability to simply zero a cache line, which was used by various codecs running on the GC/Wii, but no MIPS manual I can find seem to mention a similar capability.

I highly doubt it's a VRAM issue, if so we'd be seeing a lot worse things than macroblocks with the wrong brightness which is what most of our issues look like.

NaN handling also appears unlikely to matter in a video codec.

unknownbrackets commented 11 years ago

Other differences were mostly:

-[Unknown]

hrydgard commented 11 years ago

I don't think prefixes apply to matrix operations, only vector ones. Regarding EatPrefixes, JPCSP may very well be right.

unknownbrackets commented 11 years ago

Looks suspicious:

    INSTR("lv", &Jit::Comp_SVQ, Dis_SVLRQ, Int_SVQ, IS_VFPU),
    INSTR("lv.q", &Jit::Comp_SVQ, Dis_SVQ, Int_SVQ, IS_VFPU), //copU
...
    INSTR("sv", &Jit::Comp_SVQ, Dis_SVLRQ, Int_SVQ, IS_VFPU), //copU
    INSTR("sv.q", &Jit::Comp_SVQ, Dis_SVQ, Int_SVQ, IS_VFPU),

But I don't think I hit those.

-[Unknown]

hrydgard commented 11 years ago

That's just lvl.q/lvr.q and svl.q/svr.q (analogous to lwl and lwr).

Dunno about the "copU" comment though.

unknownbrackets commented 11 years ago

Right, I meant, pretty sure we don't handle the unaligned reads/writes properly (at least in jit.)

-[Unknown]

unknownbrackets commented 11 years ago

Oh, well, it works for 4-byte aligned stuff and I guess that's probably required. My bad.

-[Unknown]

unknownbrackets commented 11 years ago

Hmm, I'm finding some bugs (many of which JPCSP also has or has instead), but nothing seems to help the video...

vscl.q is interesting. It appears to be affected by the T prefix, but does some strange stuff. [1/3] is treated as 3, and if you do e.g. |x|, |y|, |z|, |w| (on T, not on S), it absolute values the scaled results. gas also refuses to apply a T prefix without using vpfxt directly.

Also, misuse of the swizzle appears to vary. vmov.p treats S prefix [w, w, w, w] as [0, 0], while vscl.q treats T prefix [w, w, w, w] like [x] (I think.)

And, seems like vmov.q does in fact eat the T prefix (even though it has no T.)

-[Unknown]

hrydgard commented 11 years ago

Heh, funny stuff.

As I said in the other thread but repeat here just for the record, I don't think it's worth implementing such quirks (especially those that gas won't even assemble!), we should LOG_ERROR them and only handle them if a game really makes use of out of range prefixes.

unknownbrackets commented 11 years ago

It seems that Wipeout Pure uses vscl with T prefixes, even as much as gas doesn't allow that.

-[Unknown]

hrydgard commented 11 years ago

Huh, alright then...

unknownbrackets commented 11 years ago

LittleBigPlanet at least appears to work fine from a build on Linux with gcc. Potentially this may be a bug in MSVC unless it's reproducing on other platforms / with other compilers.

-[Unknown]

hrydgard commented 11 years ago

Wow, that's nasty, if it even miscompiles in debug mode on MSVC...

Also quite interesting...

cloud1250x4 commented 11 years ago

no more problem in lbp and in army of two, dunno since when

unknownbrackets commented 11 years ago

It'll come back if you switch to the interpreter. I wish I could find a workaround...

-[Unknown]