Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android: Implement custom driver loading for ARM64 Android devices #18532

Merged
merged 3 commits into from
Dec 14, 2023

Conversation

dima-xd
Copy link
Contributor

@dima-xd dima-xd commented Dec 12, 2023

Add possibility to install custom Vulkan driver for ARM64 Android devices using libadrenotools.
Some drivers can be found here.
It is also possible to use this feature, but I'm not sure if it's actually needed.

Draft for now, until I figure out why some drivers are crashing here. Or until I figure out how to make some kind of fallback to use default if it crashes on booting the app.

@dima-xd dima-xd marked this pull request as draft December 12, 2023 16:46
@hrydgard
Copy link
Owner

Crash in debug layer? You might be able to work around it in the Vulkan initialization by disabling the VK_EXT_debug_utils extension when you load these drivers, then we won't try to call those functions.

Core/Config.h Outdated Show resolved Hide resolved
@hrydgard
Copy link
Owner

We can play with the turbo setting and stuff in followup PRs, don't need to get everything in at once.

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 12, 2023

Crash in debug layer? You might be able to work around it in the Vulkan initialization by disabling the VK_EXT_debug_utils extension when you load these drivers, then we won't try to call those functions.

Added the check if customDriver is empty. Works as it should now. Thanks for the advice

@dima-xd dima-xd marked this pull request as ready for review December 12, 2023 17:10
@hrydgard hrydgard added this to the v1.17.0 milestone Dec 12, 2023
@hrydgard
Copy link
Owner

Are you seeing any meaningful improvements with the driver, by the way?

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 12, 2023

Are you seeing any meaningful improvements with the driver, by the way?

Are you seeing any meaningful improvements with the driver, by the way?

I didn't test too much. For some reason Qualcomm drivers like 615.37 work very wierdly (hard to explain, better to see it yourself). But with turnip drivers I get like +5-20% performance, so I don't even need to apply speedhacks. But it' only in games I tested.

@hrydgard
Copy link
Owner

hrydgard commented Dec 12, 2023

That sounds promising, at least. Though, even my old Poco F1 with Adreno 630 runs all games at full speed with the proprietary drivers already... but it's good to have this to experiment with.

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 12, 2023

That sounds promising, at least. Though, even my old Poco F1 with Adreno 630 runs all games at full speed with the proprietary drivers already... but it's good to have this to experiment with.

Hmm... Almost the same for me with my Poco F3. But anyway, is there any benchmark homebrew/game to understand better performance boost (want to test with turbo mode too)?

@hrydgard
Copy link
Owner

I think the most interesting might be to try with really low-end Adreno devices that still allow driver replacements. Unfortunately I don't have any of those.

Maybe crank the resolution to see if the fragment shaders get more efficient?

Some relatively heavy games to run are the God of War games, Outrun, Wipeout Pure/Pulse, Tekken 6, GTA LCS / VCS...

BTW, does this driver has a way to set a stable power state (avoiding all the up and down clocking, setting a fixed slow clock for example)?

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 12, 2023

BTW, does this driver has a way to set a stable power state (avoiding all the up and down clocking, setting a fixed slow clock for example)?

not sure about setting a fixed slow clock, I can ask the dev of this lib. But if's possible to set stable high clocks with turbo mode, then probably it also possible to do it reversed

@hrydgard
Copy link
Owner

Just asking because it may be nice to have when optimizing shaders, to avoid false measurements due to the clock changing.

Our shaders are quite simple though generally.

@hrydgard
Copy link
Owner

How many times are you gonna force-push? :) Since this is your first PR, I have to approve each CI run manually :)

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 12, 2023

How many times are you gonna force-push? :) Since this is your first PR, I have to approve each CI run manually :)

oops, sorry :) I guess I'll wait for CI and try to force-push last time

@hrydgard
Copy link
Owner

hm, seem to be some include path issue?

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 12, 2023

hm, seem to be some include path issue?

yep, not sure why. Locally it works fine

Copy link
Owner

@hrydgard hrydgard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I know what it is. On CI here we build without gradle, instead we use a legacy Android.mk for, well, legacy reasons.

So, you'll need to add any new include paths and new cpp files to that as well.

I'd suggest adding include paths here:

https://github.com/hrydgard/ppsspp/blob/master/android/jni/Locals.mk#L24

Common/GPU/Vulkan/VulkanContext.cpp Outdated Show resolved Hide resolved
@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 12, 2023

Oh right, I know what it is. On CI here we build without gradle, instead we use a legacy Android.mk for, well, legacy reasons.

So, you'll need to add any new include paths and new cpp files to that as well.

I'd suggest adding include paths here:

https://github.com/hrydgard/ppsspp/blob/master/android/jni/Locals.mk#L24

I see. It's already late, so I'll do that tomorrow.
Also with gradle I'll need to add jniLibs.useLegacyPackaging = true to packagingOptions, because iirc drivers won't work on release builds. So I hope there won't be any problems with make since it doesn't use gradle

@hrydgard
Copy link
Owner

Won't work on release builds? That sounds weird.

We definitely still need to be able to make release builds, for Google Play. So we need to gate the driver loading feature from them?

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 12, 2023

Won't work on release builds? That sounds weird.

We definitely still need to be able to make release builds, for Google Play. So we need to gate the driver loading feature from them?

Ehh, maybe I didn't explain well. What I want to say is that custom driver loading won't work on release builds without jniLibs.useLegacyPackaging = true. So basically I need to add this option to gradle and loading will work on release builds too. But I am not sure how this works with Make since I never used it.
Probably Make doesn't even need this option to be specified, but I am not 100% sure

@hrydgard
Copy link
Owner

hrydgard commented Dec 12, 2023

Right, you can ignore it in Make since those builds are not actually used. We use gradle for all downloadable builds.

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 13, 2023

Let's hope it will compile now

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 13, 2023

Maybe crank the resolution to see if the fragment shaders get more efficient?

Didn't find any difference on my device

BTW, does this driver has a way to set a stable power state (avoiding all the up and down clocking, setting a fixed slow clock for example)?

The dev of this lib said that's impossible, unfortunately

@Cristobal15
Copy link

Cristobal15 commented Dec 13, 2023

Dima XD.
MotorStorm: Artic Edge and Porsuite Force Extreme Justice are among the games that Android devices struggle with the most. The SD865 has a hard time getting it up to 3X. They would be good test games.

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 13, 2023

Dima XD. MotorStorm: Artic Edge and Porsuite Force Extreme Justice are among the games that Android devices struggle with the most. The SD865 has a hard time getting it up to 3X. They would be good test games.

MotorStorm: Artic Edge with x4 resolution with Mesa driver seems like 1-3 fps is faster VS default driver.
But it's hard to measure. It's better to test with lower end devices

@hrydgard
Copy link
Owner

hrydgard commented Dec 13, 2023

../../Common/GPU/Vulkan/VulkanLoader.cpp:316: undefined reference to adrenotools_open_libvulkan'`

Think we need to add any cpp files from the adrenotools library as well to the Android.mk (if any), and add adrenotools_open_libvulkan as a library reference.

(Yes, I know it's a pain. we should probably switch CI to gradle).

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 13, 2023

../../Common/GPU/Vulkan/VulkanLoader.cpp:316: undefined reference to adrenotools_open_libvulkan'`

Think we need to add any cpp files from the adrenotools library as well to the Android.mk (if any), and add adrenotools_open_libvulkan as a library reference.

(Yes, I know it's a pain. we should probably switch CI to gradle).

You mean something like this? Unfortunately I can't even test it locally, and I don't want to trigger CI a lot

index 70fda2601..d0a90937b 100644
--- a/android/jni/Android.mk
+++ b/android/jni/Android.mk
@@ -140,6 +140,13 @@ RCHEEVOS_FILES := \
   ${SRC}/ext/rcheevos/src/rhash/hash.c \
   ${SRC}/ext/rcheevos/src/rhash/md5.c

+ADRENOTOOLS_FILES := \
+  ${SRC}/ext/libadrenotools/src/driver.cpp \
+  ${SRC}/ext/libadrenotools/src/hook/hook_impl.cpp \
+  ${SRC}/ext/libadrenotools/src/hook/file_redirect_hook.c \
+  ${SRC}/ext/libadrenotools/src/hook/gsl_alloc_hook.c \
+  ${SRC}/ext/libadrenotools/src/hook/main_hook.c
+
 VR_FILES := \
   $(SRC)/Common/VR/OpenXRLoader.cpp \
   $(SRC)/Common/VR/PPSSPPVR.cpp \
@@ -212,6 +219,7 @@ EXEC_AND_LIB_FILES := \
   $(SPIRV_CROSS_FILES) \
   $(RCHEEVOS_FILES) \
   $(NAETT_FILES) \
+  $(ADRENOTOOLS_FILES) \
   $(EXT_FILES) \
   $(NATIVE_FILES) \
   $(SRC)/Common/Buffer.cpp \

@hrydgard
Copy link
Owner

I can run the old build locally tomorrow and figure it out.

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 13, 2023

I can run the old build locally tomorrow and figure it out.

I just set up Make in WSL. Will fix building soon

@dima-xd
Copy link
Contributor Author

dima-xd commented Dec 13, 2023

../../Common/GPU/Vulkan/VulkanLoader.cpp:316: undefined reference to adrenotools_open_libvulkan'`

Think we need to add any cpp files from the adrenotools library as well to the Android.mk (if any), and add adrenotools_open_libvulkan as a library reference.

(Yes, I know it's a pain. we should probably switch CI to gradle).

This should be it.
I can do migration to gradle one day myself, if you don't mind

@hrydgard
Copy link
Owner

Well, we're gonna wanna talk to Unknown and see if he's still using that path. I don't use it myself anymore

@sum2012
Copy link
Collaborator

sum2012 commented Dec 13, 2023

@dima-xd please add translate string for "Current GPU Driver" and "Install Custom Driver..."
in \assets\lang*.ini

@hrydgard
Copy link
Owner

hrydgard commented Dec 13, 2023

Just noting that an easy way to add the translation strings is to use langtool, though it requires a rust install.

From Tools/langtool:

cargo run -- add-new-key Section Key

But, I can also take care of it afterwards.

I'm going to play with this myself a bit tomorrow and then merge, thanks for fixing the build.

@Narugakuruga
Copy link
Contributor

Qualcomm is well known for droppping support for old device. Even a 1 year old phone could be neglected and not receiving any driver update.

So it could be meaningful when PPSSPP need new vulkan extensions for extra features.

And better performance could mean lower power consumption, will benefit the overall user experience.

@hrydgard
Copy link
Owner

Some issues, which I'm happy to help fix after merging:

  • Choosing a file that's not a zip file (like, the unzipped .so file) gives no feedback that you picked the wrong file
  • It's a bit too prominent, and it's not really clear to a random user what it's asking for.
  • Our driver bug detection probably should special case Turnip and turn off our mitigations (hopefully it doesn't have the same bugs)...

So, I'm going to temporarily move this to Tools/Developer Tools for now, and later maybe migrate it back to Graphics settings.

But, super cool to have, and works great!

@hrydgard hrydgard merged commit 7634eba into hrydgard:master Dec 14, 2023
18 checks passed
@dima-xd dima-xd deleted the adrenotools branch December 14, 2023 14:57
@hrydgard hrydgard mentioned this pull request Dec 14, 2023
4 tasks
@hrydgard
Copy link
Owner

@dima-xd FYI, followup in #18548

@unknownbrackets
Copy link
Collaborator

Well, we're gonna wanna talk to Unknown and see if he's still using that path. I don't use it myself anymore

Gotta say, I really can't work my head around how you can stand making a button every time you want to try any little piece of code in PPSSPP. The idea of not being able to run ppssppunittest and ppssppheadless on Android phones is frustrating to me. Burns so much more time.

-[Unknown]

@hrydgard
Copy link
Owner

I run them on my Mac instead to get ARM64 coverage. But yeah. I probably would benefit from using that path more heh.

@kalebepalacio
Copy link

Is there really any advantage to using Turnip on ppsspp? I honestly don't see it, but I would like an official answer (if possible).

@hrydgard
Copy link
Owner

hrydgard commented Jun 9, 2024

Some people have indeed reported substantial speedups, though it seems to depend on the device and the particular driver it has by default. I'm personally not really convinced this is worth it but it was heavily requested and also externally contributed, so ...

@kalebepalacio
Copy link

I understood. Thanks for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants