Awesome!uze6666 wrote:Thanks for the rom and capture. I was able to reproduce the line skip issue. I will review you patch to fully understand what is going on.
I will do some audio tests with 15700 and various buffer sizes.
About the patch, at the time I just wanted to get the emulator to a working state again, so it is a bit heavy-handed and it removes all support for the delayed setting of flags in the TIFR1 register because being more delicate did not work. You are correct in that also makes it not cycle perfect, so a more careful approach is probably needed to actually solve the bug correctly.
TIFR1 is the "TIFR1 – Timer/Counter1 Interrupt Flag Register" described on page 131 of the datasheet.
Here is what the datasheet has to say about "Bit 0 – TOV1: Timer/Counter1, Overflow Flag":
But in the uzem140 code, the setting of TOV1 does not happen immediately like I believe it should, instead it is delayed by one instruction. That still does not fix the vertical jitter, so there has to be at least one more bug. I believe another bug occurs because at the end of the function there is a:The setting of this flag is dependent of the WGMn3:0 bits setting. In Normal and CTC modes, the TOV1 Flag is set when the timer overflows. Refer to Table 14-5 on page 127 for the TOV1 Flag behavior when using another WGMn3:0 bit setting.
TOV1 is automatically cleared when the Timer/Counter1 Overflow Interrupt Vector is executed. Alternatively, TOV1 can be cleared by writing a logic one to its bit location.
Code: Select all
TIFR1 = tempTIFR1;
Code: Select all
TIFR1|=OCF1B; //CTC match flag interrupt
TIFR1|=OCF1A; //CTC match flag interrupt
TIFR1&= ~OCF1A; //clear CTC match flag
TIFR1&= ~OCF1B; //clear CTC match flag
TIFR1&= ~TOV1; //clear TOV1 flag
One possible solution I thought of was to set tempTIFR1 = TIFR1 any time you need to do a direct write to TIFR1, but that doesn't work for the same exact reason. As soon as any one thing sets tempTIFR1, then it blows away any other changes to the tempTIFR1 variable that happened during that call (or a recursive call). Since the only changes to TIFR1 that need to be delayed happen when setting bits, we can probably get away by just merging those bit changes into TIFR1 at the end of the function using:
Code: Select all
TIFR1|=tempTIFR1;
Code: Select all
diff --git a/tools/uzem/avr8.cpp b/tools/uzem/avr8.cpp
index 204d768..b9ad0b7 100644
--- a/tools/uzem/avr8.cpp
+++ b/tools/uzem/avr8.cpp
@@ -648,8 +648,7 @@ void avr8::update_hardware(int cycles)
if (TCNT1 > (0xFFFF - cycles)){
- //TIFR1|=TOV1; //overflow interrupt
- tempTIFR1|=TOV1; //overflow interrupt
+ TIFR1|=TOV1; //overflow interrupt
}
@@ -684,8 +683,7 @@ void avr8::update_hardware(int cycles)
if (TCNT1 > (0xFFFF - cycles)){
if (TIMSK1 & TOIE1){
- //TIFR1|=TOV1; //overflow interrupt
- tempTIFR1|=TOV1; //overflow interrupt
+ TIFR1|=TOV1; //overflow interrupt
}
}
TCNT1 += cycles;
@@ -830,7 +828,7 @@ void avr8::update_hardware(int cycles)
TCCR1B=newTCCR1B; //Timer increments starts after executing the instruction that sets TCCR1B
- TIFR1=tempTIFR1;
+ TIFR1|=tempTIFR1;
}
u8 avr8::exec()
It seems a bit odd that the only interrupt missed would be the very first scanline all the time, but I'm guessing that instead of missing an interrupt, it missed the clearing of the interrupt flag, causing that interrupt to fire twice in a row, which would have incorrectly incremented the scanline counter variable twice, eating the first scanline.
I'm submitting this patch as a pull request.