Add game quirks for unknown BP/CP/XF commands #9576
Conversation
Pokechu22
force-pushed
the
invalid-gfx-reg-quirk
branch
from
62b4961
to
1251e62
Compare
Pokechu22
force-pushed
the
invalid-gfx-reg-quirk
branch
from
1251e62
to
23f816d
Compare
|
I've created a separate quirk for 0x00/0x10/0x20, which are unknown commands that appear to be related to performance queries (they're used by e.g. Mario Golf (bug 12461). Note that the SDK (and libogc, but perhaps unsurprisingly not anything by datel) issues 0x20 with argument 0 on startup, which I've ignored here as otherwise it would completely drown everything out. Libogc also has other examples with both 0x20 and 0x00, but nothing with 0x10 ( I've also added logging for inexact matches to |
| @@ -192,6 +194,7 @@ static void XFRegWritten(int transferSize, u32 baseAddress, DataReader src) | |||
| case 0x1017: | |||
|
|
|||
| default: | |||
| DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_UNKNOWN_XF_COMMAND); | |||
There was a problem hiding this comment.
We want to report 0 here? Any idea why it ignored for the warn?
There was a problem hiding this comment.
I've added c702180 temporarily to get a list of these from FifoCI.
There was a problem hiding this comment.
Looks like the source of the problem is that the FIFO player copies all of the registers on startup, which includes the unused ones; this produces log messages for everything (but shouldn't affect normal playback). 1099475 is a hacky workaround where the unused ones just aren't copied, but I don't think it's a great implementation.
0x1007 in particular is odd, because YAGCD says it's Perf1 (0x1006 is Perf0 which Dolphin calls XFMEM_SETGPMETRIC); games don't seem to use it though. It's marked as known in the actual XFMemory struct, but there is no constant for it. And weirder still is this code (which seems very unlikely to be correct):
dolphin/Source/Core/VideoCommon/XFStructs.cpp
Lines 41 to 48 in 8d2b0ff
There was a problem hiding this comment.
Having looked at other parts of the code, it turns out nextAddress advances address forward, but also updates the count, so it's more of an optimisation that happens to be rather unnecessary in this case, but makes more sense for e.g. XFMEM_SETPROJECTION where usually all 7 registers are updated at once, but it would be bad to update g_vertex_manager et al 7 times for that.
My earlier hackfix was backwards (I was skipping everything but the unknown registers...); 7ea201c is a correct implementation. Note that that produces rendering differences but it seems that those are only from timing changes (if instead of skipping unknown registers, the last valid register is repeated (2bd038e), there are no differences); since this also only happened on the first frame in the fifo player, I'm not going to worry about it.
With skipping unknown registers, it should be fine to log a warning even when it's set to 0; none of the testcases on fifo.ci have any unknown registers in the fifo itself.
|
I had no idea that Dolphin did tracking on this level, very cool. |
Pokechu22
force-pushed
the
invalid-gfx-reg-quirk
branch
3 times, most recently
from
06f14e8
to
1099475
Compare
Pokechu22
force-pushed
the
invalid-gfx-reg-quirk
branch
3 times, most recently
from
2bd038e
to
2755a4e
Compare
|
Looks reasonable to me. Not a coder |
…wn commands They appear to relate to perf queries, and combining them with truely unknown commands would probably hide useful information. Furthermore, 0x20 is issued by every title, so without this every title would be recorded as using an unknown command, which is very unhelpful.
This avoids some warnings, which were originally fixed by ignoring loads with a value of zero (see 636bedb / dolphin-emu#3242). Note that FifoCI will report some changes, but only on the first frame; these seem to be timing related as they don't happen if a different write is used to replace skipped ones.
Pokechu22
force-pushed
the
invalid-gfx-reg-quirk
branch
from
2755a4e
to
f32b771
Compare
|
FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:
automated-fifoci-reporter |
See e.g. bug 10931. #9497 removed assertions for unknown CP commands, and there were only warnings for BP and XF before. Since there are a few commands marked as maybe unknown, this is something worth tracking.
I have separate quirks for completely unknown CP commands (
USES_UNKNOWN_CP_COMMAND) and CP commands where the lower nybble is not in the expected range (USES_MAYBE_INVALID_CP_COMMAND); this is because YAGCD disagrees with Dolphin's current implementation and it would be good to find out which is correct. Based on bug 10931, Toy Story 3 will trigger both of those cases, but other games might only hit one.