rust-console / gba

A crate that helps you make GBA games
https://docs.rs/gba
Apache License 2.0
652 stars 50 forks source link

Many vblank irqs sent per frame #152

Closed Sp00ph closed 2 years ago

Sp00ph commented 3 years ago

I'm not sure if this is an error in the library or my code, but I'm thoroughly confused by this. Consider the following code:

#![no_std]
#![no_main]
#![feature(isa_attribute)]

use gba::{fatal, prelude::*};

#[panic_handler]
fn panic(info: &core::panic::PanicInfo) -> ! {
    fatal!("{}", info);
}

fn wait_vblank() {
    unsafe { gba::bios::VBlankIntrWait() }
}

fn init_registers() {
    unsafe {
        IME.write(true);
        IE.write(InterruptFlags::new().with_vblank(true));
    }

    DISPSTAT.write(DisplayStatus::new().with_vblank_irq_enabled(true));
}

#[instruction_set(arm::a32)]
extern "C" fn irq_handler_a32() {
    static COUNTER: Static<usize> = Static::new(0);
    let c = COUNTER.read();
    gba::warning!("{}", c);
    COUNTER.write(c + 1);
    unsafe { INTR_WAIT_ACKNOWLEDGE.write(INTR_WAIT_ACKNOWLEDGE.read().with_vblank(true)) }
}

#[no_mangle]
fn main() -> ! {
    unsafe { USER_IRQ_HANDLER.write(Some(irq_handler_a32)) };
    init_registers();
    loop {
        wait_vblank();
    }
}

All this should do is print the value of COUNTER once every frame, after the vblank irq is raised. Instead it seems to run the irq handler many times per frame (on my laptop around 5 times per frame in debug mode and 150 times per frame in release mode). I'm pretty stumped by this and can't think of an explanation as for why multiple vblank interrupts should be fired each frame. If I replace the wait_vblank implementation with one which just does a busy loop, everything works as intended, but I don't really want to do that.

Lokathor commented 3 years ago

my first thought is that the display control starts with the Forced Blank bit enabled, and that could be screwing up the vblank interrupt system.

Sp00ph commented 3 years ago

that's not it, the same thing happens when disabling the forced blank bit

Lokathor commented 3 years ago

Oh, you're not putting an acknowledge in both places.

https://github.com/rust-console/gba/blob/main/examples/irq.rs#L134-L138

You have to respond using IRQ_ACKNOWLEDGE for all interrupts, and separately respond to INTR_WAIT_ACKNOWLEDGE whenever the BIOS IntrWait routine gets used.

Sp00ph commented 3 years ago

Ok, that fixed the initial issue. Now i keep getting crashes though, which say that the code jumped to the invalid address FFFFFFFC. Any idea why that might be?

Lokathor commented 3 years ago

I've also hit that problem, and I don't know what's up with that.

Lokathor commented 3 years ago

I suspect some sort of problem with a32/t32 interwork stuff though.

Lymia commented 3 years ago

Could that be an IRQ-in-IRQ problem? AFAIK IRQ handlers need to set IME=no or some such things. Not to mention, the interrupt stack is very small and easy to overflow.

Sp00ph commented 3 years ago

Considering that i only have the vblank interrupts enabled, and my irq handler is very short, i would doubt that a second irq is fired before the first one is done. Curiously, it consistently seems to run the irq handler ~20 times before crashing too.

Sp00ph commented 3 years ago

Interestingly, it only seems to crash in debug builds, not release builds. The crashes persist even when disabling IME during the irq handler

Lymia commented 3 years ago

This feels stack-related to me between the consistant timing, and not happening on release. I wonder if some of the ASM that used to be in the IRQ handler is essential for avoiding this stuff?

Lokathor commented 3 years ago

all that it did that seems significant is switch CPU modes from irq mode over to user mode.

Lymia commented 3 years ago

Yeah. My best theory here is just that it's overflowing the interrupt stack and trashing the main stack. Release builds tend to use less stack space due to inlining/etc, so that's a major possible culprit.

Sp00ph commented 3 years ago

Then why would that not have happened on the old irq handler

Lymia commented 3 years ago

Good point. I've had similar trouble while ROM hacking (involving injecting Rust code), which is why that's my first theory. The ROM I'm working with uses a coroutine system with 512-byte stacks, and I was routinely overflowing that in my hooks by calling gba::log! and such. That thing uses more stack than it seems. let me investigate properly.

Ruin0x11 commented 3 years ago

Does the new interrupt handler switch to system mode before jumping to the user-provided interrupt handler? It seems that if you do not switch modes first, there is a chance that various things could get clobbered if another interrupt happens during the handling of the first interrupt. Switching to system mode prevents this.

Unless I'm missing something, directly setting the user interrupt handler to point to a function bypasses all that setup. ARM's documentation says this handler must be written in assembly.

See also: https://stackoverflow.com/a/22500017.

Lokathor commented 3 years ago

Technically the "real" interrupt handler on the GBA is in the BIOS and then that's what calls over to your interrupt handler.

But yeah, I guess a mode switch might be the fix.

Lokathor commented 2 years ago

The assembly level handler always sets system mode before calling the rust level handler, so this problem shouldn't happen anymore