Can SPI in Simple Libraries be speeded up?

jazzed

twm47099 wrote: »

I was thinking about declaring the pins in the start function, but I wasn't sure if some SPI devices didn't use different I/O pins for different channels or something.

The start function should set the device pins.

That way the spia_t *spia variable contains all information for dealing with a device. You only have to assign the pins or the character of how the device is used once in your program. Having to add it to every single call causes bugs. If you want another device with different pins just start a different device say spia_t *spia2.

Andy Lindsay (Parallax)

The final version might actually have both bus and device structures to minimize repeating info in function calls. For now though, it's probably best to leave it as-is until verified to correctly exchange into with the PASM Code.

twm47099

Well things could have been simple, but they weren't.

I ran (with terminal) the code with the activity board hooked up to pixy. As originally written, nothing showed up in the terminal screen.

I checked pixy and it was set for SPI transfer and was reading colors.
I ran the spin demo I had modified to read pixy in loops (described in my post above). It worked showing a stream of valid data.

I modified main() as shown below (made it one loop that would print out each read.) I expected to miss some values, but I wanted to see if anything was being read.

The result was the following printing to the terminal:

i = 1
i =1 color = 0
i = 2
nothing else.

So it did one read, went to the begining of the while loop, but never came out of the 2nd call to the spipasm_SHIFTIN function.

Unfortunately, the color = 0 result doesn't really help, since pixy sends a lot of zeros to fill in undefined blocks needed to fill up a frame, it could be a valid value or not.

Then I saw that the spin program set Flag := 1 in SHIFTIN before calling setcommand. I had forgotten that in my program. Since spipasm_SHIFTIN tests for Flag = 0 to return a value, it made sense that it was necessary. But when I added device->Flag = 1; to spipasm_SHIFTIN just before the call to spipasm_setcommand, all that was printed to the terminal was i=1.

I'm not sure how to proceed.

Unfortunately, I also won't be able to do anything on this for the most of today.

Tom

int main()                                    // Main function {   
 // Put demo code here
                   //  spia = spipasm_start(ClockDelay, ClockState)
   spia = spipasm_start(15, 0);    // Launch, get handles
   int i = 1;
   while(i<20)    //  fill pixydata array with spi bytes
   {
                   //  spipasm_shiftin(Dpin, Cpin, Mode, Bits)
 printi("i = %d\n", i );
     pixydata[i] = spipasm_SHIFTIN(spia, 0, 2, 0, 8);  // Mode 0 = MSBPRE
 //    i++;
 //  }
 //  i = 1;
 //  while(i<20)    //  write pixydata array
 //  {
     printi(" i = %d   color = %d\n", i, pixydata[i]);
     i++;
     }
 spipasm_stop(spia);
 }
 

Andy Lindsay (Parallax)

The bus structure would have the data and clock pin numbers, speed, and clock resting state. Each device structure might store enable pin, M/LSB PRE/POST for out and in. The user code will then declare bus and devic(s) and then send data with something like spi_out(bus, device, &dataAddress), but as with some of the other ideas, it's further down the development path, and maybe only after some adjustments to the ASM code.

To get your display running faster before
before modifying the ASM, we may be able to launch all this into another cog and use fcache to feed the ASM code more rapidly. fcache is away of copying function code that is compact to a cog's RAM. This can often boost execution speed by 10x by taking most of the main RAM access out of the loop. Not necessarily a final solution, but might be interesting to try later.

Andy Lindsay (Parallax)

Yeah, if you have a 74595 shift register, the first test could be checking if a single shiftout works, then try 2 in a row. Then switch to shiftin withe a 74165.

Also post your code. I may not have much more time today or this weekend either, but someone else might spot the bug in the meantime.

This is all really good progress BTW.

twm47099

Here's the code that doesn't work. I'd appreciate anyone with familiarity with the Propeller Tool Library object SPI_Asm taking a look and seeing if they can help.

Thanks
Tom

/* spipasm - Use pasm from SPI_Asm.spin propeller tool library object.
 T Montemarano (with a lot of help)

  NOTE save spin file SPI_Asm.spin as SPIAsm.spin and put in same folder as this code
 Use show project manager and project open tab to make SPIAsm.spin part of the project.
  
 This version does NOT run correctly.  It does not return from the call to spipasm_SHIFTIN.

  The SPI device called in main() is a Pixy CMUCAM5.  It does not required a start command 
    since it always transmits data at 1usec. 
 It should be possible to change main to use other spi devices 
   like the DS1620 used in the spi spin demo.

 This version uses the same calls as the SPI_Asm.spin library object.

 */
 #include "simpletools.h"                      // Library includes
  
 typedef struct spipasm_struct               // Structure contains varaibles
 {                                             // for cogs to share + cog & stack
   volatile int command;                       // Shared
   volatile int ClockDelay;            
   volatile int ClockState;
   int Dpin;
   int Cpin;
   volatile int Mode;
   volatile int Bits;
   volatile int Value;
   volatile int Flag;
   int cog;                                    // Remembers cog for spiasm_stop 
 } spia_t;                                    // Data type for struct

 int pixydata[200];

 // function prototypes
  spia_t *spipasm_start(int CLKDLY, 
                          int CLKST);
  
 void spipasm_stop(spia_t *device);

 void spipasm_SHIFTOUT(spia_t *device,
                     int DQ, int CLK, int MD, int BIT, int VLU);
 
 int spipasm_SHIFTIN(spia_t *device,int DQ, int CLK, int MD, int BIT);
 
 void spipasm_setcommand(spia_t *device, int cmd, int *argptr);

 /*
   libblinkpasm.c
   Test harness for the spipasm library. 
 */
 // #include "spipasm.h"

 spia_t *spia; 
  
 int main()                                    // Main function
 {   
 // Put demo code here
                   //  spia = spipasm_start(ClockDelay, ClockState)
   spia = spipasm_start(15, 0);    // Launch, get handles
   int i = 1;
   while(i<198)    //  fill pixydata array with spi bytes
   {
                    // Pixy SPI interface continuously transmits data (no starting or stopping it)
                    // Pixy data out rate is 1usec 
                    // Pixy SPI data out -> Prop P0, Pixy Clock -> Prop P2
                   //  spipasm_shiftin(Dpin, Cpin, Mode, Bits)
     pixydata[i] = spipasm_SHIFTIN(spia, 0, 2, 0, 8);  // Mode 0 = MSBPRE
 /*  comment block arred for debug -- make read data and print data into 1 loop
     i++;
   }
   i = 1;
   while(i<198)    //  write pixydata array
   {
 */  // end of debug comment block 
 
   printi("color = %d\n", pixydata[i]);
   i++;
   }
 spipasm_stop(spia);
 }

 // These are the actual functions

 //  ****************************************
 spia_t *spipasm_start(int CLKDLY, 
                          int CLKST)
 {
   spia_t *device;                                    // Declare spipasm pointer
   device = (void *) malloc(sizeof(spia_t));         // Allocate memory for it  
 
   device->ClockDelay = CLKDLY;
   device->ClockState = CLKST;
   if(device->cog == 0)
   {
     device->command = 0;
     extern int binary_SPIAsm_dat_start[];
     device->cog = 1 + cognew((void*)binary_SPIAsm_dat_start, (void*)device);
   }
   return device;                                       // Return pointer to structure
 }

 //  ************************************
 void spipasm_stop(spia_t *device)
 {
   if(device->cog > 0)                                  // If cog running
   {
     cogstop(device->cog - 1);                          // Shut down cog
     device->cog = 0;                                   // Clear cog varaible
     free(device);                                      // Release allocated memory
   }  
 }

 //  ************************************
 void spipasm_SHIFTOUT(spia_t *device, int DQ, int CLK, int MD, int BIT, int VLU)
 {
   device->Dpin = DQ;
   device->Cpin = CLK;
   device->Mode = MD;
   device->Bits = BIT;
   device->Value = VLU;
   spipasm_setcommand(spia,1, &device->Dpin);
 } 

 //  ************************************
 int spipasm_SHIFTIN(spia_t *device,
                     int DQ, int CLK, int MD, int BIT)
 {
   device->Dpin = DQ;
   device->Cpin = CLK;
   device->Mode = MD;
   device->Bits = BIT;
   device->Flag = 1;           // Pasm code should set Flag to 0 when read is complete
 
   spipasm_setcommand(spia, 2, &device->Dpin); 
   while(device->Flag);
   return device->Value;
 }

 //  ************************************
   void spipasm_setcommand(spia_t *device, int cmd, int *argptr)
 {
   device->command = (int) argptr | (cmd << 16);
   while(device->command);  // wait -- pasm acknowledges receipt of command (reset command to 0)
 }  

Andy Lindsay (Parallax)

This might be a situation where spin methods automatically set local variables to zero on entry. I don't remember if that's actually the way it works, but if it is, something may need to be set to zero for the ASM to deal with the second call.

kuroneko

twm47099 wrote: »

Here's the code that doesn't work. I'd appreciate anyone with familiarity with the Propeller Tool Library object SPI_Asm taking a look and seeing if they can help.

This is just based on the C code you posted. The original SPIN code injects ClockDelay and ClockState values into the PASM image before launching it. I don't see anything equivalent in your code which would mean that the original values of 0/0 stay embedded in the image. While this isn't a problem for state it certainly is for delay.

ClkDly
              mov       t6,     ClockDelay
ClkPause      djnz      t6,     #ClkPause                               
ClkDly_ret    ret

IOW a single call to ClkDly will take a bit over 200 seconds @80MHz and you get two calls per clock cycle (i.e. per bit). I think you get the picture here.

There is also an as yet [post=1061378]unresolved bug[/post] in that object.

twm47099

kuroneko wrote: »

This is just based on the C code you posted. The original SPIN code injects ClockDelay and ClockState values into the PASM image before launching it. I don't see anything equivalent in your code which would mean that the original values of 0/0 stay embedded in the image. While this isn't a problem for state it certainly is for delay.

ClkDly
              mov       t6,     ClockDelay
ClkPause      djnz      t6,     #ClkPause                               
ClkDly_ret    ret

IOW a single call to ClkDly will take a bit over 200 seconds @80MHz and you get two calls per clock cycle (i.e. per bit). I think you get the picture here.

Does that also mean that the structure declaration in post 37 is wrong with ClockDelay and ClockState between volatile int command and int Dpin?
Should it be:

typedef struct spipasm_struct               // Structure contains varaibles  
{                                             // for cogs to share + cog & stack   
 volatile int command;                       // Shared 
// volatile int ClockDelay;             
//   volatile int ClockState;    
int Dpin;    
int Cpin;    
volatile int Mode;    
volatile int Bits;    
volatile int Value;    
volatile int Flag;    
int cog;                                    // Remembers cog for spiasm_stop   
} spia_t;                                    // Data type for struct

Is there a way in C to know where to store the values for ClockDelay and ClockState in the pasm part of the program?
To test the program, along with changing the structure and removing the references to ClockDelay and ClockState in the start function, can I just change the pasm in SPIAsm.spin by defining ClockDelay in the Assembly Variables to be:

ClockDelay              long    $F    

Thanks
Tom

kuroneko

twm47099 wrote: »

Does that also mean that the structure declaration in post 37 is wrong with ClockDelay and ClockState between volatile int command and int Dpin?

Not sure about wrong, but those two entries certainly don't have the intended effect. IOW you can remove them.

twm47099 wrote: »

Is there a way in C to know where to store the values for ClockDelay and ClockState in the pasm part of the program?

I believe the current official approach is to place an injection table at the beginning of the PASM image and give C access to it (with an associated struct).

twm47099 wrote: »

To test the program, along with changing the structure and removing the references to ClockDelay and ClockState in the start function, can I just change the pasm in SPIAsm.spin by defining ClockDelay in the Assembly Variables to be:

ClockDelay              long    $F    

Provided there isn't anything else wrong this should work as a temporary solution.

If that still doesn't work I'd suggest a different PASM image which simply verifies the parameters it can see and go from there step by step.

twm47099

I tried modifying the program by defining ClockDelay in the assembly language variables to be $F, and deleted the declarations and any reference to ClockDelay and ClockState in the C program.

The program (listing below) worked and gave 8 to 12 Blocks of Pixy Data per Frame. That is a significant increase from the SimpleLibrary version of SPI code (no pasm) which gave 1 Block per Frame, and a good increase over the Spin Code with pasm which gave about 5+ Blocks.

Now the difficulty with making a general library function is to see if there is a way to get the C program to load pasm variables ClockDelay and ClockState.

Here's the version that works:

Tom

/* spipasm 2 - 

 Use pasm from SPI_Asm.spin propeller tool library object.
  T Montemarano (with a lot of help) 

   NOTES:
 Open spin file SPI_Asm, In the Assembly Language Variables,
 1.  Change the values of ClockDelay from 0 to your desired delay.  For Pixy a value of $F works.
 2.  Changethe value of ClockState to 1 if needed.  For Pixy 0 works.  
 3.  Save the spin file as SPIAsm.spin and put in same folder as this code.
 4.  Use show project manager and project open tab to make SPIAsm.spin part of the project.
   
  This version does run correctly.  

   The SPI device called in main() is a Pixy CMUCAM5.  It does not required a start command 
     since it always transmits data at 1usec. 
  It should be possible to change main to use other spi devices 
    like the DS1620 used in the spi spin demo. 

  This version uses the same calls as the SPI_Asm.spin library object.
  
  */
  
 #include "simpletools.h"                      // Library includes
  
 typedef struct spipasm_struct               // Structure contains varaibles
 {                                             // for cogs to share + cog & stack
   volatile int command;                       // Shared
   int Dpin;
   int Cpin;
   volatile int Mode;
   volatile int Bits;
   volatile int Value;
   volatile int Flag;
   int cog;                                    // Remembers cog for spiasm_stop 
 } spia_t;                                    // Data type for struct
  
 int pixydata[200];
  
 // function prototypes
  
 spia_t *spipasm_start();
  
 void spipasm_stop(spia_t *device);
  
 void spipasm_SHIFTOUT(spia_t *device,
                     int DQ, int CLK, int MD, int BIT, int VLU);
  
 int spipasm_SHIFTIN(spia_t *device,int DQ, int CLK, int MD, int BIT);
  
 void spipasm_setcommand(spia_t *device, int cmd, int *argptr);
  
 /*
   libblinkpasm.c
   Test harness for the spipasm library. 
 */
 // #include "spipasm.h"
  
 spia_t *spia; 
  
 int main()                                    // Main function
 { 
 int px;  
 // Put demo code here
  
   spia = spipasm_start();    // Launch, get handles
   int i = 1;
   while(i<198)    //  fill pixydata array with spi bytes
   {
                     // Pixy SPI interface continuously transmits data (no starting or stopping it)
                     // Pixy data out rate is 1usec 
                     // Pixy SPI data out -> Prop P0, Pixy Clock -> Prop P2
                     //  spipasm_shiftin(Dpin, Cpin, Mode, Bits)
     pixydata[i] = spipasm_SHIFTIN(spia, 0, 2, 0, 8);  // Mode 0 = MSBPRE
     i++;
   }
   i = 1;
   while(i<198)    //  write pixydata array
   {
     printi(" i = %d   color = %x\n", i, pixydata[i]);
     i++;
     }
 spipasm_stop(spia);
 }
  
 // These are the actual functions
  
 //  ****************************************
  spia_t *spipasm_start()
 {
   spia_t *device;                                    // Declare spipasm pointer
   device = (void *) malloc(sizeof(spia_t));         // Allocate memory for it  
 

   if(device->cog == 0)
   {
     device->command = 0;
     extern int binary_SPIAsm_dat_start[];
     device->cog = 1 + cognew((void*)binary_SPIAsm_dat_start, (void*)device);
   }
   return device;                                       // Return pointer to structure
 }
  
 //  ************************************
 void spipasm_stop(spia_t *device)
 {
   if(device->cog > 0)                                  // If cog running
   {
     cogstop(device->cog - 1);                          // Shut down cog
     device->cog = 0;                                   // Clear cog varaible
     free(device);                                      // Release allocated memory
   }  
 }
  
 //  ************************************
 void spipasm_SHIFTOUT(spia_t *device, int DQ, int CLK, int MD, int BIT, int VLU)
 {
   device->Dpin = DQ;
   device->Cpin = CLK;
   device->Mode = MD;
   device->Bits =  BIT;
   device->Value = VLU;
   spipasm_setcommand(spia,1, &device->Dpin);
 } 
  
 //  ************************************
 int spipasm_SHIFTIN(spia_t *device,
                     int DQ, int CLK, int MD, int BIT)
 {
   device->Dpin = DQ;
   device->Cpin = CLK;
   device->Mode = MD;
   device->Bits = BIT;
   device->Flag = 1;
  
   spipasm_setcommand(spia, 2, &device->Dpin);   
   while(device->Flag);  
   return device->Value;
 }
  
 //  ************************************
   void spipasm_setcommand(spia_t *device, int cmd, int *argptr)
 {
   device->command = (int) argptr | (cmd << 16);
   while(device->command);
 }  
 

David Betz

I'm arriving late to this party. Where can I find the SPI_pasm.spin code people are talking about here?

kuroneko

@Tom: Why did you disable setting device->Flag to 1?

@David: PropTool library or OBEX.

David Betz

kuroneko wrote: »

@David: PropTool library or OBEX.

Thanks! Is the idea to make a C interface to this PASM code? Or maybe C++ since it would match Spin better?

twm47099

kuroneko wrote: »

@Tom: Why did you disable setting device->Flag to 1?

@David: PropTool library or OBEX.

@kuroneko: I accidentally tried it commented out, and the program worked. Then I tried tried it uncommented. It also worked. But with device->Flag to 1 the number of Blocks of data received was limited to 8, with it commented out, the number was usually 10 or more. I'm not sure why the program works when it is commented out. I couldn't see anything in the asm that would set Flag.

Tom

kuroneko

twm47099 wrote: »

I couldn't see anything in the asm that would set Flag.

Update_SHIFTIN
              mov     t1,             address           ''     Write data back to Arg4
              add     t1,             #16               ''          Arg0 = #0 ; Arg1 = #4 ; Arg2 = #8 ; Arg3 = #12 ; Arg4 = #16
              wrlong  t3,             t1
              add     t1,             #4                ''          Point t1 to Flag ... Arg4 + #4
              [COLOR="#FF0000"]wrlong  zero,           t1[/COLOR]                ''          Clear Flag ... indicates SHIFTIN data is ready
              jmp     #loop                             ''     Go wait for next command

twm47099

David Betz wrote: »

Thanks! Is the idea to make a C interface to this PASM code? Or maybe C++ since it would match Spin better?

@David: The idea was to make a C interface to the PASM that would be made into a SimpleIDE Library. (My immediate goal was to get a fast SPI interface to my CMUCAM5 Pixy color tracker. The current limited code meets that goal, but I'd like to make a more general SPI library function. (This is also a learning experience for me.)

Tom

twm47099

I counted the lines of pasm code in the SPIAsm.spin file. There are 159 longs (636 bytes which agrees with the status given in the prop tool. ClockDelay is defined at long146 and ClockState at 147. Is there any way to use that information in the C program to initialize ClockDelay and Clock State?

Thanks
Tom

David Betz

twm47099 wrote: »

I counted the lines of pasm code in the SPIAsm.spin file. There are 159 longs (636 bytes which agrees with the status given in the prop tool. ClockDelay is defined at long146 and ClockState at 147. Is there any way to use that information in the C program to initialize ClockDelay and Clock State?

Thanks
Tom

I don't understand why you're trying to patch the PASM code. Why not just pass in the ClockDelay and ClockState values as parameters in the structure and have the PASM initialization code do the patching? Or is the idea to use the PASM code without having to make any changes to it?

twm47099

kuroneko wrote: »

Update_SHIFTIN
              mov     t1,             address           ''     Write data back to Arg4
              add     t1,             #16               ''          Arg0 = #0 ; Arg1 = #4 ; Arg2 = #8 ; Arg3 = #12 ; Arg4 = #16
              wrlong  t3,             t1
              add     t1,             #4                ''          Point t1 to Flag ... Arg4 + #4
              [COLOR=#FF0000]wrlong  zero,           t1[/COLOR]                ''          Clear Flag ... indicates SHIFTIN data is ready
              jmp     #loop                             ''     Go wait for next command

That cleared Flag to 0, but did you find something that set it to 1 in the pasm?

I'm wondering if there is some accident of timing that makes it work without Flag being set to 1 in the C program. I should probably put it back.

Tom

kuroneko

twm47099 wrote: »

That cleared Flag to 0, but did you find something that set it to 1 in the pasm?

What for? This is simply handshaking between SPIN and PASM. SPIN sets a non-zero value, then calls the PASM function. Completion of the latter is indicated by clearing Flag which is what the caller (SPIN) is waiting for.

twm47099

David Betz wrote: »

I don't understand why you're trying to patch the PASM code. Why not just pass in the ClockDelay and ClockState values as parameters in the structure and have the PASM initialization code do the patching? Or is the idea to use the PASM code without having to make any changes to it?

That was the idea (use the PASM as a black box). I did try to figure out how the pasm worked, to possibly modify it, but with my level of knowledge of pasm (none), I got lost very quickly.

At the begining of post 24 you can see some of my questions (that I hope to ask in the prop1 forum when I have some basic knowledge of pasm). My experience with assembly language program began and ended with the 8080 processor (35 yrs ago).

Tom

twm47099

kuroneko wrote: »

What for? This is simply handshaking between SPIN and PASM. SPIN sets a non-zero value, then calls the PASM function. Completion of the latter is indicated by clearing Flag which is what the caller (SPIN) is waiting for.

That's what I thought, but I was wondering why the code worked without the C code setting the Flag = 1.

Tom

David Betz

twm47099 wrote: »

That was the idea (use the PASM as a black box). I did try to figure out how the pasm worked, to possibly modify it, but with my level of knowledge of pasm (none), I got lost very quickly.

At the begining of post 24 you can see some of my questions (that I hope to ask in the prop1 forum when I have some basic knowledge of pasm). My experience with assembly language program began and ended with the 8080 processor (35 yrs ago).

Tom

You might want to try again with Propeller PASM. It's really quite easy compared with other assemblers. I think a minor modification to the PASM code could make patching unnecessary.

kuroneko

twm47099 wrote: »

That's what I thought, but I was wondering why the code worked without the C code setting the Flag = 1.

Think about it, if it's 0 to start with then the read function just doesn't wait for PASM to finish. If it is not equal 0 then it waits once, then never again.

twm47099

@Dave: I got lost in the first few lines starting with movd :arg, #arg0 down to mov address, t1.

That snippet of code is supposed to move the 5 arguments set by the spin program into arg0 to arg4. I just couldn't follow it, for example what does add :arg,d0 do (:arg is a label, and d0 is $200)?

I've started reading the PASM section of the Prop manual, and some of the threads for learning pasm. I can understand some of the simpler snippets, but this program seems more advanced.

SPI_Asm.spin may be to complex to make into a SimpleIDE library, using the pasm code as a black box. But that was the goal.

Tom

David Betz

twm47099 wrote: »

@Dave: I got lost in the first few lines starting with movd :arg, #arg0 down to mov address, t1.

That snippet of code is supposed to move the 5 arguments set by the spin program into arg0 to arg4. I just couldn't follow it, for example what does add :arg,d0 do (:arg is a label, and d0 is $200)?

I've started reading the PASM section of the Prop manual, and some of the threads for learning pasm. I can understand some of the simpler snippets, but this program seems more advanced.

SPI_Asm.spin may be to complex to make into a SimpleIDE library, using the pasm code as a black box. But that was the goal.

Tom

Ah, yes. Self-modifying code. That does take a bit of time to get used to. Most other processors left that behind years ago.

twm47099

kuroneko wrote: »

Think about it, if it's 0 to start with then the read function just doesn't wait for PASM to finish. If it is not equal 0 then it waits once, then never again.

That's why I wonder if its a timing accident. If the read was happening fast enough that the 8 bits completed while the C conditional was processing, it might work (until it didn't).

Tom

David Betz

for example what does add :arg,d0 do

That adds one to the destination field of the instruction at the label :arg. I assume "d0" means "bit zero of the instruction D field".

twm47099

Dave,
Thanks I see that now. How do you keep track of instruction bits?

Tom