digitalWriteFast, digitalReadFast, pinModeFast etc

[jrraines]

Solved.

I had an extra copy of the old version of digitalWriteFast.h in another folder within the Library folder. That one was getting used instead of the one that I was modifying. All 3 boards work with the new digitalWriteFast now, when I use the current (arduino 21) wiring.h and wiring_digital.c

When I try to change to the version of those core files that I built a few days ago, digitalWriteFast does not cooperate with them. I will struggle with that at least a little longer before I repost code.

It isn't absolutely critical that I get the two things to work together but it would be nice.

[jrraines]

Paul Stoffregen pointed out that these macros are not currently safe from issue #146 on the Megas. Conceptually what I have to add is something like

Code: [Select]

void inline atomicWrite( uint16_t address, unint8_t p, unint8_t v) {
 if (address < 0x20)     bitWrite(*address, __digitalPinToBit(p),v);
 else  {
   uint8_t register saveSreg = SREG;    
       cli;                                  
       bitWrite(*address, __digitalPinToBit(p), v);  
       SREG=saveSreg;
 }
}

#define digitalWrite(P, V) \
do {
 if (__builtin_constant_p(P) && __builtin_constant_p(V))   atomicWrite(digitalPinToPortReg(P),P,V);
   else  __digitalWrite((P), (V));         \
}while (0)

Last night the compiler and I couldn't agree on the exact syntax. the Xcode editor I was using wouldn't help balance the last few }'s.

Surprisingly, this morning the Arduino IDE editor agrees with how I thought the }'s balanced. I broke it up by splitting out atomicWrite this morning but it seems obvious a cast of some sort is missing and I won't test it now.

Handling issue 146 will further diverge performance on ports above F, which already required more code. It struck me overnight that this MIGHT be a reason for continued existence of digitalWriteFast beyond implementation of faster digitalWrite in the core. After all, the vast majority of code I write does not turn the interrupt on. Worth some thought.

It also strikes me that I should probably create test code for whether issue 146 is solved. Maybe I can just add Tone to my current test programs, or something like that. I need something that will generate interrupts 10-1000 times per second, I think. If anyone understands how to write that, or can point me to a forum item I would appreciate that.

[bperrybap]

Handling issue 146 will further diverge performance on ports above F, which already required more code. It struck me overnight that this MIGHT be a reason for continued existence of digitalWriteFast beyond implementation of faster digitalWrite in the core. After all, the vast majority of code I write does not turn the interrupt on. Worth some thought.

It also strikes me that I should probably create test code for whether issue 146 is solved. Maybe I can just add Tone to my current test programs, or something like that. I need something that will generate interrupts 10-1000 times per second, I think. If anyone understands how to write that, or can point me to a forum item I would appreciate that.

Ah... issue #146. (I was the one that asked Paul to post this bug)

There should be no need to write & run test code to see if this is resolved.
Either the foreground bit set/clear operations are atomic or they
aren't. A simple code inspection can determine this.
(Ideally, look at the assembler output, it will be obvious)

I recommend code inspection over test code because it is super easy and because with test code there is no guarantee that the exact timing scenario will ever exist to create a problem.

Keep in mind that ensuring that the i/o operation is atomic
is not about protecting bits set/cleared in registers by the users foreground (loop) code from being interrupted by interrupt code, it is about protecting the bits set/cleared by the other code done in interrupt routines from the users foreground (loop) code.

It does not matter if the user does not have any
code that runs as an interrupt. It will be his foreground (loop) code
that clobbers the bits set/cleared/used by the interrupt routine.
The users foreground (loop) code  i/o operations will always complete and work flawlessly with or without the atomic code masking

And so Ironically, the code that runs at interrupt level like the servo library code does not need to have the additional code overhead of masking/clearing interrupts to make the operation "atomic"  since the code is running at interrupt level, it is by definition atomic but
the foreground (loop) code running must have it, if they share the
same i/o register.

And that is the dilemma, sometimes you need the extra code overhead
to ensure the operation is atomic and sometimes you don't need
it to still have atomicity, and yet other times, you may not need atomicity at all.
The problem is the only time you can be ensured that you don't need
it, is if the code is part of an interrupt routine.

In my view,  ALL the i/o operations need to be atomic.
Anything else, doesn't really work, and eventually will cause
problems.
Yes there are situations where you can be guaranteed that the
additional code to mask/restore interrupt masks isn't needed but
those are only in interrupts routines which are typically library code
and usually not done by the normal/casual arduino user.

If it were desirable to support some thinner/faster i/o routines to be used
by ISR routines, then that is definitely possible
but again, this goes back to my original suggestion of using
layering and _  (underbar) functions/macros so that the code writer can pick and choose what level of support his code needs.

--- bill

[jrraines]

I did finally post a significant update to this:
http://code.google.com/p/digitalwritefast/downloads/list

In the original version of this library I supplied 2 versions of each command with different behavior around turning off PWM (the analogWrite facility). This version does not turn off PWM--that feature is planned to be removed from future versions of the standard commands.

In this version I only supply 1 version of the commands

Much more importantly this version is interrupt safe.

There are several good descriptions of the need for interrupt safe digitalWrite and pinMode commands. I struck me that I have a disassembly listing of the commands that certainly can be used to describe the situation in a different, not necessarily better, way:

For Ports F and below, a single instruction can be used to set or clear an individual bit that corresponds to the digitalWrite or PinMode setting you want to change. It will look like this:

Code: [Select]

pinModeFast(61,OUTPUT);
   390a:      87 9a             sbi      0x10, 7  ; 16  set a bit of port to 1
digitalWriteFast(61,LOW);
   390c:      8f 98             cbi      0x11, 7  ; 17  clear a bit of port to 0
The cbi and sbi instructions are inherently interrupt safe.

For Ports above F, the microprocessor must read in all the 8 pins in a Port, modify and rewrite all 8 pins. It would look like this if (it were done in an interrupt unsafe fashion):

Code: [Select]

digitalWriteFast(64,HIGH);
   38fe:      80 91 08 01       lds      r24, 0x0108  ;read the 8 bits of port into register 24
   3902:      84 60             ori      r24, 0x04    ; set a bit in register 24 to 1
   3904:      80 93 08 01       sts      0x0108, r24  ; write register 24 back to the port

The cbi and sbi instructions cannot address the ports higher than F.

The problem arises when the port is modified by other means in the microsecond between it being read into the register, modified and rewritten. Most commonly this seems to come up with the Servo library, but  any code that has an interrupt routine that itself does a digitalWrite or pinMode might cause the issue.

What happens is that the 'lds      r24, 0x0108' would read the current information from the port. Then it might happen that an interrupt would divert the microprocessor to handle another task. Imagine that 0x108 is modified during processing of the interrupt. Then the microprocessor returns to your program, which proceeds with 'ori      r24, 0x04' setting the bit you wanted, and 'sts      0x0108, r24' destroying whatever the interrupt routine did. The interrupt could come between the ori and the sts as well, of course. The problems are notoriously hard to debug; the exact timing of the interrupt cannot be replicated and the bits of code that interact are likely to be located far from each other--perhaps even in a library, whose source code you have never read!

This version of the library correctly generates interrupt safe code that looks like this:

Code: [Select]

digitalWriteFast(64,HIGH);
   38fa:      9f b7             in      r25, 0x3f      ; 63  read the interrupt status
   38fc:      f8 94             cli                  ; turn off interrupts
   38fe:      80 91 08 01       lds      r24, 0x0108
   3902:       84 60             ori      r24, 0x04      ; 4
   3904:       80 93 08 01       sts      0x0108, r24
   3908:       9f bf             out      0x3f, r25      ; 63  restore the interrupt status


This is needed for digitalWriteFast and pinModeFast.  The standard digitalWrite was corrected to cover this situation in Arduino 19.  

[cjands40]

Skimming through this thread, all of the examples presented have used only the constants HIGH and LOW as the values being written using digitalWriteFast.  This does correspond with the Reference documentation for digitalWrite as well as many of the example sketches but there are some examples (eg, BlinkWithoutDelay) and likely much existing code that use variables instead.  In fact, digitalWrite currently maps any non-zero value into a write of '1'.  I would like to see that behavior maintained.  How will the new "fast" routines handle these cases?

[jrraines]

Code: [Select]

pinModeFast(2,INPUT);
    1a0:      52 98             cbi      0x0a, 2      ; 10
digitalWriteFast(2,200);
    1a2:      5a 9a             sbi      0x0b, 2      ; 11
pinModeFast(5,OUTPUT);
    1a4:      55 9a             sbi      0x0a, 5      ; 10
digitalWriteFast(5,LOW);


Code: [Select]

pinModeFast2(2,INPUT);
    1c6:      52 98             cbi      0x0a, 2      ; 10
digitalWriteFast2(2,HIGH);
    1c8:      5a 9a             sbi      0x0b, 2      ; 11
pinModeFast2(5,OUTPUT);
    1ca:      55 9a             sbi      0x0a, 5      ; 10
digitalWriteFast2(5,LOW);
    1cc:      5d 98             cbi      0x0b, 5      ; 11