rust-console/gba

What's the current status of IRQs?

Lymia opened this issue · 12 comments

Lymia commented

There is currently no good story for how IRQs work, and the code for the assembly part of IRQs handlers is commented out. Is this a mistake, or are there further plans for how to handle this in the future?

There used to be a very complex wrapper around IRQs, and I had assumed that most of it was needed for one reason or another. Does it turn out not to be?

the updated irq example shows a somewhat reasonable way to do it in basically all rust without needing stuff on the assembly side.

the current main issue is that a32 functions can't inline t32 code but that includes all of code that the compiler doesn't MIR inline. So a32 functions have trash performance. Hopefully we can improve this over time.

The IRQ example was broken and is now is fixed.

However, we certainly need some docs on this area.

Is it possible to create a IRQ trait that can be used with a struct?
something along the lines of:

trait IRQ {
  #[instruction_set(arm::a32)]
  /// default impl that should not be overriden
  extern "C" fn irq_handler_a32() {
    Self::handler();
  }
  /// Default impl that should not be overrided
  fn handler() {
    // code from `irq_handler_t32` from the irq example
    // Check irq flags and call their method if needed
  }
  /// Should be overriden if the user want to do something for IRQ
  /// Will be called by Self::handler if the corresponding IRQ flag is set
  fn vblank_handler() {}
  fn hblank_handler() {}
  // other handlers
}

Then you can impl the trait

struct Handler;
impl IRQ for Handler {
  // handler functions
}

I don't much about GBA interrupts, but I feel like this would be an easier way to implement handlers.

I'm not clear on how that would simplify things though.

Instead of having to reimplement the irq_handler_t32/irq_handler_a32, in my example handler, you could impl the IRQ trait and only override the IRQ handler you want.

So if you want a simple vblank handler:

struct VBlankHandler;
impl IRQ for VBlankHandler {
    /// Only need to override the one method for VBlank
    fn vblank_handler() {
        // Do something during vblank
    }
}

// Setup IRQ handler
// Using default irq_handler_a32 impl from above IRQ trait def
unsafe { USER_IRQ_HANDLER.write(Some(VBlankHandler::irq_handler_a32)) };

Maybe I'm thinking of this too high level and just using a module with functions would be better.

Well... so the user's IRQ handler must be an a32 function, because of how the hardware works, but there's no way to enforce that within a trait. And then during the interrupt the handling code must write some specific flags to make things work right, which a trait also can't enforce.

I guess i'm approaching the situation from the perspective that traits let you define an interface for interacting with a type, and i'm not clear on what the interrupt handling interface is supposed to be like.

By enforcing it, do you mean if the default methods (equivalents to irq_handler_t32 and irq_handler_a32 ) overriden? Is there another way to enforce the user does those tasks in the interrupt handler?

In my head irq_handler_t32 and irq_handler_a32 would be default trait methods that should not be overriden, which call methods for each interrupt (vblank, hblank, etc.) that the user can override. So instead of writing the a32 function, t32 function, and the other handler functions for each interrupt handler you want, you can just override the specific interrupts and not worry about writing and reading flags for each implementation.

I've adjusted the code snippets I had to better demonstrate what I had in mind.

Well, the situation kinda works out all like this:

  • The user's IRQ handler function pointer in USER_IRQ_HANDLER must point to an a32 function. This is an absolute requirement.
  • Because of how things currently work, a32 code has mostly "very bad" performance, because it doesn't inline as much as you'd expect, and Rust relies on a lot of inline.
  • So you are advised to just have the a32 function call to a t32 function and then have the t32 function do the heavy lifting. (Alternately, you could just write some a32 inline asm, for example).
  • During the interrupt handling you should tell the GBA itself what interrupt you're handling:
    • read IE, read IRQ_PENDING, bitand the values, and what's left is the actual interrupts you should handle during this call to the handler.
    • The interrupt you just processed should be written to IRQ_ACKNOWLEDGE. Note that you are allowed to handle more than one interrupt per call to the handler, and this is more efficient, so do that if you can.
    • If it's an IntrWait interrupt you should also enable the appropriate bits in INTR_WAIT_ACKNOWLEDGE.

None of these concerns are things that the library can really handle for you. Maybe there could be some kind of optional framework thing that would have a mostly default handler setup that you could fill in somehow? But the big problem is that the interrupt handler of your program is the most performance sensitive part of the program. Having a switchboard that calls a bunch of functions that might be blank doesn't seem like a great idea to me.

I wouldn't mind a cargo feature to have a module with an interrupt helper framework, I just don't think I'd use it.

Lymia commented

It's honestly more a matter of the design of the IRQ interface more than anything else. You could do work to ensure unused IRQ bits optimize out, for example, e.g. by having const HAS_VBLANK_HANDLER: bool = true; consts in it or some such. Most of the actual interrupt handling/flag checking/etc is boilerplate.

Could we use the InterruptFlags struct as a const bit field for this to be optimized away?

So, I created an implementation using a trait, but it requires building release or setting opt-level >= 1 in order to optimize away the unused handlers. And I've tried using both const INTERRUPTS_TO_HANDLE: InterruptFlags and const DO_HANDLE_VBLANK: bool.
So it's probably not best to use them in this crate, unless there's another way to enforce that optimization.

The current status is that you assign your handler function to a GbaCell, then it gets called during interrupts and given which interrupt bits are happening during that call. That's easy to use, so we can call this complete I think.