notaz
Certified Guru
We still haven't agreed with bsp on how it would be included, until that's done inclusion is not going to happen.Firmware shouldnt wait for software to come first. an OS s role is to enable you to do things.
We still haven't agreed with bsp on how it would be included, until that's done inclusion is not going to happen.Firmware shouldnt wait for software to come first. an OS s role is to enable you to do things.
Could you write a new thread with :- Instruction to use you libraries for DSP virgin devs@EvilDragon: thanks for adding the new development section!
Good idea!Could you write a new thread with :@EvilDragon: thanks for adding the new development section!
- Instruction to use you libraries for DSP virgin devs
- Any usefull material you have at hands. Mods will sticky it, so we could have that at hand in no time
I suggest /lib/dsp/c64_tools/images/ because we already have /lib/dsp/ for DSP.
- non-application specific DSP images are placed in C64_IMAGE_SITE_PATH (currently set to "/lib/c64_tools/images/"). At the moment, this would be just the core.out image (rather small)
ok. Are you ready too keep supporting that API? I.e. when it's included it should no longer be changed, only expanded, so it's a good chance to tune it for the last time now before it's included.
- libc64.so goes to /usr/lib/
I'd prefer /usr/sbin, /sbin is usually for limited set of core stuff.
- c64_load goes to /sbin/
Ok /usr/pandora/scripts/op_dsp_c64.sh sounds good to me. I don't think we should be asking for password, as it will be part of firmware, it can at least pretend it knows what it's doing and load stuff silently. Otherwise it will just annoy the user everytime he wants to run something that supports the DSP.
- go64.sh goes to ? (/usr/pandora/scripts/?). Applications/PNDs that want to use the DSP must call the script. The first app. using the DSP (after a reboot) will cause a dialog to popup and ask for the root password, then load the c64.ko kernel module and the core.out "DSP firmware". You can either change "op_dsp.sh" to use c64_tools instead of DspBridge, or provide two separate scripts, e.g. "op_dsp_bridgedriver.sh" and "op_dsp_c64.sh" since only one of these drivers can use the DSP (for obvious reasons). The dialog feature still needs to be implemented. Should I do that or will you do it ?
Agreed.
- the example DSP images and applications should be bundled in a PND, no reason to install them in NAND (your choice, they do not take up much space but IMHO the NAND should only contain essential things). This would definitely by my task.
ok.You said that there is no release date for the next firmware update and the only reason for an update will be the c64_tools.
If that is still true, I would suggest to wait for the next update (next weekend or the one after next).
that could be confusing. what about using the same dir. (/lib/dsp) for c64_tools images as well ? The dir. is almost empty right now and dspbridge uses a different image file suffix (.dof) than c64_tools (.out).I suggest /lib/dsp/c64_tools/images/ because we already have /lib/dsp/ for DSP.
- non-application specific DSP images are placed in C64_IMAGE_SITE_PATH (currently set to "/lib/c64_tools/images/"). At the moment, this would be just the core.out image (rather small)
I am not the kind of developer who frequently changes his APIs (in fact, I like fixed APIs). I encourage everyone who reads this to review the API (in dsp.h) and voice any concerns about it.ok. Are you ready too keep supporting that API? I.e. when it's included it should no longer be changed, only expanded, so it's a good chance to tune it for the last time now before it's included.
- libc64.so goes to /usr/lib/
I'd prefer /usr/sbin, /sbin is usually for limited set of core stuff.
- c64_load goes to /sbin/
Ok /usr/pandora/scripts/op_dsp_c64.sh sounds good to me. I don't think we should be asking for password, as it will be part of firmware, it can at least pretend it knows what it's doing and load stuff silently. Otherwise it will just annoy the user everytime he wants to run something that supports the DSP.
http://www.pathname.com/fhs/pub/fhs-2.3.html#SBINSYSTEMBINARIESI'd prefer /usr/sbin, /sbin is usually for limited set of core stuff.
- c64_load goes to /sbin/
Hmm, that was the idea. It does not get much more low level / core than this ?!
My understanding of the filesystem structure is that anything close to HW goes to /sbin, and the more high level system-level software goes to /usr/sbin.
Utilities used for system administration (and other root-only commands) are stored in /sbin, /usr/sbin, and /usr/local/sbin. /sbin contains binaries essential for booting, restoring, recovering, and/or repairing the system in addition to the binaries in /bin. [18] Programs executed after /usr is known to be mounted (when there are no problems) are generally placed into /usr/sbin. Locally-installed system administration programs should be placed into /usr/local/sbin. [19]
I'll do that (originally I wanted to write a Wiki article but this could start as a thread, why not). The doclinks are in the "Reference material" thread.Could you write a new thread with :
- Instruction to use you libraries for DSP virgin devs
- Any usefull material you have at hands. Mods will sticky it, so we could have that at hand in no time
Ok let it be /lib/dsp/ then.that could be confusing. what about using the same dir. (/lib/dsp) for c64_tools images as well ? The dir. is almost empty right now and dspbridge uses a different image file suffix (.dof) than c64_tools (.out).
One problem with the dialog is that fullscreen framebuffer apps can't use it. Your dialog will be started but the user won't see it because app is controlling the whole screen. That will look like the fb app hung while it's actually waiting for your script to finish, and your script is in turn waiting for invisible uncontrollable dialog. You'd also have dependency on X server which isn't needed for the job of booting the DSP.ok, agreed then (regarding script naming). I'll write the script then. I'd still suggest to use the dialog but I'll make that configurable. I assume you are suggesting to set the "setuid" flag for the script instead of the dialog ?Ok /usr/pandora/scripts/op_dsp_c64.sh sounds good to me. I don't think we should be asking for password, as it will be part of firmware, it can at least pretend it knows what it's doing and load stuff silently. Otherwise it will just annoy the user everytime he wants to run something that supports the DSP.
+1 to that; I write and compile all my Pandora stuff on the Pandora itself.We should probably also create a -dev package, so the includes should go to /usr/include/ somewhere, I suggest /usr/include/c64_tools/ (that's optional, but would be nice as we have some people compiling on pandoras themselves).
But get ready to use a compiler that will create shitty DSP code thuss probably slower than the ARM core could do...+1 to that; I write and compile all my Pandora stuff on the Pandora itself.
I'm not planning to write DSP code, but maybe I will call some DSP routines from some standard library. For that I would still need the libdsp include files.But get ready to use a compiler that will create shitty DSP code thuss probably slower than the ARM core could do...+1 to that; I write and compile all my Pandora stuff on the Pandora itself.
good point. Now that you mention this I've seen this happening with the WiFi dialog as well (e.g. when toggling WiFi from the MiniMenu). Maybe this could (somehow) be addressed in the firmware update as well ?One problem with the dialog is that fullscreen framebuffer apps can't use it. Your dialog will be started but the user won't see it because app is controlling the whole screen.
You may have noticed that I used inc_libc64.h only in the examples. I dislike header files that include other header files, I like to be able to quickly see what headers are effectively included in a source file. The inc_libc64.h is a compromise for devs who are not too familiar with the libc64 header file dependencies and rather have a few unused header files included than spend time figuring out the dependencies.would be nice not to have to include include/inc_libc64.h, just include/dsp.h instead and that dsp.h would take care of it's dependencies. inc_libc64.h pulls in stuff that not everyone needs, like log, osal or c64_dev.h with ioctl numbers that apps shouldn't really use (the whole point of libc64). It also increases chances of name collisions if DSP is integrated in some existing program.
fixed that just now. c64_dev.h is now LGPL and also not included by inc_libc64.h anymore (was not needed anyway). libc64 internal dsp_* functions were moved to dsp_priv.h. dsp_open() can now be called multiple times (the 2nd (redudant) call will print a warning). DSP_WRITE_REGS was indeed a leftover and was used nowhere, I removed it.
- inc_libc64.h includes c64_dev.h and c64_dev.h is marked GPL only, making impossible to use libc64 from non-GPL program.
- maybe move c64_load stuff out of dsp.h (dsp_private.h?) to reduce confusion?
- maybe error out on multiple dsp_open()s in a row?
- DSP_WRITE_REGS in dsp.h - some leftover?
Regarding support for different memory attributes: I was thinking about adding this, too (cached + uncached types) (CMEM has this feature, too). Having a (third) write-through memory type would indeed be very useful.Write-through cached (on GPP side) shared memory. Such memory is cached but all writes always go to RAM, meaning that no cache cleaning is needed to pass data to DSP, improving performance (Cortex-A8 cache operations are slow). Perhaps this could be implemented as memory attribute argument to dsp_shm_init/dsp_shm_get, which would also allow uncached memory (might be useful in some cases).
[..]
With the above, it might make sense to allow 2 shared memory regions with different memory attributes, one for streaming data to DSP and one for streaming back.
Sounds interesting but I am totally unfamiliar with the huge page feature. I would suggest that I'll upload the c64_tools sources to github once the API is final, then you can add this feature if you know how to do that and find the time.Allow using user-supplied buffer as shared memory. This would be limited to either single 4kB page, or 2MB huge page (both are contiguous in physical RAM). I'm already using huge pages in PCSX ReARMed which allows to reduce TLB cache misses, noticeably improving performance. I'd like to be able to access those buffers from DSP.
I am using the code sourcery arm-2011.09 toolchain here. I removed the colons and the code still compiles fine so consider this fixed."\n\t nop"- :: ;
+ );
interestingly, the cmdline above triggers sporadic link errors (wth?!).the build also system wants libc64.so to have a version, so you need to use -Wl,-soname,libc64.so.1 or similar.
ok, that can be done later (has nothing to do with firmware update, right?)We should probably also create a -dev package
Maybe, but I don't have many ideas about that.good point. Now that you mention this I've seen this happening with the WiFi dialog as well (e.g. when toggling WiFi from the MiniMenu). Maybe this could (somehow) be addressed in the firmware update as well ?One problem with the dialog is that fullscreen framebuffer apps can't use it. Your dialog will be started but the user won't see it because app is controlling the whole screen.
Then you should add#ifndef SOMEFILE_INCLUDEDYou may have noticed that I used inc_libc64.h only in the examples. I dislike header files that include other header files, I like to be able to quickly see what headers are effectively included in a source file.
But it pulls in quite some potentially unwanted extras like logging and the osal thing. At least something like inc_libc64_minimal.h would be nice if you really want to have it that way.The inc_libc64.h is a compromise for devs who are not too familiar with the libc64 header file dependencies and rather have a few unused header files included than spend time figuring out the dependencies.
A proper header file should only include it's dependencies and nothing else. Imagine if including <stdio.h> would spit errors at you until you found and included some 10+ dependencies that it needs..If I were to move the content of inc_libc64.h to dsp.h, it would only make the situation worse because dsp.h would still include possibly unused headers and there would be no way to only include what's really needed (e.g. skip log.h if you have your own log routines).
Thanks.fixed that just now. c64_dev.h is now LGPL and also not included by inc_libc64.h anymore (was not needed anyway). libc64 internal dsp_* functions were moved to dsp_priv.h. dsp_open() can now be called multiple times (the 2nd (redudant) call will print a warning). DSP_WRITE_REGS was indeed a leftover and was used nowhere, I removed it.
I'm not sure too but it can be done one way or another. I don't think it's worth delaying the release though, I was just bringing this up for API considerations.TBH, I have no idea how to implement this [write-through mem. type], though. The DMA kernel API does not support this, yet, as far as I can see (looked at arch/arm/include/asm/dma-mapping.h and include/linux/dma-attrs.h). I presume this would require a kernel modification ?
If that's true it might be a good idea to add this before releasing a new firmware..?
For large buffers (above L2 cache size) flush_cache_all() is actually better because otherwise the code has to walk the whole address range in cacheline steps issuing slow cache control instructions. But I agree this should be looked at eventually for small buffers at least.It would also be very good if someone could investigate why the CACHE_AC_INV and CACHE_AC_WBINV ioctls resp. dmac_map_area() kernel calls cause sporadic system freezes. Currently flush_cache_all() is called instead. CACHE_AC_WB (also uses dmac_map_area()) works fine, though.
I would say that this should have higher priority than the write-through feature since the performance impact (in general) will be bigger.
Sounds good to me.Sounds interesting but I am totally unfamiliar with the huge page feature. I would suggest that I'll upload the c64_tools sources to github once the API is final, then you can add this feature if you know how to do that and find the time.Allow using user-supplied buffer as shared memory. This would be limited to either single 4kB page, or 2MB huge page (both are contiguous in physical RAM). I'm already using huge pages in PCSX ReARMed which allows to reduce TLB cache misses, noticeably improving performance. I'd like to be able to access those buffers from DSP.
That won't work because it puts the soname into ELF headers and the OE build system is checking that. Doesn't doing "-soname,libc64.so.1" and "ln -s libc64.so.1 libc64.so" work?interestingly, the cmdline above triggers sporadic link errors (wth?!).the build also system wants libc64.so to have a version, so you need to use -Wl,-soname,libc64.so.1 or similar.
Is it ok to use "-soname,libc64.so", then mv libc64.so to libc64.so.1 and ln -s libc64.so.1 libc64.so ?
(just added that to the makefile)
The -dev package is automatically built and published, if we don't fix it now it will just make a broken -dev package. You just need to tell me include location (I suggest /usr/include/c64_tools/) and I'll take care of the rest.ok, that can be done later (has nothing to do with firmware update, right?)For now, just don't try to build things within the dsp/ directory. That should work fine on the Pandora itself.We should probably also create a -dev package
okay, I'll add that.At least something like inc_libc64_minimal.h would be nice if you really want to have it that way.
I am currently busy adding the three different mem types (cached, uncached, write-through -- almost done) and noticed that the writethrough code _is already there_ ! currently dsp_shm_init() allocates write-through / read cached memoryI'm not sure too but it can be done one way or another. I don't think it's worth delaying the release though, I was just bringing this up for API considerations.TBH, I have no idea how to implement this [write-through mem. type], though. The DMA kernel API does not support this, yet, as far as I can see (looked at arch/arm/include/asm/dma-mapping.h and include/linux/dma-attrs.h). I presume this would require a kernel modification ?
If that's true it might be a good idea to add this before releasing a new firmware..?
hmm I just discovered some assembly functions in the cmemk.c source: v7_dma_inv_range, v7_dma_clean_range, v7_dma_flush_range. maybe these can be borrowed (haven't tested them, yet).For large buffers (above L2 cache size) flush_cache_all() is actually better because otherwise the code has to walk the whole address range in cacheline steps issuing slow cache control instructions. But I agree this should be looked at eventually for small buffers at least.
not reliably, sometimes the build fails and claims that the "pthread_yield" symbol suddenly cannot be resolved.That won't work because it puts the soname into ELF headers and the OE build system is checking that. Doesn't doing "-soname,libc64.so.1" and "ln -s libc64.so.1 libc64.so" work?
ok then. /usr/include/c64_tools/ sounds fine to me. This folder should include libc64/ and include/ subfolders (yea, not ideal, but this way the paths are exactly the same like when the c64_tools.tar.gz is used)The -dev package is automatically built and published, if we don't fix it now it will just make a broken -dev package. You just need to tell me include location (I suggest /usr/include/c64_tools/) and I'll take care of the rest.
Weird, it doesn't do that for me.That won't work because it puts the soname into ELF headers and the OE build system is checking that. Doesn't doing "-soname,libc64.so.1" and "ln -s libc64.so.1 libc64.so" work?
not reliably, sometimes the build fails and claims that the "pthread_yield" symbol suddenly cannot be resolved.
strange. I have to try if there is a newer version of the toolchain.
Maybe I can also add a workaround that checks whether the CS 2011.09 tools are used and use the workaround to build an .so with the wrong version info (only for dev. on my system).
libc64/ ? What for? I don't see any headers there. libc64.a will go to /usr/lib/ in the -dev package.The -dev package is automatically built and published, if we don't fix it now it will just make a broken -dev package. You just need to tell me include location (I suggest /usr/include/c64_tools/) and I'll take care of the rest.
ok then. /usr/include/c64_tools/ sounds fine to me. This folder should include libc64/ and include/ subfolders (yea, not ideal, but this way the paths are exactly the same like when the c64_tools.tar.gz is used)