git.y1.nz

commit 03613b5550ce741acb1f3307c4f17b9b86115645
parent 3a987af9f0a47dc83270496c62c8a7862f327ba2
Author: Michel Iwaniec <46843052+michel-iwaniec@users.noreply.github.com>
Date:   Tue, 22 Apr 2025 08:36:41 +0100

NES: Make sure TIM ISR handlers are thread-safe (#760)

* NES: Make sure TIM ISR handlers are thread-safe

- Backup and restore REGTEMP area when calling TIM ISR in vblank

- Update irq example to use #pragma nooverlay for tim handler

- Add warning and explanation about having to use #pragma for TIM handlers in NES section of docs

- Add @anchor to docs, and a code example for #pragma nooverlay

- Update irq.c with `#pragma nooverlay` only being applied if NINTENDO_NES is defined

- Add more detailed comment of `#pragma nooverlay` in irq.c

- Add reference to docs in gb.h

- Add similar reference to nes.h
Diffstat:
M
docs/pages/06b_supported_consoles.md
16
++++++++++++++++
M
gbdk-lib/examples/cross-platform/irq/src/irq.c
9
+++++++++
M
gbdk-lib/include/gb/gb.h
4
++++
M
gbdk-lib/include/nes/nes.h
3
+++
M
gbdk-lib/libc/targets/mos6502/nes/crt0.s
22
++++++++++++++++++++++

5 files changed, 54 insertions(+), 0 deletions(-)
diff --git a/docs/pages/06b_supported_consoles.md b/docs/pages/06b_supported_consoles.md
@@ -527,6 +527,22 @@ If you are porting a GB game to gbdk-nes where the music handler is called in th
 
 Keep in mind that you should NOT write any graphics registers in the TIM handler. This will likely not do what you want, and it may result in bad graphical glitches.
 
+@anchor docs_nes_tim_overlay
+@note
+Because the TIM handler will be called from the vblank NMI, this function and all functions it calls need to use the `#pragma nooverlay` command. This makes memory for local variables and function parameters unique to this function instead of being shared with other functions' allocations in the reusable overlay segment.
+
+    #pragma save
+    #pragma nooverlay
+    void tim_isr(void)
+    {
+        // Do TIM isr things
+    }
+    #pragma restore
+
+Without this pragma the calls in your TIM handler could end up overwriting local variables or function parameters that the main program was using when it was interrupted.
+You should also avoid calls to the standard library, and even multiplications and division / modulo operations in your TIM handlers. These more expensive math operations in SDCC are currently implemented with functions that use the overlay segment and would cause similar conflicts.
+For more details on overlay segment and interrupts, please see section 3.7 in the SDCC manual.
+
 ### Tile Data and Tile Map loading
 
 #### Tile and Map Data in 2bpp Game Boy Format
diff --git a/gbdk-lib/examples/cross-platform/irq/src/irq.c b/gbdk-lib/examples/cross-platform/irq/src/irq.c
@@ -14,11 +14,20 @@ void vbl(void)
   vbl_cnt++;
 }
 
+#if defined(NINTENDO_NES)
+// For NES make sure to wrap TIM interrupt handlers with the nooverlay pragma.
+// This avoids interrupts accidentally overwriting variables in the overlay segment that were being used when the NMI occurred.
+#pragma save
+#pragma nooverlay
+#endif
 void tim(void)
 {
   // Upon IRQ, interrupts are automatically disabled
   tim_cnt++;
 }
+#if defined(NINTENDO_NES)
+#pragma restore
+#endif
 
 void print_counter(void)
 {
diff --git a/gbdk-lib/include/gb/gb.h b/gbdk-lib/include/gb/gb.h
@@ -294,6 +294,10 @@ void add_LCD(int_handler h);
 
     Up to 4 handlers may be added, with the last added
     being called last.
+    
+    For NES make sure to wrap TIM interrupt handlers 
+    with a nooverlay pragma. 
+    For more details see @ref docs_nes_tim_overlay
 
     @see add_VBL
     @see set_interrupts() with TIM_IFLAG, ISR_VECTOR()
diff --git a/gbdk-lib/include/nes/nes.h b/gbdk-lib/include/nes/nes.h
@@ -282,6 +282,9 @@ void add_LCD(int_handler h) NO_OVERLAY_LOCALS;
     It is therefore currently limited to 60Hz / 50Hz 
     (depending on system).
 
+    Make sure to wrap TIM interrupt handlers with a nooverlay pragma. 
+    For more details see @ref docs_nes_tim_overlay
+
     @see add_VBL
     @see set_interrupts() with TIM_IFLAG, ISR_VECTOR()
 */
diff --git a/gbdk-lib/libc/targets/mos6502/nes/crt0.s b/gbdk-lib/libc/targets/mos6502/nes/crt0.s
@@ -294,7 +294,29 @@ __crt0_NMI_skip:
     ; Call the timer interrupt for non-graphics events if timer register overflow, and reload TIMA_REG
     inc _TIMA_REG
     bne 3$
+    ; Skip if disabled (check for RTS)
+    lda .jmp_to_TIM_isr
+    cmp #0x60
+    beq 6$
+    ; Save REGTEMP to stack
+    ldx #7
+4$:
+    lda *REGTEMP,x
+    pha
+    dex
+    bpl 4$
+    ; Call handler
     jsr .jmp_to_TIM_isr
+    ; Restore REGTEMP from stack
+    ldx #0
+5$:
+    pla
+    sta *REGTEMP,x
+    inx
+    cpx #8
+    bne 5$
+    ;
+6$:
     lda _TMA_REG
     sta _TIMA_REG
 3$:

This webpage is intended to be an accessible preview of this repository. To get a fuller picture, clone it and use the git CLI.