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

OPENDINGUX: Add support for Opendingux Beta #3415

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

@citral23
Copy link

@citral23 citral23 commented Oct 11, 2021

Hello, this is a V2 for my upstreaming proposal of Opendingux beta support.

It now uses an improved, unified build script, that takes the cpu variant as parameter (cf backends/platform/sdl/opendingux/README.BUILD).

It uses the keymapper API instead of the old custom, hardcoded events backend.

Thorough testing has ensure performance is adequate (for instance, discworld runs ok on the weakest CPU) and all options that have proven too demanding removed.

Hugetlb support has been dropped as it hasn't proven to be production worthy, and compilation flags refined.

I'm of course open to questions/remarks here or on discord.

KR

@digitall digitall changed the title For upstream Support for OpenDingux Beta Port Oct 11, 2021
configure Outdated
@@ -3208,6 +3209,9 @@ if test -n "$_host"; then
_optimization_level=-O1
append_var LDFLAGS "-L$GCCSDK_INSTALL_ENV/vfp/lib"
append_var CXXFLAGS "-isystem $GCCSDK_INSTALL_ENV/vfp/include"
if test -z "$PKG_CONFIG_LIBDIR"; then
Copy link
Member

@digitall digitall Oct 11, 2021

This change appears to be for the RISCOS port, rather than OpenDingux ...

If there are fixes for RISCOS, these should be separated out and sent as a separate commit / PR.

configure Outdated
@@ -3220,7 +3224,9 @@ if test -n "$_host"; then
arm-*riscos)
append_var LDFLAGS "-L$GCCSDK_INSTALL_ENV/lib"
append_var CXXFLAGS "-isystem $GCCSDK_INSTALL_ENV/include"
_pkgconfig=$GCCSDK_INSTALL_ENV/ro-pkg-config
if test -z "$PKG_CONFIG_LIBDIR"; then
Copy link
Member

@digitall digitall Oct 11, 2021

This change appears to be for the RISCOS port, rather than OpenDingux ...

If there are fixes for RISCOS, these should be separated out and sent as a separate commit / PR.

configure Outdated
@@ -5017,7 +5052,7 @@ case $_host_os in
;;

darwin*)
FLUIDSYNTH_STATIC_LIBS="$FLUIDSYNTH_LIBS -framework Foundation -framework CoreMIDI -framework CoreAudio -lglib-2.0 -lintl -liconv -lreadline"
FLUIDSYNTH_STATIC_LIBS="$FLUIDSYNTH_LIBS -framework Foundation -framework CoreServices -framework CoreMIDI -framework CoreAudio -framework AudioToolbox -lglib-2.0 -lintl -liconv -lreadline"
Copy link
Member

@digitall digitall Oct 11, 2021

This change appears to be for the OSX Darwin port, rather than OpenDingux ...

If there are fixes for OSX, these should be separated out and sent as a separate commit / PR.

Copy link
Member

@criezy criezy Oct 11, 2021

That change is one I made in 9024122. I am guessing it was included here by mistake (a rebase gone wrong?)

configure Outdated
@@ -6150,7 +6185,9 @@ case $_host_os in
fi
;;
riscos)
append_var CXXFLAGS "-mno-poke-function-name"
if test "$_debug_build" = no; then
Copy link
Member

@digitall digitall Oct 11, 2021

This change appears to be for the RISCOS port, rather than OpenDingux ...

If there are fixes for RISCOS, these should be separated out and sent as a separate commit / PR.

@digitall
Copy link
Member

@digitall digitall commented Oct 11, 2021

Looks fine with changes encapsulated as OpenDingux specific generally... though there seem to be some RISCOS and OSX changes in the configure script which should be at least separate commits, if not a different PR so that the RISCOS / MacOS porters have visibility and can review.

@sev-
Copy link
Member

@sev- sev- commented Oct 11, 2021

Before even looking closely at the changes, could you please bring the commit log messages in line with our Commit Guidelines? https://wiki.scummvm.org/index.php?title=Commit_Guidelines

@citral23 citral23 force-pushed the for-upstream branch 2 times, most recently from e7907ae to b4bde2b Oct 12, 2021
@citral23 citral23 changed the title Support for OpenDingux Beta Port Add support for Opendingux Beta Oct 12, 2021
@citral23
Copy link
Author

@citral23 citral23 commented Oct 12, 2021

Hello, yes sorry it was a rebase problem.
Fixed that, and stashed my commits into a single one with a proper description.
KR

Copy link
Member

@lephilousophe lephilousophe left a comment

The commit message should begin with OPENDINGUX: prefix.

With the changes to get rid of build_odbeta.sh applied, it should be fairly simple to integrate this new opendingux build in Buildbot.
I don't see support for A320/A330 platform though. Are they still supported?
Is there a repository where all the toolchain code is? We try to not depend on binary toolchains to avoid being stuck with old distributions.

@@ -0,0 +1,41 @@
#!/bin/bash

CONFIG="./configure --host=opendingux --enable-release --disable-detection-full"
Copy link
Member

@lephilousophe lephilousophe Oct 17, 2021

It would be better to integrate all of this script in configure script.
It has been done for other platforms.
host could be opendingux-platform

Copy link
Author

@citral23 citral23 Oct 22, 2021

I don't think so, because we support 3 cpus which each need their own toolchain :
mips32r2 fpu aka gcw0
mips32r1 fpu aka lepus
mips32r1 no fpu aka rs90

And 2 variants for each (normal, and dualopk which adds needed autodetect functionalities for a popular emulationstation-like frontend called simplemenu)

this would be cumbersome to integrate in configure, furthermore, it's best if any change is needed in the future, to do it in build_odbeta.sh than touch configure which is more sensible imho as it's shared by all platforms.

toolchains are here : https://github.com/OpenDingux/buildroot/actions
each new commit triggers an action to build the toolchains, updaters and flashers


export PATH=$TOOLCHAIN/usr/bin:$SYSROOT/usr/include:$TOOLCHAIN/bin:$PATH
export CXX=mipsel-linux-g++
export CXXFLAGS="-funsigned-char" # workaround for a scummvm tolower() bug when adding games
Copy link
Member

@lephilousophe lephilousophe Oct 17, 2021

Is this really specific to opendingux?

Copy link
Author

@citral23 citral23 Oct 22, 2021

I think it applies to any MIPS cpu, because on mips char is signed by default. In that regard I suspect the GCW0 port is probably not functional at the moment.

OD Beta is a mips linux OS based on buildroot, that runs on ingenic gaming handhelds
like the gcw0, rg350 and many more.

There was a previous GCW0 port, that was running on legacy opendingux, but that
OS is not maintained since 2014 and has an outdated kernel and toolchain.

Od Beta on the other hand, runs with linux 5.15, has a GCC 10 toolchain and
an active developpment community.

This adds support for 3 flavors :

gcw0 that runs on all jz4770 cpus
lepus that runs on all jz4760 cpus
rg99 which is a jz4725 device

Signed-off-by: Christophe Branchereau <cbranchereau@gmail.com>
@citral23 citral23 changed the title Add support for Opendingux Beta OPENDINGUX: Add support for Opendingux Beta Oct 22, 2021
@citral23
Copy link
Author

@citral23 citral23 commented Oct 22, 2021

Renamed commit/PR titles with prefix and tried to answer the comments individually, one question remains to be adressed :
the original Dingoo A320 is not supported by Opendingux Beta at the moment.
It has its fair share of challenges but is not excluded in the future, WIP.

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