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

WII: Implement changes needed by DevKitPPC R26 and later #413

Merged
merged 1 commit into from Jan 21, 2014

Conversation

Projects
None yet
6 participants
@AReim1982
Contributor

AReim1982 commented Nov 22, 2013

This pull request makes ScummVM compilable with newer versions of DevKitPPC. ScummVM can be linked against the original libogc and libfat. That makes some newer WiiMotes work (FR#671), improves audio-/video-playback and contains various improvements.

@somaen

This comment has been minimized.

Member

somaen commented Nov 22, 2013

The commit messages don't follow the convention we use, i.e. it lacks the proper prefixes.

@digitall

This comment has been minimized.

Member

digitall commented Nov 22, 2013

@AReim1982 : Thank you for the updates to the newer Wii DevkitPPC. Alexander, for future reference, we prefer PR to be logical changesets with descriptions as Lordhoto indicated... but any work on the Wii backend is welcomed. I will look at updating the buildbot SDK to r26 and moving the current SDK to a deprecated folder. I will then look at cherry-picking your changes to our master as logical changesets... I can then test the result on my Wii...

@lordhoto

This comment has been minimized.

Member

lordhoto commented Nov 22, 2013

First of all thank you for your pull request. I have some questions/suggestions about these changes:

  1. Is there any particular reason why all the commits mention devkit PPC r25 but this pull request mentions r26?
  2. I think this breaks building with our devkit PPC r16 on buildbot (at least a quick compile check confirms this, becuase, for example, KEYBOARD_Init now takes a parameter, see also this post about this general issue: http://forums.scummvm.org/viewtopic.php?p=63007#63007).
  3. Can we squash all these changes into one commit and give it some more meaningfull commit message like "WII: Update code to work with devkit PPC r26" (or similar)?
    EDIT:
  4. This post mentions some issue with libfat and this homebrew channel (due to renaming of devices in libfat) http://forums.scummvm.org/viewtopic.php?p=43133#43133 Is this still anything to worry about?
@digitall

This comment has been minimized.

Member

digitall commented Nov 22, 2013

@lordhoto : +1 to your questions... Re: 2, I was intending to add the latest (r26?) SDK leaving the old SDK in place and available and switch the path over in the buildbot master.cfg wii builder... then I was intending to add the changes, but guarded with a preprocessor if-else to switch based on the SDK version to maintain r16 build until we are happy with r26+.

@digitall

This comment has been minimized.

Member

digitall commented Nov 22, 2013

AH right... r26 is the latest release. See:
http://sourceforge.net/projects/devkitpro/files/devkitPPC/

@lordhoto

This comment has been minimized.

Member

lordhoto commented Nov 22, 2013

AH right... r26 is the latest release. See:
http://sourceforge.net/projects/devkitpro/files/devkitPPC/

I noticed that. Let me rephae the question to make it more clear: The commit messages seem to suggest that the changes were made to fix compilation with r25. Is that correct, i.e. can one also use r25 to build after these changes are merged? (In that case it might make more sense to state that in the pull request title too, i.e. "Implement changes needed by devkit PPC r25 and later" or similar).

Re: 2, I was intending to add the latest (r26?) SDK leaving the old SDK in place and available and switch the path over in the buildbot master.cfg wii builder... then I was intending to add the changes, but guarded with a preprocessor if-else to switch based on the SDK version to maintain r16 build until we are happy with r26+

That sounds good if we really want to keep support for r16. But is there really any way to determine the devkit version?

@digitall

This comment has been minimized.

Member

digitall commented Nov 23, 2013

Sigh! I had thought there would be a define for this... Just looked through the code and there isn't. We can still detect this, but not directly... r16 uses GCC 4.2.4 and r26 uses GCC 4.6.3. This may need support or changes in configure / expected environment variable of DEVKITPRO_VERSION or similar... (The rules already depend on a DEVKITPRO environment variable for path so adding one for version is not a major leap).

My point here was to keep compatiblity with the old toolchain until we are happy that the new builds are good. I know that dhewg had issues with newer libogc a while back...

Either way, I doubt the above changes are all that is required... The compiler prefix has changed between powerpc-gekko in r16 and powerpc-eabi in r26 and there may be other things we need to amend in configure and other parts of buildchain...

@AReim1982 : Have you tested building this? On what platform?

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Nov 23, 2013

@lordhoto:

  1. that was a fault, my actual build environment is PPC R26
  2. sure, that breaks the actual buildbot
  3. sure
  4. my changes don't breaks the libfat and the hbc

@digitall:
yes, i tested that changes on debian wheezy amd64 (Linux reim-laptop 3.2.0-4-amd64 #1 SMP Debian 3.2.51-1 x86_64 GNU/Linux).

Yesterday and today I tested my build with all my games. The most games works fine... but comi, ft and feeble
throws an dsi exception at startup. Maybe one of my 3rd party libs don't work. Is it possible to get the
3rd party libs from the buildbot server. I could link my build agains the old libs. Hopefully there is no more bug in libogc
or libfat.

@digitall

This comment has been minimized.

Member

digitall commented Nov 24, 2013

@AReim1982 : I have checked on the buildbot and this uses libOGC v1.8.3 currently. libjpeg-turbo, libpng-1.5.13, zlib, tremor (Vorbis) and FLAC libraries are compiled. Please see here for patches to FLAC and Tremor:
http://wiki.scummvm.org/index.php/Compiling_ScummVM/Wii

I need to look at fixing the buildbot to ensure we can rebuild the libraries from scratch as the links on that page to precompiled binaries and the unofficial forks of libogc and libfat are broken... at least currently.

I would hope the normal libOGC and libfat have merged the differences in.

@dhewg : Any comment?

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Nov 29, 2013

I also made some changes on Libgxflux in my repository. Now it's possible to compile ScummVM and Libgxflux with DevKitPPC R26 and the original Libogc and Libfat.

But actually the Scumm-7-8-Engine is broken. All other engines works well. Does anybody know why that engine could be broken? I try to fix that issue, but it could take some time for testing and debugging.

@bluegr

This comment has been minimized.

Member

bluegr commented Dec 2, 2013

Btw, a related patch is here:
https://sourceforge.net/p/scummvm/patches/1359/
...however it has bitrotted, and we should probably close it. Perhaps there's something interesting to salvage from it, into this pull request?

@digitall

This comment has been minimized.

Member

digitall commented Dec 2, 2013

Looks like quite a bit useful in that patch to be merged in... Will need to think about how to do this.

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Dec 4, 2013

Good news everyone... I fixed all issues and now all games works perfectly.
Now it's possible to compile ScummVM with the newest DevKitPPC and the original LibOGC and LibFAT. But my fixed LibGXFlux-Fork is needed: http://github.com/AReim1982/libgxflux.

Is it possible to release an test-candidate on the scummvm-forums? Maybe more people should test my changes.
( test-candidate of ScummVM 1.6.0: https://github.com/AReim1982/binaries )

@lordhoto

This comment has been minimized.

Member

lordhoto commented Dec 8, 2013

Now it's possible to compile ScummVM with the newest DevKitPPC and the original LibOGC and LibFAT. But my fixed LibGXFlux-Fork is needed: http://github.com/AReim1982/libgxflux.

Is there any chance that your changes can be integrated in upstream?

Is it possible to release an test-candidate on the scummvm-forums? Maybe more people should test my changes.

I think that should be fine.

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Dec 10, 2013

Is there any chance that your changes can be integrated in upstream?

I forked Dhewg's LibGxFlux-Repository. Maybe somebody could ask Dhewg.

@bluegr

This comment has been minimized.

Member

bluegr commented Dec 13, 2013

@AReim1982: Could you explain your changes to LibGXFlux?

Why do you need to change the video mode? And why is stddef.h needed?

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Dec 13, 2013

Why do you need to change the video mode?

In the newer versions of LibOgc the video mode TVPal574IntDfScale was changed to TVPal576IntDfScale. Maybe they changed the video mode because european pal has 576 rows and not 574.

And why is stddef.h needed?

In newer versions of DevKitPPC/LibOgc it seems that "stddef.h" is not automatically included. The file "gfx_con.c" can not be compiled because the compiler don't know the structure "size_t". But "size_t" is defined in "stddef.h".

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Jan 13, 2014

@bluegr: The configure-script automatically detects it and sets the preprocessor directive: USE_TASKBAR. Maybe something with the configure-script is wrong, but i don't want to touch it. I thought that the taskbar never will be used with the wii port. That is the reason why I completely remove it for the wii port.

@bluegr

This comment has been minimized.

Member

bluegr commented Jan 13, 2014

@AReim1982: You can disable the libunity feature in the configure script, which is the cleanest approach, IMHO.

Check around line 2707 in the configure file. That's where the Wii specific defines are. You can just add this line in the Wii section:

_taskbar=no

which will disable the Unity taskbar

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Jan 14, 2014

@bluegr: Thanks for your help!

@bluegr

This comment has been minimized.

Member

bluegr commented Jan 19, 2014

As fuzzie pointed out, the commits here need to be squashed. Also, the commit messages should be adjusted as per our formatting guidelines. Finally, it would be nice if you could check if the patch mentioned above has anything interesting to include in the changes of this pull request

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Jan 20, 2014

As fuzzie pointed out, the commits here need to be squashed. Also, the commit messages should be adjusted as per our formatting guidelines.

Now I sqashed all commits into a single commit with new commit message and hopefully anything is ok now... :-)

Finally, it would be nice if you could check if the patch mentioned above has anything interesting to include in the changes of this pull request

There is no more interesting stuff in that old patch.

@fuzzie

This comment has been minimized.

Member

fuzzie commented Jan 20, 2014

Note that as digitall said earlier, we shouldn't merge this until the new toolchain is tested-good and someone updates buildbot (since this breaks compatibility).

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Jan 20, 2014

Note that as digitall said earlier, we shouldn't merge this until the new toolchain is tested-good and someone updates buildbot (since this breaks compatibility).

My Test-Release was downloaded more than 150 times from the ScummVM Forums and 2 further forums also share that download. In that time I fixed performance issues and one bug in the mass add feature. Now all people seems to be satisfied. I don't know how long we should wait, because the old toolchain prevent the further developement of the scummvm wiibuild. Also there are many bugs related to that old toolchain.

@digitall

This comment has been minimized.

Member

digitall commented Jan 21, 2014

@AReim1982 : Looking much better. One minor point, could you remove the trailing whitespace from line 41 of backends/fs/wii/wii-fs.h . Overall, I would be happy to merge this as-is and I can then look at upgrading the buildbot toolchain fairly quickly.

WII: Implement changes needed by DevKitPPC R26 and later
This changes makes ScummVM compilable with newer versions of DevKitPPC. ScummVM can be linked against the original libogc and libfat. That makes some newer WiiMotes work, improves audio-/video-playback and contains various improvements.
@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Jan 21, 2014

@digitall : Ok, I removed the trailing whitespace.. ;-)

@digitall

This comment has been minimized.

Member

digitall commented Jan 21, 2014

@AReim1982 : OK, thanks! I will deal with merging this in an hour or so, when I am ready to switch over the buildbot toolchain just after the merge.

@digitall

This comment has been minimized.

Member

digitall commented Jan 21, 2014

Merging now... Will update buildbot toolchain.

digitall added a commit that referenced this pull request Jan 21, 2014

Merge pull request #413 from AReim1982/master
WII: Implement changes needed by DevKitPPC R26 and later

@digitall digitall merged commit b098b74 into scummvm:master Jan 21, 2014

@digitall

This comment has been minimized.

Member

digitall commented Jan 25, 2014

@AReim1982 : Afternoon. Could you take another look at this code. It builds fine for Wii, but there are some omissions for building for the Gamecube which are breaking the build. I fixed one in the configure, but the build is still failing with:
ccache powerpc-eabi-g++ -L/opt/toolchains/devkitPPC-r26/3rd/lib -mogc -mcpu=750 -L/opt/toolchains/devkitPPC-r26/libogc/lib/cube -logc -Wl,--no-gc-sections -Wl,--whole-archive backends/platform/wii/main.o backends/platform/wii/options.o backends/platform/wii/osystem.o backends/platform/wii/osystem_gfx.o backends/platform/wii/osystem_sfx.o backends/platform/wii/osystem_events.o base/libbase.a gui/libgui.a backends/libbackends.a engines/libengines.a video/libvideo.a graphics/libgraphics.a audio/libaudio.a common/libcommon.a -Wl,--no-whole-archive -lgxflux -liso9660 -lfat -logc -ldb -lm -lvorbisidec -lmad -ljpeg -lpng -lz -lz -lfreetype -lz -o scummvm.elf
backends/libbackends.a(wii-fs-factory.o):(.sdata.dvd+0x0): undefined reference to `__io_wiidvd'
collect2: ld returned 1 exit status

Not sure if this is a problem with my compilation of the toolchain for the buildbot though? Any ideas?

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Jan 25, 2014

@digitall : Afternoon. It's my fault... :-(
The problem is the row: 128 in the file: ./backends/fs/wii/wii-fs-factory.cpp.
This row should be encased in: #ifdef USE_WII_DI ... #endif

Should I open a new pull request or do you like to change it youself?

@digitall

This comment has been minimized.

Member

digitall commented Jan 25, 2014

@AReim1982 : No need. Done in 417f755. Thanks for the help.

@digitall

This comment has been minimized.

Member

digitall commented Jan 25, 2014

@AReim1982 : Ah. That is not sufficient to fix the Gamecube build. The USE_WII_DI symbol is set for the Gamecube build in the configure script at line 2468:
https://github.com/scummvm/scummvm/blob/master/configure#L2468

I could fix this by commenting that out as per #define DEBUG_WII_GDB etc. ... but I assume that the Gamecube can access CD/DVDs, so not sure what is going on here...

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Jan 25, 2014

@digitall : That should fix our problem:

#ifndef GAMECUBE
const DISC_INTERFACE* dvd = & __io_wiidvd;
#else
const DISC_INTERFACE* dvd = & __io_gcdvd;
#endif

P.S. remove space between & and __

@digitall

This comment has been minimized.

Member

digitall commented Jan 25, 2014

@AReim1982 : Thanks. Committed as 08d3b57. Will have to see if buildbot is now happy...

@digitall

This comment has been minimized.

Member

digitall commented Jan 25, 2014

@AReim1982 : Nope. Still failing... Did you try to compile this with your toolchain with --host=gamecube instead of --host=wii ?

../../src-master/src/backends/fs/wii/wii-fs-factory.cpp:132:32: error: '__io_gcdvd' was not declared in this scope

@AReim1982

This comment has been minimized.

Contributor

AReim1982 commented Jan 26, 2014

@digitall : Morning. This weekend I have no access to some computers. But at monday I can try to fix this.
But I'm sure if you add the following line the gc-port should compile again...

After row 34:
#include < ogc/dvd.h >

Try it yourself or I will do it at monday.

I'm sorry about because I didn't test my changes on the gc-build... :-(

P.S. again remove the 2 spaces.

@digitall

This comment has been minimized.

Member

digitall commented Jan 26, 2014

@AReim1982 : OK. Have committed this change after checking the headers as commit b6e5865. Will see if buildbot is now happy.

@digitall

This comment has been minimized.

Member

digitall commented Jan 26, 2014

@AReim1982 : Buildbot has now successfully built on Gamecube. Thanks for your help.

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