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

lr-pcsx-rearmed - fix build by using the libretro makefile directly #2653

Merged
merged 1 commit into from Mar 19, 2019

Conversation

Projects
None yet
3 participants
@hhromic
Copy link
Contributor

hhromic commented Mar 15, 2019

  • After upstream commit b26afb9 the build broke because of the configure script incorrectly trying to always build gpu_unai.so
  • Instead use the libretro makefile directly and set platform as done in other cores in RetroPie
  • For non-RPI2/3 devices, use arm and neon flags to set platform
  • For none of the above, use unix as fallback platform
  • The built core is now correctly named: pcsx_rearmed_libretro.so

I tested the PR on an RPI3B+ using __platform set to rpi1, rpi2 and rpi3 to ensure it builds for all these variants. For other platforms it should use the correct parameters, but can't test myself. I also tested playing some games, all seems okay on my side.

@hhromic hhromic force-pushed the hhromic:fix-lr-pcsx-rearmed branch from d8d70d6 to 2b45a0b Mar 15, 2019

@hhromic

This comment has been minimized.

Copy link
Contributor Author

hhromic commented Mar 15, 2019

Just for completeness, I figured out the makefile parameters from the recipes in libretro-super. This is why the build doesn't fail for their buildbot.

@hhromic hhromic changed the title lr-pcsx-rearmed - fix build by using make directly [WIP] lr-pcsx-rearmed - fix build by using make directly Mar 15, 2019

@hhromic

This comment has been minimized.

Copy link
Contributor Author

hhromic commented Mar 15, 2019

I wil investigate this a bit more as gpu_unai.so might be good for older RPIs.
Please don't merge it yet, but keep it open. Thanks.

@retro-wertz

This comment has been minimized.

Copy link

retro-wertz commented Mar 15, 2019

make -f Makefile.libretro "${params[@]}" BUILTIN_GPU=unai to build unai gpu. dunno if it builds with your targets though, but it does with armeabi and ctr(3ds) which exclusively uses unai

@hhromic hhromic force-pushed the hhromic:fix-lr-pcsx-rearmed branch from 2b45a0b to d654d40 Mar 15, 2019

@hhromic

This comment has been minimized.

Copy link
Contributor Author

hhromic commented Mar 15, 2019

Okay I can confirm now that indeed gpu_unai.so was never used anyway. From the previous version of the scriptmodule using configure --platform=libretro --disable-neon:

# Automatically generated by configure
# Configured with: './configure' '--platform=libretro' '--disable-neon'
CC = gcc
CXX = g++
AS = as
CFLAGS +=  -fPIC -Wno-unused-result
ASFLAGS +=
LDFLAGS +=
MAIN_LDFLAGS +=  -shared -Wl,--no-undefined
MAIN_LDLIBS += -lpng  -ldl -lm -lpthread -lz
PLUGIN_CFLAGS +=  -fPIC

TARGET = libretro.so
ARCH = x86_64
PLATFORM = libretro
BUILTIN_GPU = peops
SOUND_DRIVERS = libretro
PLUGINS = plugins/spunull/spunull.so plugins/dfxvideo/gpu_peops.so plugins/gpu_unai/gpu_unai.so

It can be seen that BUILTIN_GPU is still peops, so building directly via the libretro makefile is correct.

I simplified the PR now to use the platform makefile variable properly. In summary, the following logic is applied according to the platform:

  • RPI3 -> platform=rpi3 -> gpu is set to neon
  • RPI2 -> platform=rpi2 -> gpu is set to neon
  • ARM devices (includes RPI1) -> platform=armv -> gpu is set to peops
  • ARM devices w/NEON -> platform=armvneon -> gpu is set to neon

The PR is now ready to review/merge.

@hhromic

This comment has been minimized.

Copy link
Contributor Author

hhromic commented Mar 15, 2019

@retro-wertz thanks for the clarification.
I just checked how it was built before and unai is never actually used for RetroPie target devices, so no problem with that after all. I'm now just configuring the build according to the platform and it picks the right choices out of the box.

@hhromic hhromic changed the title [WIP] lr-pcsx-rearmed - fix build by using make directly lr-pcsx-rearmed - fix build by using make directly Mar 15, 2019

@hhromic hhromic force-pushed the hhromic:fix-lr-pcsx-rearmed branch from d654d40 to f433e5f Mar 18, 2019

@hhromic

This comment has been minimized.

Copy link
Contributor Author

hhromic commented Mar 18, 2019

Added a fallback unix platform in case the device is neither ARM nor RPI. This core shouldn't be used in non-ARM devices anyway, but at least we don't fail on building in these now.

@hhromic hhromic changed the title lr-pcsx-rearmed - fix build by using make directly lr-pcsx-rearmed - fix build by using the libretro make directly Mar 18, 2019

lr-pcsx-rearmed - fix build by using libretro makefile directly
* After upstream commit b26afb9 the build broke because of the
  configure script incorrectly trying to always build `gpu_unai.so`
* Instead use the libretro makefile directly and set `platform` as
  done in other cores in RetroPie
* For non-RPI2/3 devices, use `arm` and `neon` flags to set platform
* For none of the above, use `unix` as fallback platform
* The built core is now correctly named: `pcsx_rearmed_libretro.so`

@hhromic hhromic force-pushed the hhromic:fix-lr-pcsx-rearmed branch from f433e5f to f9c26d3 Mar 18, 2019

@hhromic hhromic changed the title lr-pcsx-rearmed - fix build by using the libretro make directly lr-pcsx-rearmed - fix build by using the libretro makefile directly Mar 18, 2019

@joolswills

This comment has been minimized.

Copy link
Member

joolswills commented Mar 19, 2019

Thanks. Sorry for delay. Intended to test but was short on time. Looks good so merging.

@joolswills joolswills merged commit 8675cf2 into RetroPie:master Mar 19, 2019

@hhromic hhromic deleted the hhromic:fix-lr-pcsx-rearmed branch Mar 19, 2019

@hhromic

This comment has been minimized.

Copy link
Contributor Author

hhromic commented Mar 21, 2019

I just mentioned this in the forums, but in case you don't get notified: due to the shared library name change, the binaries in the RetroPie archive needs to be updated as well so the entry in emulators.cfg using the new core name works correctly.

@joolswills

This comment has been minimized.

Copy link
Member

joolswills commented Mar 21, 2019

Just a couple of comments due to the current issue. It may be better to not use the rpi2/rpi3 platform flags and just set the neon/dynarec stuff ourselves. Ideally we would use our own compiler flags too, and not the libretro makefile one.

@hhromic

This comment has been minimized.

Copy link
Contributor Author

hhromic commented Mar 21, 2019

Just a couple of comments due to the current issue. It may be better to not use the rpi2/rpi3 platform flags and just set the neon/dynarec stuff ourselves. Ideally we would use our own compiler flags too, and not the libretro makefile one.

If you think is better, sure I don't oppose. I thought the logic in the makefile for rpi2/3 should have been enough. Then, for this we don't pass platform (so it defaults to unix) and then we just pass HAVE_NEON=0/1, BUILTIN_GPU=peops/neon and USE_DYNAREC=0/1 depending on our flags. The makefile anyways will pick up the CFLAGS from RetroPie (it already does in the current state).

Do you want me to apply these changes? I can test them too. Anyway the PR I sent upstream should be approved shortly (in Discord they seem okay with it). However it won't affect what you suggest.

@hhromic

This comment has been minimized.

Copy link
Contributor Author

hhromic commented Mar 21, 2019

@joolswills the PR upstream is now merged.. the build for RPI0/1 should be good now. If you want to retry building the binaries. We can improve the scriptmodule with your suggestions with time.

@joolswills

This comment has been minimized.

Copy link
Member

joolswills commented Mar 22, 2019

I dont mind leaving the manual flags above if the cflags are not being overridden. Looking at the makefile I assumed they were. Will rebuild bins.

@hhromic

This comment has been minimized.

Copy link
Contributor Author

hhromic commented Mar 22, 2019

No the cflags are not being overriden, they are being appended (CFLAGS += ...). See the example below.
The appended cflags are very minimal and practically the same as th RetroPie cflags so all should be fine.


RPI3 RetroPie CFLAGS:

-O2 -march=armv8-a+crc -mtune=cortex-a53 -mfpu=neon-fp-armv8 -mfloat-abi=hard -ftree-vectorize -funsafe-math-optimizations

RPI3 compile command generated by the makefile:

cc -O2 -march=armv8-a+crc -mtune=cortex-a53 -mfpu=neon-fp-armv8 -mfloat-abi=hard -ftree-vectorize -funsafe-math-optimizations -pipe -DGIT_VERSION=\"" 9c22d06"\" -marm -mcpu=cortex-a53 -mfpu=neon-fp-armv8 -mfloat-abi=hard -fPIC -Wall -Iinclude -ffast-math -O2 -DNDEBUG -DFRONTEND_SUPPORTS_RGB565 -DHAVE_LIBRETRO -DNO_FRONTEND -c -o libpcsxcore/cdrom.o libpcsxcore/cdrom.c

RPI2 RetroPie CFLAGS:

-O2 -mcpu=cortex-a7 -mfpu=neon-vfpv4 -mfloat-abi=hard -ftree-vectorize -funsafe-math-optimizations

RPI2 compile command generated by the makefile:

cc -O2 -mcpu=cortex-a7 -mfpu=neon-vfpv4 -mfloat-abi=hard -ftree-vectorize -funsafe-math-optimizations -pipe -DGIT_VERSION=\"" 9c22d06"\" -marm -mcpu=cortex-a7 -mfpu=neon-vfpv4 -mfloat-abi=hard -fPIC -Wall -Iinclude -ffast-math -O2 -DNDEBUG -DFRONTEND_SUPPORTS_RGB565 -DHAVE_LIBRETRO -DNO_FRONTEND -c -o libpcsxcore/cdrom.o libpcsxcore/cdrom.c

RPI1 ReroPie CFLAGS:

-O2 -mcpu=arm1176jzf-s -mfpu=vfp -mfloat-abi=hard

RPI1 compile command generated by the makefile:

cc -O2 -mcpu=arm1176jzf-s -mfpu=vfp -mfloat-abi=hard -pipe -DGIT_VERSION=\"" 9c22d06"\" -marm -fPIC -Wall -Iinclude -ffast-math -O2 -DNDEBUG -DFRONTEND_SUPPORTS_RGB565 -DHAVE_LIBRETRO -DNO_FRONTEND -c -o libpcsxcore/cdrom.o libpcsxcore/cdrom.c
@joolswills

This comment has been minimized.

Copy link
Member

joolswills commented Mar 22, 2019

That's not ideal if they are but not a big issue either. Really RetroArch would be better not having these set in each makefile and setting them globally for their builds (like we do). But this isn't the only retropie module with this issue so I'll leave it for now.

@hhromic

This comment has been minimized.

Copy link
Contributor Author

hhromic commented Mar 23, 2019

Totally agree with that, the libretro makefiles are far from perfect and a lot of redundant/obscure logic. They have acknowledged this some times before, so I guess they would be happy to improve them. I will see in free time how that can be improved on that end and help there too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.