While the code is namespaced out properly, the files weren't separated
into their own directory. This moves the files so that introducing a general
interface is easier in the future for supporting other architectures.
There are two special cases that the DSP accelerator handles in a
special way: when the end address is of the form xxxxxxx0 or
xxxxxxx1.
For these two cases, the normal overflow handling doesn't apply.
Instead, the overflow check is different, the ACCOV exception never
fires at all, the predscale register is not updated, reads are not
suspended, and if the end address is 16-byte aligned, the DSP loops
back to start_address + 1 instead of the regular start_address.
When an ACCOV is triggered, the accelerator stops reading back anything
and updating the current address until the YN2 register is set.
This is kept track of internally by the DSP; this state is not exposed
via any register.
However, we need to emulate this behaviour correctly because some
ucodes rely on it (notably AX GC); failure to emulate it will result
in reading past the end and start address for non-looped voices.
When the current address is xxxxxxxf, after doing the standard ADPCM
decoding and incrementing the current address as usual to get the
next address, the DSP will update the predscale register by reading
2 bytes from memory, and add two to get the next address.
This means xxxxxx10 cannot be a current address, as the DSP goes
from 0f to 12 directly.
A more serious issue with the old code is that if the start address
is 16-byte aligned, some samples will always be skipped, even when
that should not be the case.
An easy way to test whether this behaviour is correct is to check
the current address register and the predscale after each read.
Old code:
...
ACCA=00000002, predscale=<value>
ACCA=00000003, predscale=<value>
...
ACCA=0000000f, predscale=<value>
ACCA=00000010, predscale=<another value>
ACCA=00000013, predscale=<another value>
ACCA=00000014, predscale=<another value>
...
New code (and console):
...
ACCA=00000002, predscale=<value>
ACCA=00000003, predscale=<value>
...
ACCA=0000000f, predscale=<value>
ACCA=00000012, predscale=<another value>
ACCA=00000013, predscale=<another value>
...
Slightly cleaner, allows DSP accelerator behaviour to be
added to both HLE and LLE pretty easily, and makes the accelerator
easier to unit test.
I chose to include all accelerator state as private members, and
to expose state that is accessible via registers with getters/setters.
It's more verbose, yes, but it makes it very clear what is part of
the accelerator state and what isn't (e.g. coefs).
This works quite well for registers, since the accelerator can do
whatever it wants internally. For example, the start/end/current
addresses are masked -- having a getter/setter makes it easier to
enforce the mask.
The logic is entirely the same; only the inputs and outputs are
different, so deduplicating makes sense.
This will make fixing accelerator issues easier.
Based on hardware tests, masking occurs for the accelerator registers.
This fixes Red Steel and Far Cry Vengeance, which rely on this behavior
when reading back the current playback position from the DSP.
The class NonCopyable is, like the name says, supposed to disallow
copying. But should it allow moving?
For a long time, NonCopyable used to not allow moving. (It declared
a deleted copy constructor and assigment operator without declaring
a move constructor and assignment operator, making the compiler
implicitly delete the move constructor and assignment operator.)
That's fine if the classes that inherit from NonCopyable don't need
to be movable or if writing the move constructor and assignment
operator by hand is fine, but that's not the case for all classes,
as I discovered when I was working on the DirectoryBlob PR.
Because of that, I decided to make NonCopyable movable in c7602cc,
allowing me to use NonCopyable in DirectoryBlob.h. That was however
an unfortunate decision, because some of the classes that inherit
from NonCopyable have incorrect behavior when moved by default-
generated move constructors and assignment operators, and do not
explicitly delete the move constructors and assignment operators,
relying on NonCopyable being non-movable.
So what can we do about this? There are four solutions that I can
think of:
1. Make NonCopyable non-movable and tell DirectoryBlob to suck it.
2. Keep allowing moving NonCopyable, and expect that classes that
don't support moving will delete the move constructor and
assignment operator manually. Not only is this inconsistent
(having classes disallow copying one way and disallow moving
another way), but deleting the move constructor and assignment
operator manually is too easy to forget compared to how tricky
the resulting problems are.
3. Have one "MovableNonCopyable" and one "NonMovableNonCopyable".
It works, but it feels rather silly...
4. Don't have a NonCopyable class at all. Considering that deleting
the copy constructor and assignment operator only takes two lines
of code, I don't see much of a reason to keep NonCopyable. I
suppose that there was more of a point in having NonCopyable back
in the pre-C++11 days, when it wasn't possible to use "= delete".
I decided to go with the fourth one (like the commit title says).
The implementation of the commit is fairly straight-forward, though
I would like to point out that I skipped adding "= delete" lines
for classes whose only reason for being uncopyable is that they
contain uncopyable classes like File::IOFile and std::unique_ptr,
because the compiler makes such classes uncopyable automatically.
This could cause the first branch of the bootucode procedure, which
takes its parameters from the AX registers, to run during the ROM init
sequence. Since the ROM doesn't set any of the AX registers, the values
aren't meaningful, and can cause bad DMA transfers and crashes.
The GameCube IPL sounds the same when using the free ROM as it does when
using the official ROM (and in Audacity, I couldn't visually distinguish
between the waveforms). It has a reference to an unimplemented function
at 0x8644 which seems to only be used in an inlined version of the CARD
ucode.
- coef: Explicitly set 23 different values that are used by GBA UCode,
and tweaked overall parameters to more closely match those 23 values.
- irom: Moved a few functions to their proper places, updated BootUCode
to configure DMA transfers using AX registers as well as IX registers
(the GBA UCode uses this to do two sequential transfers in one call),
and added partial functions used by GBA UCode.
All functions were reverse-engineered solely based off of observed
effects on the virtual machine: register states before-and-after, dmem
interactions, and DMA transfers. The specific coefficients were observed
being read from dmem, and must be exactly those values to function
properly. I have no knowledge of how the official ROM implements these
functions, or how it is implemented overall.
Tested with The Legend of Zelda: Four Swords Adventures, Final Fantasy
Crystal Chronicles, and Billy Hatcher and the Giant Egg (to download
ChuChu Rocket!).
`P_REG1C` had the same value as `P_ACCL`, so was causing spurious errors
when used with ACCM registers. Gcdsptool (which calls this `P_ACCLM`)
gives it the value `P_REG | 0x1c10` instead, and handles errors in the
same block as other REG## enums.
This adds the WARNPC directive from xkas/asar to complement the existing ORG
directive. A common useful idiom is "WARNPC 0xXXXX\nORG 0xXXXX," which only
seeks forward and raises an error if you've already written to that part
of the file.
- Makes DSP-LLE code checksums the same as those from DSP-HLE. I'm
assuming DSP-HLE was doing it correctly, since there are numerous
references to these pre-endian-swapped checksums (including in
DSPHost.cpp itself).
- Fixes disassembly when dumping code from DSP-LLE, which was using the
wrong endianness and giving totally bogus output.
- Reveals error messages of the format, "Bah! ReadAnnotatedAssembly
couldn't find the file ../../docs/DSP/DSP_UC_AX_07F88145.txt," which
seems to be intended behavior that was previously hidden.
Given none of these are used outside of the DSPEmitter class (nor does
it really make sense to allow them to be used outside of the class),
these should all be made private.
