[announce] c64_tools (DSP loader and IPC)

[notaz]

proper defaults according TI are (MPU/DSP) 500/360 for OMAP3 and 600/520 for DM3730.

hmm, I was not aware that a stock "Ghz" Pandora already used overclocking, good to know. On the other hand, the overclocking probably only becomes an issue when the system is under constant load (vs. spending most of the time in deep sleep) ? Or is it already the increased voltage that shortens the life span ?

Well it's "TI supported" overclock, which means all chips are guaranteed to work at 1GHz. Other manufacturer probably would just call it normal operation, TI seem to be pretty conservative.


And yeah I wouldn't think that "life timer" should be ticking while in deep sleep. They specify >100k hours at 600Mhz and 75k hours at 1GHz (unless the ambient temperature is 90C, then you only get 25K hours).

I would suggest that you do the same with the DSP clock -- determine the minimum OPP required for the desired GPP and DSP clocks and set that one.


You do not have to know whether the DSP is currently powered on/running, you can write to CM_CLKSEL1_PLL_IVA at any time.

Yeah changing clock is not a problem. What I'm concerned with is that high values of dsp_mhz_max will needlessly deny low OPPs even while DSP is not running, that's why I think the clock code should know about DSP's state. Maybe I could add 2 functions, which would be called before starting the DSP and after stopping it?

 

[Linux-SWAT]

I integrated c64 tools and the newest kernel into Slackware and i was able to run the demo pnd ^^.

I would like to ask for a minor change in /usr/pandora/scripts/op_dsp_c64.sh

Line 92

-modprobe c64 $KMODARGS

+/sbin/modprobe c64 $KMODARGS

 

[sebt3]

why dont you force /sbin to be at the start of the PATH env vars of your root user ?

 

[Linux-SWAT]

It is, it's sudo who likes absolute pathes.

Also, the script mixes relative and absolutes ones so...

 

[bsp]

Yeah changing clock is not a problem. What I'm concerned with is that high values of dsp_mhz_max will needlessly deny low OPPs even while DSP is not running, that's why I think the clock code should know about DSP's state. Maybe I could add 2 functions, which would be called before starting the DSP and after stopping it?


You do not have to know whether the DSP is currently powered on/running, you can write to CM_CLKSEL1_PLL_IVA at any time.

Hmm ok, in this case adding a notification function would be a proper solution, indeed.


Here's an update c64_tools that implements this feature: http://tkscript.de/c64_tools/c64_tools-29Oct2013_pwrnotify.tar.gz

USE_DSP_POWER_NOTIFY must be defined in kmod.h (it is by default)


Before the DSP is powered on, c64_tools now calls dsp_power_notify(1), and after the DSP is powered off, dsp_power_notify(0) is called.

I would like to ask for a minor change in /usr/pandora/scripts/op_dsp_c64.sh

this should probably be posted in this http://boards.openpandora.org/topic/12202-superzaxxon-incremental-update-changelog/ thread. The c64_tools go64.sh script does use /sbin/insmod and /sbin/rmmod :]

however, cool thing you got it running in Slackware!

p.s.: @notaz: You can use the attached DSP binaries to check whether the DSP clockrate is correct. They are compiled w/o optimizations (to leave the NOPs intact). To execute the test, run "./c64_dsprite -nops".

dsprite_noptest-29Oct2013.tar.gz

 

[notaz]

Thanks, I'll try to cook an update for the weekend with this, if all goes well.

 

[notaz]

OK I mostly have OPP code working. It would be nice if you fixed those cache flushes, as for every c64.ko update I need to issue full kernel update because of current packaging limitations in the OS, so we could avoid one large update with this.

Also another cacheability option would be useful - noncached buffered, which is set with pgprot_writecombine(). With that we would have fast writes to shared mem (from GPP) but slow, uncached reads which don't need invalidation. dsp_cache_wb() would just do asm("dsb") for such memory. You could name it DSP_CACHE_W I guess.

 

[LTStone]

Is this DSP stuff just for the 1gig Pandoras or the rebirth and cc's as well?

 

[Jey123456]

all the pandora sport a dsp, so yes, its for the cc as well

 

[bsp]

OK I mostly have OPP code working. It would be nice if you fixed those cache flushes, as for every c64.ko update I need to issue full kernel update because of current packaging limitations in the OS, so we could avoid one large update with this.

Also another cacheability option would be useful - noncached buffered, which is set with pgprot_writecombine(). With that we would have fast writes to shared mem (from GPP) but slow, uncached reads which don't need invalidation. dsp_cache_wb() would just do asm("dsb") for such memory. You could name it DSP_CACHE_W I guess.

ok nice.

Regarding the cache functions: I tried passing the userspace virtual addresses to dmac_flush_range() but that did not work ("unable to handle kernel paging request") *BUT* this was my mistake.

Before fixing that, I grabbed the v7_dma*() assembly functions from cmemk.c (TI) and prefixed them with "c64_".

These fxns look like hacked versions of arch/arm/mm/cache_v7.S.

I have not investigated how exactly they differ.

I remember that when I used the "stock" dmac_flush_range() (which effectively calls the regular v7_dma_flush_range() function) some weeks ago, I got occasional system freezes.

There are a couple of new c64_tc testcases now and the (hacked/patched/whatever) cache functions from TI seem to work fine. We should probably stick with them for now.

And you're right, I forgot the DSP_CACHE_W type (already tested pgprot_writecombine() a while ago, though), it's implemented now.

Here's today's update: (all testcases seem to work ok, maybe we can already use this for the next FW update):

+ add DSP_CACHE_W (write combine) cache type


+ add new c64_tc testcase TC_SHM_CACHE_W


+ add v7_dma_*() asm utility functions from cmemk


   (note) these look like hacked Linux cache-v7.S functions


+ change kmod/dev.c:fops__ioctl():[C64_IOCTL_CACHE] to use c64_v7_dma_map_area() for


   CACHE_AC_INV, CACHE_AC_WB, and CACHE_AC_WBINV actions


+ add DEMO_CHECKSUM_CMD_RAND_WRITE command to demo_checksum DSP component


   (writes a configurable number of random integers to the specified buffer)


+ add new c64_tc testcase TC_SHM_CACHE_NONE_DSP


+ add new c64_tc testcase TC_SHM_CACHE_RW_DSP


+ add new c64_tc testcase TC_SHM_CACHE_R_DSP


+ add new c64_tc testcase TC_SHM_CACHE_W_DSP


+ add DEMO_CHECKSUM_CMD_RAND_READ command to demo_checksum DSP component


  (reads a configurable number of random integers from the specified buffer and builds a checksum)


+ add new c64_tc testcase TC_SHM_CACHE_NONE_GPP


+ add new c64_tc testcase TC_SHM_CACHE_RW_GPP


+ add new c64_tc testcase TC_SHM_CACHE_R_GPP


+ add new c64_tc testcase TC_SHM_CACHE_W_GPP

+ updated DSP_CACHE_* docs in dsp_common.h (added some comments)


+ release http://tkscript.de/c64_tools/c64_tools-01Nov2013_snapshot.tar.gz

 

[notaz]

I've done some overclocking tests and it seems DPLL2 is failing to lock at 1188MHz or higher on DM3730. At 1187MHz the nop test seems to run fine and completes in 727ms, fractal demo is running fine too (it seems nop test doesn't take DSP wakeup latency into account, so if you estimate clock from that, the results are lower). Maybe the PLL problem could be overcome with different dividers, but for a start 1187MHz (+ ~48%) max overclock should be enough I think.

I've tested CC unit too and was able to reach 780MHz (+ ~81%) with those tests. It worked for a few times at 800MHz, but then locked up the system.

As we know on ARM NEON stress test can crash the system at much lower clocks than when running just regular ARM code, so it's most likely the same for DSP, the nop test and fractal demo are most likely only using 1/8 of the pipeline most of the time, so actual useful overclock will be lower.

 

[bsp]

I for one will probably not overclock the DSP that high..at least not for long. Good thing that it is powered off after an app quits (resp. when the last app using the DSP quits) so this will limit the potential damage in case someone forgets to downclock it again.

it seems nop test doesn't take DSP wakeup latency into account, so if you estimate clock from that, the results are lower

No it does not take that into account because the DSP is already awake when the nop test starts Therefore, the results should be rather exact, except for the DSP-side loop overhead since only 100 nops are executed per loop iteration.

As we know on ARM NEON stress test can crash the system at much lower clocks, so it's most likely the same for DSP, the nop test and fractal demo are most likely only using 1/8 of the pipeline most of the time, so actual useful overclock will be lower.

we need (more) real world applications, of course, then we'll know the true limits.

 

[rohezal]

Would be great for an app to set a dsp clock for it. Maybe something like this:

if(__pandora__model() == 1) //ghz

{

  set_dsp_clock(1100);

}

else //cc or rebirth

{

 set_dsp_clock(700);

}

 

[bsp]

In general, apps really should not do (hardcoded) things like that (IMHO).

The OS/FW will configure the default "sane" GPP/DSP clock settings (different for OMAP3530 and DM3730) and make sure that the maximum limit is not exceeded by any userspace applications (the DSP clock rate will be configurable just like the GPP clock rate)

Overclocking should always be the user's choice.

 

[rohezal]

We have several apps which set the gpp speed to 800 when starting, asking the user first time the app is started. Black Shades does it and it is realy needed.

Of course the OS should catch invalid values to prevent a disaster. Maybe make a script and apps can call this script?

Like

system("/bin/bash /opt/pandora/set_maximum_safe_dsp_clock.sh");

or

system("/bin/bash /opt/pandora/set_800_dsp_clock.sh");

or

system("/bin/bash /opt/pandora/120_percent_dsp_clock.sh");

 

[notaz]

I have one more (hopefully last) feature request: dsp_virt_to_phys() to work with user-allocated hugetlb memory (2MB pages). The code should look like this (in kernelspace):

#include <linux/mm.h>
 
u32 virt_to_phys(u32 virt_addr)
{
  pgd_t *pgd = pgd_offset(current->mm, virt_addr);
  u32 l1_entry = *(u32 *)pgd;
  u32 phys = 0;

  if ((l1_entry & 3) == 2) { /* section a.k.a. huge page */
    if (l1_entry & (1<<18)) /* supersection */
      phys = (l1_entry & 0xff000000) | (virt_addr & 0x00ffffff);
    else
      phys = (l1_entry & 0xfff00000) | (virt_addr & 0x000fffff);
  }
  return phys;
}

I could put it somewhere else in the kernel, but there is simply no appropriate place, and the only use of this is to allow DSP to access that memory.

I was also thinking about allowing to get phys address of normal 4kB pages, but decided it's a bad idea because those normal pages can be migrated or swapped anytime, so if DSP access them, memory corruption would happen. Hopefully that can't happen with huge pages.

Testcase for hugetlb memory is here: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/tools/testing/selftests/vm/map_hugetlb.c

Explanation of the whole thing: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/vm/hugetlbpage.txt

On pandora, before using huge pages, you have to run:

sudo /usr/pandora/scripts/op_hugetlb.sh <megabytes>

 

[bsp]

hmm ok, so if I understood this right there's a "special" kernel memory area reserved for those "huge pages" and you want a way to retrieve the physical address for such memory blocks ? (..as a performance improvement to reduce TLB lookups..)

This kind of "huge page" memory must be allocated via mmap() in userspace, right ? (MAP_HUGETLB flag)

Per design, the dsp_virt_to_phys() and dsp_phys_to_virt() functions operate solely in userspace (simple add+sub ops), i.e. no kernel interaction necessary.

What do you think about adding dsp_hugetlb_alloc(), dsp_hugetlb_free() ?

Upon allocation, the alloc() fxn would retrieve the physical address via an ioctl.

dsp_virt_to_phys() (and dsp_phys_to_virt()) could then figure out that special steps need to be taken to handle that kind of memory, i.e. scan a list of previously allocated hugetlb regions if the given address is not located within the "usual" mem areas.

Agreed?

p.s.: If this were a security sensitive device, I would have some concerns since what this effectively means is that the userspace tells the kernelspace what virtual memory areas are safe to be mapped to physical addresses. Then again, this is a video game console but your requests this evening were keeping me from playing through the Carrier Air Wing arcade (revisited Area 88 yesterday..what a game..)

 

[notaz]

This kind of "huge page" memory must be allocated via mmap() in userspace, right ? (MAP_HUGETLB flag)

Right.

What do you think about adding dsp_hugetlb_alloc(), dsp_hugetlb_free() ?

Should be good as long as you allow some arguments through your calls to mmap (need at least to be able to supply my virtual address, MAP_FIXED flag, permissions and size of course).

 

[bsp]

the idea is that the API call will return a dsp_mem_region_t which returns to you the virtual and physical addresses of the "huge" mem area, no need to pass any additional args (permissions will be read/write, like the other mmaps, an ARM restriction after all, AFAIK (i.e. if you can read a page, you can write to it, correct me if I'm wrong))

 

[bsp]

On second thought, I think I'll just add a dsp_hugetlb_virt_to_phys() API function (using your (notaz') code posted above).

The 'proper' solution would be to allocate the huge pages in the kernel, not in userspace, but to be honest I'm too lazy right now to investigate how exactly this is done and code all the 'infrastructure' needed for this.

I'll post an update later today (in a couple of hours).