buserror / simavr

simavr is a lean, mean and hackable AVR simulator for linux & OSX
GNU General Public License v3.0
1.54k stars 366 forks source link

External pin interrupt kills performance despite being disabled #465

Open maltaisn opened 2 years ago

maltaisn commented 2 years ago

The avr_exint module keeps registering a cycle timer for the next cycle to make the low level triggered pin interrupt work correctly. Is there any reason why this is done even when all external interrupts are disabled in EIMSK? Wouldn't be better if there was a check to only register the timer it if the interrupt is enabled, and when the interrupt is enabled if the pin is low?

I'm adding a new part and this kills performance when using PORTD as a parallel port, because the port values changes at 1 MHz. Profiling shows that >70% of the time is spent processing the external interrupt cycle timers. I'm using the ATmega328p part it matters. For now I used avr_extint_set_strict_lvl_trig(avr, x, 0) to disable level triggering.

gatk555 commented 2 years ago

I think the reason is that it was designed that way: my guess is that it was to keep the code small. To fix this, the code needs to monitor EIMSK and EICRA (xx8 family names), but it does not. I believe that introduces several bugs: for example, enabling EIMSK just after startup will not raise the interrupt.

I have a private change that addresses that and removes the cycle timer, but I do not believe it uses the correct approach. I think the best way is to rework the interrupt code so that it directly supports level-triggering and then use that. I have an initial version, but no tests. The interrupt changes are relevant to #463.

If you have any use for them, I can push the relevant changes to my fork.

gatk555 commented 2 years ago

Should be fixed here where the fast timer was removed and level-triggering handled as described above. Unfortunately it is not easy to make a PR for it, other than one that requests copying the whole fork.