Assembler Woes

Gerry Keely · 2009-07-23 11:41

Hi
Attached is a contrived program for flashing a led on and off. I know there are much simpler ways of doing this but purpose of the exercise was to learn something about assembly language proramming and passing parameters.
The program works, but only comes alive after a few minutes. I suspect it is something to do with the system counter but not too sure. Any comments would be welcomed.
Note I'm using a DLP-Prop board which has a 6MHz clock.

Gerry

{{ Flash a led on pin 27 using 3 cogs , main + two assembly routines}}
CON
_clkmode = xtal1 + pll16x
_xinfreq = 6_000_000
VAR
 long cmd0
 long dly0 
 long cmd1
 long dly1
PUB Main
 cmd0 := 0
 cmd1 := 0
 dira[noparse][[/noparse]27] := 1
 cognew(@ASM0, @cmd0)           'start a new cog                                                
 cognew(@ASM1, @cmd1)           'and again
  
repeat
  Off(1,48_000_000)            'switch off
  On(2,48_000_000)             'switch on
 
PUB Off(val,del) 
 cmd0 := val
 dly0 := del
 repeat while cmd0 <> 0        'wait for asm0 to complete
 outa[noparse][[/noparse]27] := 0
 
PUB On(val,del)
 cmd1 := val
 dly1 := del
 repeat while cmd1 <> 0        'wait for asm1 to complete
 outa[noparse][[/noparse]27] := 1
 
DAT
org 0
ASM0
                mov tmpCmmd0, par
                mov tmpDelay0 , par
                add tmpDelay0, #4
WaitForCmd0     rdlong Delay0, tmpDelay0  
                rdlong Cmmd0, tmpCmmd0
                tjz Cmmd0, #WaitForCmd0
CheckCmd0       cmp Cmmd0, #1 wz
         if_nz  jmp #WaitForCmd0
                 mov Time0, cnt 'Get system counter
                'add Time0 ,10
                add Time0, Delay0 'Add delay
                waitcnt Time0, Delay0
                wrLong zero0, tmpCmmd0
                jmp #WaitForCmd0
              
              
tmpCmmd0      long 0
tmpDelay0    long 0              
Cmmd0        long 0
zero0        long 0
Delay0       long 0
Time0        res  1
FIT      496
 
DAT 
org 0
ASM1
              mov tmpCmmd1, par                  'get command address
              mov tmpDelay1, par
              add tmpDelay1, #4                 'get delay address
WaitForCmd1   rdlong Delay1, tmpDelay1          'read the command
              rdlong Cmmd1, tmpCmmd1             'read the delay
              tjz Cmmd1, #WaitForCmd1             
CheckCmd1     cmp Cmmd1, #2 wz
        if_nz jmp #WaitForCmd1             
              mov Time1, cnt                    'Get system counter
              add Time1, Delay1                 'Add delay
              waitcnt Time1, Delay1
              wrLong zero1, tmpCmmd1             'clear the command
              jmp #WaitForCmd1                  'repeat the dose
' ENDASM
tmpCmmd1      long 0
tmpdelay1     long 0
Cmmd1         long 0
zero1         long 0
Delay1        long 0
Time1         res 1
FIT 496

ericball · 2009-07-23 12:36

You are setting Delay1 after COGNEW. COGINIT/COGNEW copies the code from HUB RAM to COG RAM. So once COGNEW has executed, updating the HUB RAM data doesn't change the COG RAM value. Thus, Delay0 is 0, which then means WAITCNT is waiting for 2^32 clock cycles.

Note: COGINIT/COGNEW are not instantaneous, it takes ~8000 cycles to copy the data from HUB to COG. So it is possible to modify the HUB data after COGNEW and have it show up in the COG.

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
Composite NTSC sprite driver: Forum
NTSC & PAL driver templates: ObEx Forum
OnePinTVText driver: ObEx Forum

lonesock · 2009-07-23 12:45

Gerry Keely said...
...
The program works, but only comes alive after a few minutes. I suspect it is something to do with the system counter but not too sure. Any comments would be welcomed.

Right you are! You are not setting either Delay0 or Delay1 to a > 8 value before calling cognew. Thus the first time each assembly cog runs it will properly read the vale of DelayX, add cnt, then wait until the system counter reaches that exact value (cnt + DelayX). Since DelayX is 0, and we already passed by the old value of cnt, the cog will wait until the counter has looped through its entire 32 bit range.

If you don't mind some PASM pointers:

You use the line "mov tmpCmmd0, par". There are 2 ways to get around this. One is to simply use par instead of tmpCmmd0, as that register will remain the same value, but of course you lose readability that way. The second option is to set the value of tmpCmmd0 in Spin _before_ calling cognew. What happens is that tmpCmmd0 is a perfectly valid long variable declared in Hub RAM. So you can read its value or set its value from Spin...but it will only change the value of tmpCmmd0 in the Hub RAM, not in cog RAM. When you call cognew, it takes a starting address in Hub RAM and loads 496 consecutive longs from there into the cog which is about to be started. If you happen to have already updated the data that is sitting in Hub RAM before you call cognew, then the cog gets copied in the value that you already modified. So you could just say something like this:

 cmd0 := 0
 tmpCmmd0 := @cmd0
 tmpDelay0 := @dly0
 cmd1 := 0
 tmpCmmd1 := @cmd1
 tmpDelay1 := @dly1
 dira[noparse][[/noparse]27] := 1
 cognew(@ASM0, @cmd0)           'start a new cog                                               
 cognew(@ASM1, @cmd1)           'and again

I see you using this code:

rdlong Cmmd0, tmpCmmd0
tjz Cmmd0, #WaitForCmd0

This works just fine, but note that you could do:

        rdlong Cmmd0, tmpCmmd0 wz
if_z    jmp #WaitForCmd0

This sets the zero flag iff Cmmd0 is zero, then if that flag is set, the jump occurs on the subsequent line. This is faster in the case that Cmmd0 is non-zero, because tjz (and all test & jump commands) take 4 clocks if they jump, and 8 clocks if the don't. The if_z modifier, on the other hand, will take only 4 clocks either way.

When you check your commands, there are 2 things you could do for readability. Instead of using #1 as the command you check against, you could either declare a constant in a CON block and use that (as long as it's < 512), or you could use a character which is a mnemonic for the action to be performed. For example, combining both suggestions (which is a bit silly [noparse][[/noparse]8^):

CON
  CMD_FLASH = "f"
...
DAT
...
  cmp Cmmd0, #CMD_FLASH wz
...

Your code is looking great, though! Please don't take these tiny suggestions as criticism.

Jonathan

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
lonesock
Piranha are people too.

Gerry Keely · 2009-07-23 12:47

Ericball

Thank you for your quick response, Initalised dly0 and dly1 and that solved the problem.

Regards

A Grateful Gerry

Nick Mueller · 2009-07-23 12:50

> When you check your commands, there are 2 things you could do for readability. Instead of using #1 as the command you check against, you could either declare a constant in a CON block and use that (as long as it's < 512), or you could use a character which is a mnemonic for the action to be performed.

I prefer using enums for that. YMMV.

Nick

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
Never use force, just go for a bigger hammer!

The DIY Digital-Readout for mills, lathes etc.:
YADRO

lonesock · 2009-07-23 12:55

Nick Mueller said...
> When you check your commands, there are 2 things you could do for readability. Instead of using #1 as the command you check against, you could either declare a constant in a CON block and use that (as long as it's < 512), or you could use a character which is a mnemonic for the action to be performed.

I prefer using enums for that. YMMV.

Nick

Hi, Nick.

Good point, for multiple commands especially.

Jonathan

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
lonesock
Piranha are people too.

Gerry Keely · 2009-07-23 12:57

Jonathan

Thank you for your response and pointers, much appreciated and no I don't take what you say as any kind of critism.All help much appreciated.

Gerry

Post Edited (Gerry Keely) : 7/23/2009 1:16:01 PM GMT

Nick Mueller · 2009-07-23 14:30

> Good point, for multiple commands especially.

I look at this communication-register (or byte or whatever length it is) a bit different.
If it would be strictly a command-register, only the master could write to it.
So I call it communication register. And both can read and write. But this is just a naming convention.

So that whole transaction can have different states like maybe:
SlaveIdle, SlaveBusy, ExecCmdThis, ExecCmdThat, SlaveError ...

With enums, you can also play tricks what bit to set (see manual for enums). But then you'd have to pay atention not to get overlaps.

Anyhow, this increases readability and is much easier to maintain should there be a need to add new states or remove unnecessary ones.

Just a little excursus on writing better code ("literate programming").

Nick

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
Never use force, just go for a bigger hammer!

The DIY Digital-Readout for mills, lathes etc.:
YADRO

Assembler Woes

Comments