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

[Draft] - Test Pioneer running on Raspberry Pi 5 #5700

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 22 additions & 17 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,23 +52,28 @@ if (APPLE)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-gnu")
endif(APPLE)

option(USE_SSE42 "Compile for SSE4.2 compatible microarchitectures (enables optimizations)" ON)

if (USE_SSE42)
if (NOT MSVC)
add_compile_options("-msse4.2" "-mlzcnt" "-mpopcnt")
endif()
endif (USE_SSE42)

option(USE_AVX2 "Compile for AVX2 compatible microarchitectures (Haswell and newer)" OFF)

if (USE_AVX2)
if (MSVC)
add_compile_options("/arch:AVX2")
else()
add_compile_options("-mavx2")
endif()
endif(USE_AVX2)
if(CMAKE_SYSTEM_PROCESSOR MATCHES "arm" OR CMAKE_SYSTEM_PROCESSOR MATCHES "aarch")
# maybe for performance we can add options here?
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -mcpu=cortex-a76 -ffast-math")
else()
Comment on lines +55 to +58
Copy link
Member

Choose a reason for hiding this comment

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

Instead of wrapping the USE_SSE42 etc. if-defs, prefer adding a new option(PIONEER_TARGET_RASPBERRY_PI) declaration with the compile flags and adding a preset definition to either CMakePresets.json or a new scripts/CMakeExperimentalPresets.json which sets the appropriate options:

"cacheVariables": {
     "USE_SSE42": false,
     "USE_AVX2": false,
     "PIONEER_TARGET_RASPBERRY_PI": true
}

If added to scripts/CMakeExperimentalPresets.json (preferred), you'll have to manually cp scripts/CMakeExperimentalPresets.json CMakeUserPresets.json before running e.g. cmake --preset linux-arm-raspberry-pi.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My only problem with that is defaulting to USE_SSE42 is only correct for x86/64 and wrong for PPC, Arm, RiscV etc so we should guard it somehow

Copy link
Member

Choose a reason for hiding this comment

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

If there's a way to default that option to off on non-x86 style platforms I'm all for it. Otherwise, it's assumed the options are managed via a CMake preset or explicit build flag set from a distribution build harness of some sort.

...which now that I think about it blindly omits Windows ARM64 builds, but I don't think I've seen anyone yet who's tried to build Pioneer on that platform.

option(USE_SSE42 "Compile for SSE4.2 compatible microarchitectures (enables optimizations)" ON)

if (USE_SSE42)
if (NOT MSVC)
add_compile_options("-msse4.2" "-mlzcnt" "-mpopcnt")
endif()
endif (USE_SSE42)

option(USE_AVX2 "Compile for AVX2 compatible microarchitectures (Haswell and newer)" OFF)

if (USE_AVX2)
if (MSVC)
add_compile_options("/arch:AVX2")
else()
add_compile_options("-mavx2")
endif()
endif(USE_AVX2)
endif(${CMAKE_SYSTEM_PROCESSOR} MATCHES "arm")

option(USE_LLD_LINKER "Use the LLVM lld linker instead of gcc's linker" OFF)
if (CMAKE_COMPILER_IS_GNUCXX)
Expand Down
18 changes: 17 additions & 1 deletion src/graphics/opengl/RendererGL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,16 @@ namespace Graphics {
Uint32 winFlags = 0;

winFlags |= SDL_WINDOW_OPENGL;
// We'd like a context that implements OpenGL 3.2 to allow creation of multisampled textures
// We'd like a context that implements OpenGL 3.1 to allow creation of multisampled textures
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 3);
#if (defined(__arm__) || defined(__aarch64__) || defined(__PPC64__) || defined(__riscv))
Copy link
Member

Choose a reason for hiding this comment

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

Instead of testing for platform feature flags here, prefer adding an explicit PIONEER_TARGET_RASPBERRY_PI define in buildopts.h and testing for that define. If we're doing legacy backwards-compatibility hacks like this, it should be opt-in at build time, rather than automatic based on CPU architecture.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try to work this out but I detest CMake, it's the worst possible build system and I can barely hack around in it at the best of times 🤷

Copy link
Member

Choose a reason for hiding this comment

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

I'd probably consider Autotools to be worse, but I completely understand where you're coming from.

// on Arm and other platforms we attempt to create an OpenGL 3.1 context as this is what Raspberry Pi 4 & 5 currently support
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 1);
#else
// x86/x64 we use newer OpenGL
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 2);
#endif

// Request core profile as we're uninterested in old fixed-function API
// also cannot initialise 3.x context on OSX with anything but CORE profile
SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_CORE);
Expand Down Expand Up @@ -180,11 +187,20 @@ namespace Graphics {
if (vs.enableDebugMessages)
GLDebug::Enable();

#if (defined(__arm__) || defined(__aarch64__) || defined(__PPC64__) || defined(__riscv))
// on Arm and other platforms we attempt to create an OpenGL 3.1 context as this is what Raspberry Pi 4 & 5 currently support
if (!glewIsSupported("GL_VERSION_3_1")) {
Error(
"Pioneer can not run on your device (Raspberry Pi?) as it does not appear to support OpenGL 3.1"
}
#else
// x86/x64 we use newer OpenGL
if (!glewIsSupported("GL_VERSION_3_2")) {
Error(
"Pioneer can not run on your graphics card as it does not appear to support OpenGL 3.2\n"
"Please check to see if your GPU driver vendor has an updated driver - or that drivers are installed correctly.");
}
#endif

if (!glewIsSupported("GL_EXT_texture_compression_s3tc")) {
if (glewIsSupported("GL_ARB_texture_compression")) {
Expand Down
Loading