avr8js icon indicating copy to clipboard operation
avr8js copied to clipboard

timer1: mode 9 is not working anymore

Open fjangfaragesh opened this issue 4 years ago • 13 comments

In real (tested with Arduino Uno), the LED on pin 10 is blinking fast. In simulation nothing happens.

#ifndef F_CPU
#define F_CPU 16000000UL // 16 MHz clock speed
#endif

int main(void){
  DDRB |=  (1 << PORTB2); //Define OCR1B as Output (pin number arduino: 10)
  TCCR1A |= (1 << COM1A0) | (1 << COM1B1) | (1 << WGM10); // PB2: output, PWM mode 9
  TCCR1B |= (1 << CS10) | (1 << CS11) | (1 << WGM13);// prescaler=64, mode 9
  OCR1A = 4000;
  OCR1B = 1000;
  while(1) {
  }
}

fjangfaragesh avatar Oct 22 '21 10:10 fjangfaragesh

Thanks for reporting! Do you have any idea if it worked in previous version of the simulator?

urish avatar Oct 22 '21 17:10 urish

Initial analysis: it seems like the issue happens if OCR1A is still 0 when you configure the timer. A workaround would be to first set OCR1A / OCR1B, and then enable the timer:

int main(void){
  DDRB |=  (1 << PORTB2); //Define OCR1B as Output (pin number arduino: 10)
  OCR1A = 4000;
  OCR1B = 1000;
  TCCR1A |= (1 << COM1A0) | (1 << COM1B1) | (1 << WGM10); // PB2: output, PWM mode 9
  TCCR1B |= (1 << CS10) | (1 << CS11) | (1 << WGM13);// prescaler=64, mode 9
  while(1) {
  }
}

Here's a complete functional example: https://wokwi.com/arduino/projects/313183501205635648

To fix it, we need to figure out what to do if the timer starts and TOP (OCR1A) == 0. Should it generate compare match on every single timer cycle?

urish avatar Oct 22 '21 20:10 urish

Thanks for reporting! Do you have any idea if it worked in previous version of the simulator?

It worked in version 0.11.4.

The change of the OCR1A register is apparently only adopted after the change of direction at the old top:
(i think this behavior is wrong) grafik (blue: ORC1A, red:TCNT1, yellow:ORC1B, at tick 26,000,000 and 32,500,000 OCR1A is changed and TCNT1 is set to 0)

fjangfaragesh avatar Oct 23 '21 09:10 fjangfaragesh

Thanks!

Is the graph based on version 0.11.4 or on an actual chip?

urish avatar Oct 23 '21 18:10 urish

version 0.18.2

fjangfaragesh avatar Oct 23 '21 18:10 fjangfaragesh

The change of the OCR1A register is apparently only adopted after the change of direction at the old top:

As far as I can tell from the datasheet, this is the correct behavior, isn't it?

image

urish avatar Oct 23 '21 18:10 urish

I've tested it. That doesn't seem like the right behavior. I have written a program that reads TCNT1 and changes OCR1A. At the end, the results are transmitted via the serial interface.

int results[500];
int main(void){
  Serial.begin(9600);
  DDRB |=  (1 << PORTB2); //Define OCR1B as Output (pin number arduino: 10)
  OCR1A = 2000;
  OCR1B = 500;
  TCCR1A |= (1 << COM1A0) | (1 << COM1B1) | (1 << WGM10); // PB2: output, PWM mode 9
  TCCR1B |= (1 << CS10) | (1 << CS11) | (1 << WGM13);// prescaler=64, mode 9
  for (int i = 0; i < 250; i++) {
    results[i] = TCNT1;
    _delay_us(100);
  }
  OCR1A = 1000;
  OCR1B = 100;
  TCNT1 = 0;
  for (int i = 0; i < 250; i++) {
    results[i+250] = TCNT1;
    _delay_us(100);
  }
  for (int i = 0; i < 500; i++) Serial.println(results[i]);
  Serial.flush();
  while(1) {
  }
}

Results: (wimulation compared with real Arduino Uno) grafik

fjangfaragesh avatar Oct 23 '21 19:10 fjangfaragesh

Thank you for doing this comparison, it's really useful!

I need to search the data sheet to see if I can find where the exact behavior is defined, and adjust the simulation accordingly

urish avatar Oct 23 '21 19:10 urish

Ok, we have a fix. I verified it it the following test program:

int main(void) {
  uint8_t v0, v1, v2, v3;
  Serial.begin(9600);
  asm(R"(
    CLR r1          ; r1 is our zero register
    LDI r16, 0x0    ; OCR1AH = 0x0;
    STS 0x89, r1    
    LDI r16, 0x8    ; OCR1AL = 0x8;
    STS 0x88, r16  
    ; Set waveform generation mode (WGM) to PWM Phase/Frequency Correct mode (9)
    LDI r16, 0x01   ; TCCR1A = (1 << WGM10);
    STS 0x80, r16  
    LDI r16, 0x11   ; TCCR1B = (1 << WGM13) | (1 << CS00);
    STS 0x81, r16  
    STS 0x85, r1    ; TCNT1H = 0x0;
    STS 0x84, r1    ; TCNT1L = 0x0;

    LDI r16, 0x5   ; OCR1AL = 0x5; // TCNT1 should read 0x0
    STS 0x88, r16  ; // TCNT1 should read 0x2 (going up)
    STS 0x84, r1   ; TCNT1L = 0x0;
    LDS %0, 0x84  ; // TCNT1 should read 0x1 (going up)
    LDS %1, 0x84  ; // TCNT1 should read 0x3 (going up)
    LDS %2, 0x84  ; // TCNT1 should read 0x5 (going down)
    LDS %3, 0x84  ; // TCNT1 should read 0x3 (going down)
  )" : "=r"(v0) , "=r"(v1), "=r"(v2), "=r"(v3) );
  Serial.println(v0);
  Serial.println(v1);
  Serial.println(v2);
  Serial.println(v3);
  Serial.flush();
  for (;;);
}

When running on a physical board, I get the following output:

1
3
5
3

I added a test case that validates this behavior (it fails with the previous version, passes with the fix).

Would love to get your feedback on the new version with the fix (0.18.5)

urish avatar Oct 29 '21 10:10 urish

The behavior of TCNT1 now seems to be correct.

fjangfaragesh avatar Nov 01 '21 10:11 fjangfaragesh

Lovely! Thanks again for reporting it and all your help in tracing the issue!

urish avatar Nov 01 '21 10:11 urish

But the timer output does not seem to be correct at the beginning.
The first pulse is missing.

#define DLY 135
#define N 128

int results[N*2];
int resultsPINB[N*2];
int main(void){
  Serial.begin(9600);
  DDRB |=  (1 << PORTB2); //Define OCR1B as Output (pin number arduino: 10)
  OCR1A = 2000;
  OCR1B = 1500;
  TCCR1A |= (1 << COM1A0) | (1 << COM1B1) | (1 << WGM10); // PB2: output, PWM mode 9
  TCCR1B |= (1 << CS10) | (1 << CS11) | (1 << WGM13);// prescaler=64, mode 9
  TCNT1 = 0;
  for (int i = 0; i < N; i++) {
    results[i] = TCNT1;
    resultsPINB[i] = PINB;
    _delay_us(DLY);
  }
  OCR1B = 100;
  for (int i = 0; i < N; i++) {
    results[i+N] = TCNT1;
    resultsPINB[i+N] = PINB;
    _delay_us(DLY);
  }
  // send results
  for (int i = 0; i < N*2; i++) {
    Serial.print(results[i]);
    Serial.print("\t");
    Serial.println(((resultsPINB[i] >> 2) & 1)*1000);
  }
  Serial.flush();
  while(1) {
  }
}

There is a difference in the behavior of the physical and the simulated:
image

fjangfaragesh avatar Nov 01 '21 10:11 fjangfaragesh

Thanks for providing detailed reproduction with graphs! I haven't had the time to look into it yet, but it's on my list.

urish avatar Nov 14 '21 20:11 urish