== DETAILS
TIL that it's bad to call synchronization code from callbacks.
To avoid that, I made the following changes:
- Implemented an atomic swap (see previous commit) to avoid explicit
locking when working with the event list
- ensure locks are only acquired in either the main thread or the
I/O polling thread
- use an explicit polling loop; we still use async reads, but the
read doesn't immediately re-invoke itself.
- remove the sleep in the polling thread.
- remove unnecessary locking in the thread cleanup call--verified that
the list can't be modified while it is being executed.
== TESTING
I tested locally, and was able to disconnect/reconnect USB devices several times without the worker thread getting deadlocked.
Fortunately, the gcc port implements the builtins and, from basic
testing, they seem to work.
This is only really useful on Wii U--other platforms have more
robust atomic operations, or aren't using gcc to build.
== DETAILS
- fix the bitshift math
- read the right bytes out of the ds3 data packet
- remove verbose logging in critical path
- stop caring about errors in the hid read loop -- seems to just
be benign "device not ready" -- or at least, that's what I'm assuming
given that the read eventually succeeds.
== TESTING
Played Mario 3 with the DS3 with no issues.
== DETAILS
- update to not try starting the read loop until after the device
is successfully initialized
- add new HID wrapper macros needed by ds3 driver
- add some debug logging to help with troubleshooting
- add button map for DS3
== TESTING
Tested with local build. DS3 init is not working.
== DETAILS
Whereas the last commit had a hack (that disabled the wiimote
driver in the process), this has.. well, a *different* hack that
allows pads to register in any order.
Note that due to the initialization routines, the gamepad will still
likely always get slot 0. Not sure if this can be overridden via config
or not.
== TESTING
Tested locally with GC adapter
== DETAILS
Now that I have a working implementation, it's time to tidy up a bit:
- there was no need for the HID subsystem's object data to have a reference
to the global hid state (since it's global), so removed it.
- refactored the users of that member to use the global state, defining
reusable macros.
- reorganized the information in *.h files
- removing the hid state also made the constructor changes to the hid driver
unneeded, so I reverted those changes.
== TESTING
Confirmed clean build. Haven't tested the build yet to make sure everything
still works, though.
== DETAILS
- Added a new method to the joypad_connection_t interface for
getting a single button
- wired everything into the hidpad driver
- for testing purposes, hacking the top-level joypad driver
so that kpad isn't used
- add a new RARCH_LOG_BUFFER method to verbosity for logging the
contents of a binary buffer (useful for writing/debugging pad drivers)
- fix a few bugs in the wiiu GC pad driver
The button mapping isn't quite right, and I'm not sure what's
going wrong.
== DETAILS
Trying to do weird pad math just wasn't working so I bit the bullet and just
let it allocate all 16 pads in the slot list, then just mark 0-4 as
connected so that the slot allocator would start at 5.
I can see it detect the pad, but no idea if it works. Out of time for
today.
== DETAILS
Turns out freeing memory that's already been freed is.. bad.
Fix two double-free instances; one due to over-freeing and the other
due to wrong order-of-operations causing a double free.
Also updated logging a little.
== TESTING
The GC adapter still clobbers slot 0, but the "emergency exit" sequence
works to quit RA cleanly.
== DETAILS
(I think)
- Uncomment the call in the read loop to start feeding packets to the
driver
- implement the GCA packet driver
- implement the pad interface
- fix indentations in GCA driver
== TESTING
Compiles. Haven't tested yet.
== DETAILS
Turns out the cause of the crash was a bad cast, resulting in a
function call to nowhere.
Also, I think the DSI exception handler only works on the primary core;
when this was happening in the background thread, I got a black
screen error instead.
Next up: finishing up the GCA driver.
== DETAILS
We're at a point where we need to do more than just
clean up a local data structure, so I've started
implementing the "detach" part of the code so that
everything gets cleaned up properly.
Also, added error handling inside the polling
thread.
== TESTING
Have not tested yet.
== DETAILS
I've created the concept of a hid_driver_instance_t which is basically
a central place to store the hid pad driver, hid subsystem driver,
the pad list, and the instance data for the above in a central location.
The HID pad device drivers can use it to perform HID operations in a
generic manner.
This is more-or-less a pause point so I can catch up with upstream.
== TESTING
Haven't tested this yet. Compiles without warnings though!
== DETAILS
- detect() methods in device_* files now check for VID/PID
instead of just returning false
- add "name" field on hid device, mainly for logging purposes
== TESTING
Verified my WiiU GC adapter detected properly
Add some timing functions so that we can know the time (woo-hoo?)
I use errno here, which worries me a bit. I wouldn't put it past
devkitPPC to have it as a null pointer or something.
Can confirm this makes XMB's clock work, much to @cucholix's delight;
I'm sure ;D
== DETAILS
When I split out wiiu_gamepad into multiple sub-drivers, I missed a spot.
The code expects the device type to be initialized to a magic number
(0xfd), and instead it was getting initalized to zero, which meant it
never triggered the detection process.
I re-added the initialization, and now it works.
== TESTING
Tested with nestopia core to confirm autodetect and game response works.
== DETAILS
1. Noticed that the HID driver wasn't loading after setting WIIU_HID=1. Found
that the HID driver init was ifdef'd out. Removed that.
2. Current theory around "System Memory Error 160-2203" is in-memory
corruption. So, to try to identify it:
- Created a routine that does a hex dump of the RPX over the logger. (I
have a python3 script that can extract the hex dump back into a binary
file). If a SME occurs with this routine enabled, we can see if the
corruption is happening at read-time, or somewhere between when we send
the RPX to the loader and try to execute it.
- I noticed that we allocate slightly more memory than the RPX actually
needs, and we don't zero the memory, which means there's a handful of
bytes at the end that could be anything. I added a call to memset() to
zero out the memory prior to loading the RPX off the SD card.
And, of course, after adding those, I haven't been able to reproduce the
System Memory Error, so maybe the uninitialized memory was the problem?
Here's hoping.
== DETAILS
- Add *.swp to gitignore so editor swap files don't get committed
- Remove unneeded commented-out defines from WiiU build
- Start on fix for DSI when switching cores on WiiU
== TESTING
Sigh. I'm back at "System Memory error", which makes me think the problem
might be the SD card. (On the plus side, I manually verified the hash so
at least the copy process is working).
So, that's to say that I can't actually test to see if the DSI error is
fixed.
== DETAILS
We're trying to track down the source of crashes when switching cores.
To rule out the HID code, this commit does the following:
- Wraps the library imports in an ifdef
- Wraps the object files in conditionals in Makefile.wiiu
- In wiiu_joypad, calls into the hidpad driver are wrapped in ifdef
== TESTING
This didn't solve the "System memory error" crash I've been experiencing.
But, maybe it will impact the other flavors of crashes others are seeing.
== DETAILS
TIL that the "max_packet_size_*" fields in the HIDDevice struct
are actually requirements, not maximums.
The reason HIDRead() was blocking was because the HIDWrite() that
sent the activation command was silently failing.
The reason HIDWrite() was silently failing was because I was using too
small of a buffer for the device.
In this case: the Wii U GC adapter, which has a "max" tx packet size of 5.
"max" is misleading. You actually have to write all 5 bytes.
Which means: copying the 1-byte buffer to the 5-byte buffer, and writing
the 5-byte buffer via HIDWrite().
So, in summary:
- Use correct semantics for HIDWrite() so that HIDRead() can work.
- Update the OSThread struct to reflect the current knowledge over at
WUT. It's unlikely this was actually causing a problem, but never
hurts to be more correct.
== TESTING
I temporarily enabled the call to log_buffer() and confirmed that the
GC adapter's state is successfully being read.
== DETAILS
The BIT256_GET() macro expects a bit number (from 0-255), and we're giving it
a 32-bit mask (0x000080000).
Solution:
- Define VPAD_BUTTON_xxx_BIT macros using the bit number
- Use said macro in wiiu_input.c
- organizational cleanup:
* put VPAD_BUTTON_TOUCH into the enum in stead of as a hokey define
* put the touch bits in the right order
* put in placeholder enums for (currently) unused bits
can load *.slangp and *.slang files. since there is no runtime compiler,
precompiled shaders (*.gsh) need to be present next to each *.slang
source file.
== DETAILS
This should fix#6025.
After confirming that dummying out the init block of the HID subsystem
driver eliminated the crash, I narrowed it down to the event loop.
And that's when I noticed that, when the thread consumes the event,
it doesn't free it.
Oops.
Updated the event loop to free the event after it has been processed.
== TESTING
Local build, was able to load multiple ROMs in succession where prior
I was getting the system memory errors.
I've #if 0'd out a call to HIDRead that is still getting deadlocked,
because it slows down the startup/shutdown process.
== DETAILS
The joypad driver was only copying the first 16 bits when executing
get_buttons(). The touchpad button is bit 19, so as a result
RETRO_DEVICE_ID_POINTER_PRESSED would never fire.
We fix this for now by copying 32 bits.
== TESTING
Will need someone to verify. I don't have a core handy that leverages
the pointer device state functionality.
== DETAILS
@r-type got me the link to the fixed-fixed version.
Which was actually closer to where I was originally!
So:
- Revert the previous commit
- Apply the actual changes needed to fix the problem
- Bring in and fix the logging code from the previous implementation
== TESTING
- Verified that top-left is -0x7fff,-0x7fff and that bottom-right is
0x7fff, 0x7fff.
== REVIEW
@QuarkTheAwesome @r-type @twinaphex
== DETAILS
Apparently when I refactored this code, I missed out on the fix for
touchpad coordinates that @QuarkTheAwesome had committed.
Oops.
- Fix the touchpad coordinates
- Remove unneeded references to video driver in wpad_driver.c
- Remove unneeded video driver include in pad_driver.h
- Add logging in wpad_driver to verify the fix (#ifdef'd out by default)
== TESTING
Verified fix manually.
== DETAILS
If a call to HIDRead() ends up blocking indefinitely, it will
cause the shutdown process to wait forever.
To avoid a deadlock, I've put in a retry counter so that it will
give up after 5s and print a warning to the log.
== DETAILS
RetroArch's general HID drivers are intended as a full-on substitute for
other input drivers such as XInput, DInput, SDL, etc. The Wii U port is,
to my knowledge, the first case of heterogenous input drivers working
concurrently.
As such, I've moved things around:
- The HID driver source is moved into the wiiu/input/ directory alongside
the joypad subdrivers.
- We no longer use the input_hid_init_first() method to instantiate; instead
we just init the wiiu HID driver directly.
- The HID pad driver and HID subsystem driver enjoy a tighter coupling,
mainly having to do with the initialization of the joypad connections
list, because there's no way to inform the HID driver's init() method
how many slots to allocate.
== TESTING
Will test in a moment, but at least it compiles cleanly. ;)
== DETAILS
The gamepad didn't work because I had tried to rename the pad from
'WIIU Gamepad' to 'WiiU Gamepad'.
I added some debug logging and (to cut out a lot of trial-and-error)
discovered that the reason it didn't work was because a bug in a macro
was using the define literally instead of substituting it (so e.g.
the autodetect handler was trying to match 'WiiU Gamepad' against the
literal string 'PAD_NAME_WIIU_GAMEPAD').
- Fixed the macro bug
- Left a minimal amount of the debug logging in place; may come in
handy for someone else.
- Updated wpad/kpad/hidpad to use the define constants
== TESTING
Did a test build and confirmed the gamepad responded.
== DETAILS
- the free() method of the hid_driver_t interface needs its
parameter defined as const in order for the compiler to stop
complaining about losing const-ness.
- if a joypad list is created with <MAX_USERS slots in it, the
destroy() function will crash because it assumes there are MAX_USERS
entries.
To do this, the allocate function creates n+1 slots, and gives the
last slot a canary value that the destroy() method can then watch for
when iterating through the list.
== DETAILS
Well, after a lot of code analysis, this seems like the
best way to handle things on the Wii U without also completely
re-architecting the I/O handling in RetroArch.
How it works:
- the top-level wiiu_joypad driver is now nothing more than a
delegator.
- the wiiu-specific drivers live in `wiiu/input/`
- wpad_driver.c handles the WiiU gamepad
- kpad_driver.c handles the wiimotes
- hidpad_driver.c will handle HID devices like the GC adapter, DS3/DS4, etc.
(I say "will" because this isn't implemented yet)
== TESTING
Haven't actually tried the build to see if it works, but it does
compile.
== DETAILS
Starting to implement the I/O handling on the HID driver.
The old implementation basically had a never-ending HIDRead() callback
set up, so that callback N would start the read process for invocation
N+1.
We will take the same approach here; but now that the I/O thread is
happenning on its own core, we should be able to let it run full-
throttle without impacting emulator performance.
Of course, this hinges on the callback actually running on the same
core as the syscall was initiated on.
== TESTING
Can confirm that the read_loop_callback gets invoked on the same core
that the HIDRead() was invoked on.
== DETAILS
This implements the WiiU-specific functions.
Since the wiiu_hid_t data structure contains the handle and interface
index, the method signatures can be simplified quite a bit. And since
(at least for now) we want these to be synchronous, we don't need to
expose the callback parameters.
== DETAILS
The handshake stuff is derived from the old HID2VPAD, just in knowing
what data goes in what report.
- Added the HID_REPORT_ flags to syshid.h
- Renamed the generic "REPORT_TYPE" flags to be meaningful
- also fixed incorrect parameter list for set_protocol
== TESTING
The functions aren't implemented in wiiu_hid.c just yet,
so this is gonna crash if you try to run it.
== DETAILS
The old code was crashing; I did a minimalized branch and the crash
went away, so I'm bringing that over here. Meaning I'll have to
redo some of the other work I'd put in, but oh well.
(now watch it start crashing again)
== TESTING
Can confirm it builds. Wii U is busy ATM so I can't test.
== DETAILS
We discovered that the controller_patcher code was causing
the WiiU to intermittently crash when switching ROMs.
Changes:
- Completely extricates the controller_patcher code
- Create a skeleton wiiu_hid driver
- Wire up the build system to build/link it successfully
== TESTING
Has not been tested. Probably doesn't crash, since the
skeleton driver is just a copy of the null driver.
Thanks for @gblues for finding the strcmp bug.
I've removed the opcode display since it's a bit unsafe - if the
exception handler causes a DSI; Cafe OS will take over and freeze up the
console. This is obviously not a good thing. There are possible
situations where memory is executable and not readable; so this is just
about covering all bases until a better solution can be implemented.
Maybe we can experiment a bit more once this handler isn't being used so
often ;)
Coreinit seems to get special handles, so adding a special case should
make its relative addresses actually become helpful again. Let's just
hope __PPCExit stays at 0x180!
Also added the violating opcode to the DSI message; may have to move it
in case it pushes stuff off the end of the screen.
Untested at time of commit. Hopefully this should make debugging #5357 a
bit easier...
This hopefully fixes the issues when you try to use 2 Controllers with
the same vid/pid at the same time.
Tested with 2 DS4 controller via the Hid to VPAD Network Client.
Adapters with multiple ports (like the official GC-Adapter) are still
working
Allow using the Gamepad's touch screen as a RETRO_DEVICE_POINTER.
Methodology could use some work, had to add an extra axis to
joypad in order to get the data transferred into the input driver.
Feel free to change this.
Needs to emulate RETRO_DEVICE_LIGHTGUN to really be useful.
Potential for Wiimote IR in future.
Partially addresses libretro/RetroArch#5294
