Add DRM platform (through DRM+EGL+GBM backend) to dolphin-nogui #9015
Conversation
|
Please make sure you press "view all lines" on the lint log. It consists of about 2000 lines for this PR, not just the 40 ones that are shown initially. I would recommend using clang-format to fix the lint issues automatically. |
There was a problem hiding this comment.
Much of this code seems to be brought over in its original form without migrating it to C++. I'd suggest migrating it over so that it conforms to the general conventions that we use in the rest of the non-external files in the source tree.
It doesn't seem like a clang-format pass was even run over the code.
Source/Core/Common/CMakeLists.txt
Outdated
| GL/GLInterface/EGLDRM.cpp | ||
| GL/GLInterface/EGLDRM.h | ||
| ) | ||
| else() | ||
| target_sources(common PRIVATE |
There was a problem hiding this comment.
This command should be indented by two spaces like the command in the related if statement.
|
|
||
| #include "Common/GL/GLContext.h" | ||
|
|
||
| typedef struct |
There was a problem hiding this comment.
In C++ this can formatted more concisely like so (with naming altered to follow our naming conventions):
struct EGLContextData
{
EGLContext ctx;
EGLSurface surf;
EGLDisplay dpy;
EGLConfig config;
int interval;
unsigned major;
unsigned minor;
};| // Copyright 2020 Dolphin Emulator Project | ||
| // Licensed under GPLv2+ | ||
| // Refer to the license.txt file included. | ||
|
|
There was a problem hiding this comment.
Top level comment for the file just to state that a lot of this code currently doesn't match our naming conventions and coding styles and should conform to that.
A lot of it is very C89 (variables at top of scopes, rather than where they're used, etc) and should be migrated to idiomatic C++ before being merged.
|
|
||
| typedef bool (*egl_accept_config_cb_t)(void *display_data, EGLDisplay dpy, EGLConfig config); | ||
|
|
||
| typedef struct gfx_ctx_drm_data |
There was a problem hiding this comment.
struct GFXContextDRMData
{
egl_ctx_data_t egl;
int fd;
int interval;
unsigned fb_width;
unsigned fb_height;
bool core_hw_context_enable;
bool waiting_for_flip;
gbm_bo *bo;
gbm_bo *next_bo;
gbm_surface *gbm_surface;
gbm_device *gbm_dev;
};| static uint32_t g_connector_id = 0; | ||
| static int g_drm_fd = 0; | ||
| static uint32_t g_crtc_id = 0; | ||
| static drmModeCrtc *g_orig_crtc = NULL; |
There was a problem hiding this comment.
nullptr instead of NULL. Ditto for all other occurrences.
|
|
||
| WindowSystemInfo GetWindowSystemInfo() const override; | ||
|
|
||
| private: |
There was a problem hiding this comment.
This isn't necessary, given it's not used.
Co-authored-by: LC <[email protected]>
Co-authored-by: LC <[email protected]>
Co-authored-by: LC <[email protected]>
|
Thanks for the productive comments and suggestions, I will tackle most of these as soon as possible. |
| * | ||
| * RetroArch is free software: you can redistribute it and/or modify it under the terms | ||
| * of the GNU General Public License as published by the Free Software Found- | ||
| * ation, either version 3 of the License, or (at your option) any later version. |
There was a problem hiding this comment.
It seems you copied a GPLv3+ file into a GPLv2+ project, you should first get the approval from all of the owners of the original file to relicense under a different license if you were to do so.
There was a problem hiding this comment.
Indeed this all started as an experiment but I will rebase the problematic code on another DRM implementation from some batocera folks to avoid licensing issues. I have started a big refactor to meet both the licensing and Dolphin coding standards.
|
Can someone relaunch a buildbot try ? I tried to fix failing checks. |
|
sorry, I missed some #ifdef and lint-checks |
|
why are you adding DRM to Dolphin? |
|
To be able to run it on ARM boards (ODROID N2 or Khadas VIM3 for instance)
using Linux distros like batocera
Le sam. 8 août 2020 à 01:08, Rin <[email protected]> a écrit :
… why are you adding DRM to Dolphin?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#9015 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAW4WBN63RJYTWNDREPEA3DR7SCNTANCNFSM4PXREN4A>
.
|
|
to avoid any confusion: in this context DRM stands for Direct Rendering Manager, not Digital Rights Management |
thats a load off lol |
|
Latest commit refactors the whole code to C++, could you re-run a buildbot and reconsider ? |
|
This means that the retroarch fork will be smaller? |
|
retroarch fork is straight-up not currently being maintained at the moment as far as i am aware |
|
Yeah, i hoped this would fix that, much like i can compile ppsspp as a core (even with all of its problems), but lesser (since not completely upstream). I guess this is the wrong place to ask for this, i'll just open a bug report on the fork when this is merged. |
|
I don't think a RPi 4 4Gb would be enough to run dolphin android ? My RPi is running raspbian 32 bit btw (as 64 bit is still in beta, and could use more ram at idle afaik) |
This would be potentially useful for more powerful *unix computers too to avoid requiring X11/wayland. |
|
Correct, i play dolphin in KMS in retroarch because it's a bit faster. Having shed a lot of useless processes. There are problems with this approach, though - they occur because of hardware and driver problems and are not especially relevant for dolphin but for example, for scummvm. My portable has a 'quirky' touchpad hardware. Lots of hardware 'quirks' are not specified in the kernel drivers, but in higher level libraries like libinput, that are loaded by desktop environments. Retroarch cores 'directly' access udev (or something) but that's kind of bullshit because libinput is already placing itself between the kernel and those outputs... unless you are in a TTY. In short, i can't play scummvm in KMS because the synaptics touchpad doesn't have a right click (the difference between 'left and right' is probably a question of coordinates in the quirk) and because the cursor projects absolutely into the screen, so if you lift your finger, touch elsewhere in the touchpad, and move it , the cursor jumps. Problems like this should be uncommon for actual controllers though. |
|
Another approach would be to revive a SDL2 build, command-line based, as SDL2 now features a really solid kmsdrm backend (and it's even going to be better in upcoming SDL 2.0.13 release). |
I can vouch this already works well in some cases, but I suspect a lot of distros do not ship SDL2 with their kmsdrm backend enabled. Maybe it will be enabled by default in future SDL2 releases? |
|
Hey, I tried this (with a slightly modified retroarch dolphin launcher core to use a standalone path and receive the -p drm argument. This mode has a major problem, both there and even with x11 and no-gui: the hotkeys simply don't work, so there is no way to exit the game (which is a bit of a problem if you happen to be on KMS/tty. Not sure if it's a bug or a lack of window (and thus focus) preventing them from being delivered, but it makes it almost useless for the 'no window manager case'. but neither helped. I also tried to map both stop and exit, rationalizing that it doesn't matter with only 1 window, but it didn't help either. The game controllers keys mappings do work, so it's just the hotkeys. I'm on linux, and i tried this on both wayland (with the x11 support, aka xwayland) and a tty to run DRM. Both had the same problem (but i only had to hard reboot in the tty). edit: there is a issue about this https://bugs.dolphin-emu.org/issues/11689 edit2: there is also the setting 'HotkeysRequireFocus = False'. Which doesn't work. Neither does HotkeysRequireFocus = False or RenderToMain = True or BackgroundInput = True or ConfirmStop = False or all of them at once. |
This comment has been minimized.
This comment has been minimized.
|
I have a patch that can get the keys running, but all the modifier keys don't activate and i have no idea why. Also it crashes on exit because i'm not good at programming c++ (and DRM requires a closed Window Manager to get full gpu rights i guess). As a aside, if your keys are not working at all; check in dolphin qt if your hotkeys shortcuts haven't got some invalid keys on them or a invalid device ('virtual pointer' is not good for the keyboard hotkeys). Ideally, HotkeyScheduler could be abstracted away from QT and reused as a central point to have shortcuts with a thread for all platforms. Would need to have all the hotkeys be redefined without the emit, maybe by making it subclassable, then forbid a bunch of hotkeys and make others different. Without pulling in QT into the compilation unit preferably. Testing this is dangerous, so i've taken to doing buggy patch: edit: not sure if it influences anything but even on a tty console, caps lock isn't showing the led on this usb keyboard so i don't even know where this key modifier problem is. Time to maybe workaround it using the 1-9 keys i guess. |
|
Btw you should probably also disable the signal to stop (ctrl+s) if you don't expect lusers to complain about unexpected freezing if they don't know how to unfreeze (ctrl+q). The above (that happened to me, i'm a luser) does not mean i found a way to make ctrl work, these are handled by the kernel keyboard driver i think. Similarly, caps lock 'sticks' in the cmd console but doesn't in the application after i mapped the turbo to it; the leds don't light up (though this is no different in the cmd console itself), etc. The only modifier combinations that seem to work are limited to things the kernel recognizes... Quitting throws a iterator exception that probably signals something wrong. |
|
Can you share / submit your patch ?
Le ven. 18 sept. 2020 à 14:32, i30817 <[email protected]> a écrit :
… Btw you should probably also disable the signal to stop (ctrl+s) if you
don't expect lusers to complain about unexpected freezing if they don't
know how to unfreeze (ctrl+q).
The above (that happened to me, i'm a luser does not mean i found a way to
make ctrl work, these are handled by the kernel keyboard driver i think.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#9015 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAW4WBKSQHR67OHFDYSRN7DSGNHPBANCNFSM4PXREN4A>
.
|
|
As a pr? it's up there in the previous post to the last as a patch. And ctrl/shift not working is a pain in the ass considering the default dolphin shortcuts relating to savestates. I'm also unsure if i need everything in the Init or the shutdown function. That was me hacking away a copy of something i forgot, probably the QT MainWindow. It's also not the best way to do this probably, reusing hotkeyscheduler would seem saner, after a refactoring abstracting it and moving it to common or something like that. Lots of common structure and where it's not common it could be stubbed, and many 'probably valid' shortcuts missing still (camera manipulation, debugging etc). And let's face it, people aren't going to line up to fix the cmd line platforms after adding functionality, see shortcuts X11 not even working or missing the savestates F9 and F10 and many other hotkeys. If the build breaks with a changed interface until those are implemented, even as a no-op, things would be smoother there. I'm also unsure if anything special should be done to prevent multiple presses of load or save state, or if the State class handles all of that noise. I was able to load and save (if without a modifier hotkey) a Gc game at least. I don't remember if you need the Sys directory for dolphin to configure/get some defaults. I opened the normal QT dolphin to reconfigure hotkeys. I used this commandline: Not sure if needed to turn off X11 before i started to get input (the fact input is not not working in X11 might be important). |
|
I just tried some games and emulated wiimotes aren't working here, though the classic attachment is if the game supports it, so that's fun. Possibly because the wiimote tries to get the 'virtual pointer' (eg: the mouse) and evdev goes 'what mouse?'. But i'm not going to fearmonger too much until someone more competent than me confirms it. I don't have a real wiimote to test either. |
|
Ah I think the above happens because only 'xinput' allowed the merging of input sources (in this case the keyboard and a touchpad / mouse pointer). edev doesn't and when you try to configure one of the other in the dolphin compilation, you get one of the other working and when you drop down to the actual DRM layer it's no different. A shame. Not sure if this is a combination that dolphin does purposefully on its input classes that wasn't done for evdev or what. The 'ctrl/shift/alt doesn't register and caps lock doesn't stick' are still mysteries though. I'm going to test the above with a game with a icon to see if i can get a 'wiimote' without keyboard to press keys and vice versa by changing the configuration backend in dolphin from evdev/touchpad to evdev/usb keyboard. edit: turns out i can't, the only 'merged' one with the keyboard and mouse is xinput2/0/virtual core pointer. There are also further problems if i try to use synaptics touchpad as a pointer (namely synaptics driver directly giving buckfuck nut values to axis without a '0 center' and moving by itself to '0' when the finger is lifted, maybe solvable by using 'relative input' but not worth it without the keyboard at the same time.). It was good try. Retroarch appears to use udev to support both keyboard and mouse and touchpads so maybe that'd work if dolphin had udev backend. Libudev is required for some reason for this KMS thing, maybe i should just build the QT version again to see if the configure appears for udev? Mmm.
|
@i30817 This is running in DRM mode -- I also built the branch with no special flags and same behavior resulted, Wii Remote and nunchuk support both work fine in drm. |
|
I am encountering an odd problem of my own. My laptop's screen is failing, so I have grub set to disable the internal screen and only output over HDMI to my 4K TV. Grub is set for 3840x2160@30Hz. Give me a moment to describe how RetroArch behaves in KMS on the same system. My system boots into a systemd unit file that launches RetroArch in TTY3 at boot:
So the problem I'm having is dolphin-emu-nogui is behaving the exact same way as RetroArch, except, the seemingly similar settings for Dolphin.ini are not being recognized. I think the correct settings for Dolphin.ini would be something like: I have tried the same setting without "HDMI1:" but does not seem to make a difference. When I launched Dolphin in GUI mode, those particular settings were saved when running fullscreen (as I was running the display in 1360x768 to see if the settings would autosave) and so that's why I assumed they would work in nogui mode too. I have tried changing the kernel parameter in grub to 1360x768, but dolphin-emu-nogui still switches to 1920x1080 in TTY. I know dolphin-emu-nogui is reading the Dolphin.ini file for some settings, because if I switch audio backend away from ALSA to Pulse, my sound starts working. Similarly, if I change WiimoteContinuousScanning to True, my Wii Remotes start working. If I enable frame counters or framerates, then those show up too in nogui mode. If I am adding the wrong settings to Dolphin.ini please advise so I can test the correct settings, but I am beginning to wonder if those particular Display settings simply are not processed when running in DRM instead of x11. I can attest that if I make those changes to Dolphin.ini, no matter my resolution on my desktop, dolphin-emu-nogui starts in the correct resolution when running in x11. So if I'm in 4K on the desktop, Dolphin would start with 1360x768@60Hz, 640x480@60Hz, or any compatible resolution I have tested. I just need DRM to function the same way if possible. Ps No, I do not really understand why apps launched via a TTY session behave differently than graphical X11 apps or the TTY terminal itself. But my ignorance in this setup is definitely showing. I apologize in advance. The reason I do not just use 1080p is there is definitely a performance hit in that resolution versus a lower resolution. My older laptop is fast enough to play many GameCube/Wii games but not all within Dolphin, turning the resolution down definitely makes the system perform better and makes more of the library usable. |
|
I'm talking about the emulated wiimote, i don't have the actual one. Wii remote is a unique controller so it isn't surprising that edev supports it, the problem is when you tried to configure keyboard keys and used the touchpad as a pointer/tilt axis at the same time. Xinput2 (available in X11 and Xwayland) supports this, evdev (available in KMS) does not. |
|
@ i30817 |
|
Any updates on this? Spent a few days on a similar thing |
|
Did it via Vulkan here: #13222 |
Tested on ODROID N2 (Mali G52 GPU).
This relies on libdrm, libgbm and EGL. Everything is properly detected through CMake modules.
There's room for improvements for sure, especially in EGLDRM.cpp code.
Feedback is welcome.