Teensy4.0 and _disable_irq in the core code

Status
Not open for further replies.
K

Kartman

Member
My project is rather timing conscious and my isr gets delayed by around 100ns at random times when sending data via the usb serial connection. Upon some investigation, it seems interrupts are disabled in a number of places in the usb.c code. Is is advisable to change _disable_irq() and _enable_irq() to NVIC_DISABLE/ENABLE_IRQ(IRQ_USB1)?
 
That might work. Please give it a try and let us know if you experience any averse side effects?

Truth is, how sensitive that code is to being interrupted and having its timing changed really has not yet been explored. Teensy 4.0 is still relatively new and almost all software work so far has gone into porting libraries and filling in features. That sort of optimization work really should be done at some point, but it's been a much lower priority than so many other features.
 
I did try changing _disable_irq() and _enable_irq() to NVIC_DISABLE/ENABLE_IRQ(IRQ_USB1) in usb.c and there didn't seem to be any negative effects, however, it didn't seem to produce any positive ones! When I send characters from the PC to the teensy via usb serial, I do observe a perturbation but have yet to determine what code is responsible. There's also the event code that is littered with enable/disable_irqs, but commenting these out didn't improve the situation.
Any suggestions?
 
Kartman said:
I did try changing _disable_irq() and _enable_irq() to NVIC_DISABLE/ENABLE_IRQ(IRQ_USB1) in usb.c and there didn't seem to be any negative effects, however, it didn't seem to produce any positive ones! When I send characters from the PC to the teensy via usb serial, I do observe a perturbation but have yet to determine what code is responsible. There's also the event code that is littered with enable/disable_irqs, but commenting these out didn't improve the situation.
Any suggestions?
Click to expand...

Can you share some code?
Maybe it's helpful to locate the exact problem..
 
Not quite the highest priority - I left that to the thermal trip. I'm running the critical isr at priority 16 (1 in real terms). I've dumped the NVIC priority registers to check the priority of the other interrupt sources.

The code I'm running relies on specific hardware, so posting my code wouldn't be useful. Nevertheless, the effect can be simulated by connecting a pwm signal into a pin with pinchange enabled. The isr toggles a port pin. You can scope both the pwm and the toggled port pin. It helps if you have a higher end scope with variable/infinite persistence. The observed effect is the execution of the isr is delayed.
 
Ok, I'll try this evening.
It's not a "high end scope" but nowadays cheaper scopes have persistence, too.. ;)
 
Code: 
FASTRUN void pinChange_isr(void)
{
  
  //clear the interrupt flag
  GPIO6_ISR = (1 << 3); //clear interrupt saves 30ns by using constant vs ISR & IMR

  GPIO9_DR_TOGGLE = (1<< 4);//toggle teensy4.0 pin 2
}
void setupPinChange(void)
{

  //let's cheat a little and just use the teensy core code to do most of the work for the setup
  attachInterrupt(0, NULL, RISING);

  //and then use our isr to bypass the longwinded teensy core one
  attachInterruptVector(IRQ_GPIO6789, &pinChange_isr);
  NVIC_ENABLE_IRQ(IRQ_GPIO6789);
  NVIC_SET_PRIORITY(IRQ_GPIO6789, 16); //irqnum,priority. lower num = higher priority
}

I haven't quite figured out how to generate a 1-2MHz source for the interrupts which is what the actual hardware is running at. For the purposes of the initial testing, I used analogWrite(1,50); //the duty cycle is immaterial and joined teensy pins 0 + 1.
 
I can't confirm this:

pic_58_1.png
( Yellow = Pin 0 )

Code: 
FASTRUN void pinChange_isr(void)
{
  digitalWriteFast(0,1);
  delayNanoseconds(50);
  digitalWriteFast(0,0);
  //asm("dsb");
}


void setup() {  
  //Connect Pin 12 to pin 2!!
  pinMode(13,OUTPUT);  
  pinMode(0, OUTPUT);
  pinMode(2, INPUT);
  digitalWrite(13,1);
  analogWriteResolution(8);
  analogWriteFrequency(12,2000000);
  analogWrite(12,127);
  attachInterrupt(digitalPinToInterrupt(2), pinChange_isr, RISING);
  NVIC_SET_PRIORITY(IRQ_GPIO6789, 16); //irqnum,priority. lower num = higher priority  
}


void loop() {
 Serial.println("Hello world");
 delay(10);
}
 
... so
a) you could just use a hardware serial
b) we can now try to find the problem in the USB-Code. Maybe you've missed some disables..
 
Also confirmed, I can see it here immediately on my scope, even without display persistence.

But persistence is needed for a screenshot.

file.png

I dream of someday having a low-effort oscilloscope video capture to youtube upload....
 
Well I think that's the difference of good vs. cheap scope. It takes a long time to see the glitch - on my cheap scope (Hantek DSO5102P)
 
Here's a list (which are responsible depends on your program):
Code: 
Search "__disable_irq" (28 hits in 14 files)
  C:\Arduino\hardware\teensy\avr\cores\teensy4\AudioStream.cpp (6 hits)
	Line 63: 	__disable_irq();
	Line 90: 	__disable_irq();
	Line 130: 	__disable_irq();
	Line 200: 	__disable_irq();
	Line 234: 	__disable_irq();
	Line 266: 		__disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\avr_emulation.h (2 hits)
	Line 43: 	__disable_irq();
	Line 49: 	__disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\delay.c (1 hit)
	Line 95: 	__disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\DMAChannel.cpp (2 hits)
	Line 49: 	__disable_irq();
	Line 95: 	__disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\eeprom.c (2 hits)
	Line 244: 	__disable_irq();
	Line 284: 	__disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\EventResponder.h (2 hits)
	Line 228: 		__disable_irq();
	Line 267: 		__disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\HardwareSerial.cpp (1 hit)
	Line 461: 	__disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\imxrt.h (1 hit)
	Line 8955: #define __disable_irq() __asm__ volatile("CPSID i":::"memory");
  C:\Arduino\hardware\teensy\avr\cores\teensy4\tempmon.c (1 hit)
	Line 16:   __disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\Tone.cpp (2 hits)
	Line 71: 	__disable_irq();
	Line 133: 	__disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\usb.c (3 hits)
	Line 351: 	__disable_irq();
	Line 866: 	__disable_irq();
	Line 999: 		__disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\usb_audio.cpp (2 hits)
	Line 239: 	__disable_irq();
	Line 361: 	__disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\usb_touch.c (1 hit)
	Line 74: 	__disable_irq();
  C:\Arduino\hardware\teensy\avr\cores\teensy4\wiring.h (2 hits)
	Line 156: #define cli() __disable_irq()
	Line 158: #define noInterrupts() __disable_irq()
 
Thanks guys for having a look. Now i can see how to crank up the pwm frequency. Funnily enough i came across the same feature on a xmc1100 cpu just a few days ago.

Changing the code in usb.c didn’t seem to make any discernible difference. Nor did it in event responder.
Another test is to type characters on your terminal program and seems to give more consistent results - ie jitter.

Paul, love your work. Teensy 4.1 is just what the doctor ordered. My current projects need the extra i/o and hopefully there is a sequential 8bits in a port so i don't need to do shifting shenanigans.
 
Seems to work with an empty yield() function:

Code: 
void yield() {}

void loop() {
 Serial.println("Hello world");
 delay(1);
}
( + replace the interrupt-disables in usb.c the way you mentioned in #post 1)

I don't see the glitches anymore - but again - my scope seems to be not the best to measure this! (I accept donations ;))

@Paul - if you find the time.. it would be good to make the changes to usb.c in the next weeks - I don't think it slows down anything.
We should review the event-responder, too (and not forget Teensy 3.x)

Edit: After some minutes now the max glitch is about ~30ns. Wonder where a glitch that short can come from...
Hopefully 30ns are OK for you, Kartman?
 
Last edited:
Frank B said:
Wonder where a glitch that short can come from...
Click to expand...
A part of the solution is - again - the event responder.. :-(

I wonder if there is any person who uses it...

Anyway, it's best to replace the systick with a shorter function.
Code: 
  void mySystick_isr(void) {
    systick_cycle_count = ARM_DWT_CYCCNT;
    systick_millis_count++;
  }
...
_VectorsRam[15] = mySystick_isr;

Now ~24ns - i bet this could be less - but I run out of ideas.
 
Last edited:
Thanks for the good work Frank and Paul. I have two projects - one interfaces to an asynchronous bus, so 24-30ns is not a concern and I have hardware to latch the data. The other project interfaces to a synchronous bus, so 30ns is tolerable methinks - just need to make sure the worst case doesn't cause problems.

Danke.
 
Frank B said:
Seems to work with an empty yield() function:

Code: 
void yield() {}

...
...
Click to expand...

I was going to add void yield() - but that is an easy thing to say ... hadn't looked lately to see the affect so directly on jitter.

Especially given that a T_4.x typically calls it 1 to 7+ million times per second just leaving loop()
 
Further testing was done and I can confirm the fixes to usb.c did have the most effect and retargeting the systick interrupt had a smaller effect - just as Frank detailed. Thanks to all that contributed.
 
Did anyone try the stuff I mentioned yesterday I mentioned in the eventResponder thread: https://forum.pjrc.com/threads/6094...onder-properly?p=240107&viewfull=1#post240107

I believe runFromYield already has short cut in place that should not normally disable interrupts...

Code: 
static void runFromYield() {
		if (!firstYield) return;
		// First, check if yield was called from an interrupt
...

So again I would suggest the same for running from interrupt.
Code: 
void EventResponder::runFromInterrupt()
{
	[COLOR="#FF0000"]if (!firstInterrupt) return;[/COLOR]
	while (1) {
		bool irq = disableInterrupts();
		EventResponder *first = firstInterrupt;
		if (first) {
			firstInterrupt = first->_next;
			if (firstInterrupt) {
				firstInterrupt->_prev = nullptr;
			} else {
				lastInterrupt = nullptr;
			}
			enableInterrupts(irq);
			first->_triggered = false;
			(*(first->_function))(*first);
		} else {
			enableInterrupts(irq);
			break;
		}
	}
}
I know that some may dislike the event responder and the like, in much the same way I dislike the Arduino serialEvent(X) code was defined...
But maybe we can whittle away some of the rough edges.

Edit: I personally prefer to see if there are ways with remove any of these obstacles when possible as hopefully it improves it for most sketches without everyone having to worry about it.

As for using eventResponder - It is used in SPI for doing Async transfers.... Although I personally typically use it to call immediate
 
Status
Not open for further replies.
Back
Top