From cf126d109bbb0ab7229ca0b6344bc4461d35f922 Mon Sep 17 00:00:00 2001 From: Brendan Fletcher Date: Wed, 10 Jul 2024 18:04:18 -0400 Subject: [PATCH] Make emu_keypad_event thread-safe, improve CPU core response to external events. Fixes #486. --- core/atomics.h | 60 +++++++++++++++++++- core/cpu.c | 22 ++++---- core/cpu.h | 14 +++-- core/debug/debug.c | 6 +- core/emu.c | 15 +++-- core/keypad.c | 97 ++++++++++++++++++++------------ core/keypad.h | 11 ++-- gui/qt/emuthread.cpp | 2 +- gui/qt/keypad/qtkeypadbridge.cpp | 3 - 9 files changed, 160 insertions(+), 70 deletions(-) diff --git a/core/atomics.h b/core/atomics.h index c0df5a01f..0a5d5be42 100644 --- a/core/atomics.h +++ b/core/atomics.h @@ -6,9 +6,41 @@ #ifndef __cplusplus #if !defined(__STDC_NO_ATOMICS__) || (defined(_MSC_VER) && _MSC_VER >= 1935) #include + #define atomic8_fetch_and atomic_fetch_and + #define atomic32_fetch_and atomic_fetch_and + #define atomic8_fetch_and_explicit atomic_fetch_and_explicit + #define atomic32_fetch_and_explicit atomic_fetch_and_explicit + #define atomic8_fetch_or atomic_fetch_or + #define atomic32_fetch_or atomic_fetch_or + #define atomic8_fetch_or_explicit atomic_fetch_or_explicit + #define atomic32_fetch_or_explicit atomic_fetch_or_explicit #else #define _Atomic(X) volatile X /* doesn't do anything, but makes me feel better... although if you are trying to do multithreading glhf */ - #define atomic_compare_exchange_strong(object, expected, desired) (*(object) == *(expected) ? (*(object) = (desired)) : (*(expected) = (desired))) + #define atomic_load_explicit(object, order) (*(object)) +static inline atomic8_fetch_and(volatile uint8_t *obj, uint8_t arg) { + uint8_t result = *obj; + *obj = result & arg; + return result; +} +static inline atomic32_fetch_and(volatile uint32_t *obj, uint8_t arg) { + uint32_t result = *obj; + *obj = result & arg; + return result; +} +static inline atomic8_fetch_or(volatile uint8_t *obj, uint8_t arg) { + uint8_t result = *obj; + *obj = result | arg; + return result; +} +static inline atomic32_fetch_or(volatile uint32_t *obj, uint8_t arg) { + uint32_t result = *obj; + *obj = result | arg; + return result; +} + #define atomic8_fetch_and_explicit(object, arg, order) atomic8_fetch_and(object, arg) + #define atomic32_fetch_and_explicit(object, arg, order) atomic32_fetch_and(object, arg) + #define atomic8_fetch_or_explicit(object, arg, order) atomic8_fetch_or(object, arg) + #define atomic32_fetch_or_explicit(object, arg, order) atomic32_fetch_or(object, arg) #endif #else #include @@ -16,7 +48,31 @@ #endif #else #define _Atomic(X) X - #define atomic_compare_exchange_strong(object, expected, desired) (*(object) == *(expected) ? (*(object) = (desired)) : (*(expected) = (desired))) + #define atomic_load_explicit(object, order) (*(object)) +static inline atomic8_fetch_and(uint8_t *obj, uint8_t arg) { + uint8_t result = *obj; + *obj = result & arg; + return result; +} +static inline atomic32_fetch_and(uint32_t *obj, uint8_t arg) { + uint32_t result = *obj; + *obj = result & arg; + return result; +} +static inline atomic8_fetch_or(uint8_t *obj, uint8_t arg) { + uint8_t result = *obj; + *obj = result | arg; + return result; +} +static inline atomic32_fetch_or(uint32_t *obj, uint8_t arg) { + uint32_t result = *obj; + *obj = result | arg; + return result; +} + #define atomic8_fetch_and_explicit(object, arg, order) atomic8_fetch_and(object, arg) + #define atomic32_fetch_and_explicit(object, arg, order) atomic32_fetch_and(object, arg) + #define atomic8_fetch_or_explicit(object, arg, order) atomic8_fetch_or(object, arg) + #define atomic32_fetch_or_explicit(object, arg, order) atomic32_fetch_or(object, arg) #endif #endif diff --git a/core/cpu.c b/core/cpu.c index cbf2a09d5..e571396ff 100644 --- a/core/cpu.c +++ b/core/cpu.c @@ -814,7 +814,7 @@ static void cpu_execute_bli() { void cpu_init(void) { memset(&cpu, 0, sizeof(eZ80cpu_t)); - atomics.abort = CPU_ABORT_NONE; + atomics.abort = CPU_ABORT_ON_KEY | CPU_ABORT_ANY_KEY; gui_console_printf("[CEmu] Initialized CPU...\n"); } @@ -841,20 +841,20 @@ void cpu_nmi(void) { #endif } -void cpu_transition_abort(uint8_t from, uint8_t to) { - atomic_compare_exchange_strong(&atomics.abort, &from, to); +void cpu_request_abort(uint8_t abort) { + atomic8_fetch_or_explicit(&atomics.abort, abort, memory_order_release); } -uint8_t cpu_check_abort(void) { - return atomics.abort; +uint8_t cpu_check_aborts(void) { + return atomic_load_explicit(&atomics.abort, memory_order_relaxed); } -void cpu_exit(void) { - atomics.abort = CPU_ABORT_EXIT; +uint8_t cpu_clear_aborts(void) { + return atomic8_fetch_and_explicit(&atomics.abort, CPU_ABORT_EXIT, memory_order_acquire); } void cpu_crash(const char *msg) { - cpu_transition_abort(CPU_ABORT_NONE, CPU_ABORT_RESET); + cpu_request_abort(CPU_ABORT_RESET); gui_console_printf("[CEmu] Reset caused by %s.\n", msg); #ifdef DEBUG_SUPPORT if (debug_get_flags() & DBG_OPEN_ON_RESET) { @@ -880,7 +880,7 @@ static void cpu_halt(void) { } void cpu_restore_next(void) { - if (cpu.NMI || (cpu.IEF1 && (intrpt->status & intrpt->enabled)) || atomics.abort != CPU_ABORT_NONE) { + if (cpu.NMI || (cpu.IEF1 && (intrpt->status & intrpt->enabled)) || cpu_check_aborts() != CPU_ABORT_NONE) { cpu.next = 0; /* always applies, even after cycle rewind during port writes */ } else if (cpu.IEF_wait) { cpu.next = cpu.eiDelay; /* execute one instruction */ @@ -943,7 +943,7 @@ void cpu_execute(void) { } else { cpu_restore_next(); } - if (cpu.cycles >= cpu.next || atomics.abort != CPU_ABORT_NONE) { + if (cpu.cycles >= cpu.next) { break; } if (cpu.inBlock) { @@ -1135,7 +1135,7 @@ void cpu_execute(void) { break; } if (context.z < 4) { - if (cpu.SUFFIX && atomics.abort != CPU_ABORT_NONE) { /* suffix can infinite loop */ + if (cpu.SUFFIX && (cpu_check_aborts() & (CPU_ABORT_RESET | CPU_ABORT_EXIT))) { /* suffix can infinite loop */ return; } cpu.L = context.s; diff --git a/core/cpu.h b/core/cpu.h index eb39a954e..73afc4145 100644 --- a/core/cpu.h +++ b/core/cpu.h @@ -35,9 +35,11 @@ extern "C" { #define cpu_mask_mode(address, mode) ((uint32_t)((address) & ((mode) ? 0xFFFFFF : 0xFFFF))) typedef enum eZ80abort { - CPU_ABORT_NONE, - CPU_ABORT_RESET, - CPU_ABORT_EXIT, + CPU_ABORT_NONE = 0, + CPU_ABORT_EXIT = 1 << 0, + CPU_ABORT_RESET = 1 << 1, + CPU_ABORT_ON_KEY = 1 << 2, + CPU_ABORT_ANY_KEY = 1 << 3 } eZ80abort_t; typedef union eZ80context { @@ -112,9 +114,9 @@ void cpu_flush(uint32_t, bool); void cpu_nmi(void); void cpu_execute(void); void cpu_restore_next(void); -void cpu_transition_abort(uint8_t from, uint8_t to); -uint8_t cpu_check_abort(void); -void cpu_exit(void); +void cpu_request_abort(uint8_t abort); +uint8_t cpu_check_aborts(void); +uint8_t cpu_clear_aborts(void); void cpu_crash(const char *msg); bool cpu_restore(FILE *image); bool cpu_save(FILE *image); diff --git a/core/debug/debug.c b/core/debug/debug.c index 3a2764b30..45fc9d5d0 100644 --- a/core/debug/debug.c +++ b/core/debug/debug.c @@ -41,15 +41,15 @@ void debug_free(void) { } bool debug_is_open(void) { - return atomics.open; + return atomic_load_explicit(&atomics.open, memory_order_relaxed); } int debug_get_flags(void) { - return atomics.flags; + return atomic_load_explicit(&atomics.flags, memory_order_relaxed); } void debug_open(int reason, uint32_t data) { - if (cpu_check_abort() == CPU_ABORT_EXIT || atomics.open || ((atomics.flags & DBG_IGNORE) && (reason >= DBG_BREAKPOINT && reason <= DBG_PORT_WRITE))) { + if ((cpu_check_aborts() & CPU_ABORT_EXIT) || debug_is_open() || ((debug_get_flags() & DBG_IGNORE) && (reason >= DBG_BREAKPOINT && reason <= DBG_PORT_WRITE))) { return; } diff --git a/core/emu.c b/core/emu.c index 27a2e8c67..dd866e523 100644 --- a/core/emu.c +++ b/core/emu.c @@ -3,6 +3,7 @@ #include "asic.h" #include "cpu.h" #include "cert.h" +#include "keypad.h" #include "os/os.h" #include "defines.h" #include "schedule.h" @@ -20,7 +21,7 @@ #define IMAGE_VERSION 0xCECE0017 void EMSCRIPTEN_KEEPALIVE emu_exit(void) { - cpu_exit(); + cpu_request_abort(CPU_ABORT_EXIT); } void EMSCRIPTEN_KEEPALIVE emu_reset(void) { @@ -239,12 +240,18 @@ emu_state_t emu_load(emu_data_t type, const char *path) { } void emu_run(uint64_t ticks) { + uint8_t aborts; sched.run_event_triggered = false; sched_repeat(SCHED_RUN, ticks); - while (cpu_check_abort() != CPU_ABORT_EXIT) { + while (!((aborts = cpu_clear_aborts()) & CPU_ABORT_EXIT)) { + if (aborts & CPU_ABORT_ON_KEY) { + keypad_on_check(); + } + if (aborts & CPU_ABORT_ANY_KEY) { + keypad_any_check(); + } sched_process_pending_events(); - if (cpu_check_abort() == CPU_ABORT_RESET) { - cpu_transition_abort(CPU_ABORT_RESET, CPU_ABORT_NONE); + if (aborts & CPU_ABORT_RESET) { gui_console_printf("[CEmu] Reset triggered.\n"); asic_reset(); #ifdef DEBUG_SUPPORT diff --git a/core/keypad.c b/core/keypad.c index c9a2df15a..c6b693c93 100644 --- a/core/keypad.c +++ b/core/keypad.c @@ -1,4 +1,5 @@ #include "keypad.h" +#include "atomics.h" #include "defines.h" #include "emu.h" #include "schedule.h" @@ -10,25 +11,49 @@ #include #include +#define ON_KEY_PRESSED 1 +#define ON_KEY_EDGE 2 + /* Global KEYPAD state */ keypad_state_t keypad; -void keypad_intrpt_check() { - intrpt_set(INT_KEYPAD, (keypad.status & keypad.enable) | (keypad.gpioStatus & keypad.gpioEnable)); +typedef struct keypad_atomics { + _Atomic(uint32_t) keyMap[KEYPAD_MAX_ROWS]; + _Atomic(uint8_t) onKey; +} keypad_atomics_t; + +static keypad_atomics_t atomics; + +static inline void keypad_intrpt_check() { + intrpt_set(INT_KEYPAD, keypad.status & keypad.enable); +} + +static inline uint8_t keypad_row_limit() { + return keypad.rows >= KEYPAD_MAX_ROWS ? KEYPAD_MAX_ROWS : keypad.rows; } -static void keypad_any_check(void) { - uint8_t any = 0; - unsigned int row; +static inline uint16_t keypad_data_mask() { + uint8_t colLimit = keypad.cols >= KEYPAD_MAX_COLS ? KEYPAD_MAX_COLS : keypad.cols; + return (1 << colLimit) - 1; +} + +static inline uint16_t keypad_query_keymap(uint8_t row) { + uint32_t data = atomic32_fetch_and_explicit(&atomics.keyMap[row], (1 << KEYPAD_MAX_COLS) - 1, memory_order_relaxed); + return (data | data >> KEYPAD_MAX_COLS) & ((1 << KEYPAD_MAX_COLS) - 1); +} + +void keypad_any_check(void) { + uint8_t row; if (keypad.mode != 1) { return; } - for (row = 0; row < keypad.rows && row < sizeof(keypad.data) / sizeof(keypad.data[0]); row++) { - any |= keypad.keyMap[row] | keypad.delay[row]; - keypad.delay[row] = 0; + uint16_t any = 0; + uint8_t rowLimit = keypad_row_limit(); + for (row = 0; row < rowLimit; row++) { + any |= keypad_query_keymap(row); } - any &= (1 << keypad.cols) - 1; - for (row = 0; row < keypad.rows && row < sizeof(keypad.data) / sizeof(keypad.data[0]); row++) { + any &= keypad_data_mask(); + for (row = 0; row < rowLimit; row++) { keypad.data[row] = any; } if (any) { @@ -37,23 +62,34 @@ static void keypad_any_check(void) { } } +void keypad_on_check(void) { + uint8_t onState = atomic8_fetch_and_explicit(&atomics.onKey, ON_KEY_PRESSED, memory_order_relaxed); + intrpt_set(INT_ON, onState); + if (onState == ON_KEY_EDGE) { + intrpt_set(INT_ON, false); + } + if ((onState & ON_KEY_EDGE) && control.off) { + control.readBatteryStatus = ~1; + control.off = false; + intrpt_pulse(INT_WAKE); + } +} + void EMSCRIPTEN_KEEPALIVE emu_keypad_event(unsigned int row, unsigned int col, bool press) { if (row == 2 && col == 0) { - intrpt_set(INT_ON, press); - if (press && control.off) { - control.readBatteryStatus = ~1; - control.off = false; - intrpt_pulse(INT_WAKE); + if (press) { + atomic8_fetch_or_explicit(&atomics.onKey, ON_KEY_EDGE | ON_KEY_PRESSED, memory_order_relaxed); + } else { + atomic8_fetch_and_explicit(&atomics.onKey, ~ON_KEY_PRESSED, memory_order_relaxed); } - } else { + cpu_request_abort(CPU_ABORT_ON_KEY); + } else if (row < KEYPAD_MAX_ROWS && col < KEYPAD_MAX_COLS) { if (press) { - keypad.keyMap[row] |= 1 << col; - keypad.delay[row] |= 1 << col; + atomic32_fetch_or_explicit(&atomics.keyMap[row], (1 | 1 << KEYPAD_MAX_COLS) << col, memory_order_relaxed); + cpu_request_abort(CPU_ABORT_ANY_KEY); } else { - keypad.keyMap[row] &= ~(1 << col); - keypad_intrpt_check(); + atomic32_fetch_and_explicit(&atomics.keyMap[row], ~(1 << col), memory_order_relaxed); } - keypad_any_check(); } } @@ -83,9 +119,7 @@ static uint8_t keypad_read(const uint16_t pio, bool peek) { case 0x10: value = read8(keypad.gpioEnable, bit_offset); break; - case 0x11: - value = read8(keypad.gpioStatus, bit_offset); - break; + case 0x11: /* GPIO status is always 0 */ default: break; } @@ -97,10 +131,9 @@ static uint8_t keypad_read(const uint16_t pio, bool peek) { /* Scan next row of keypad, if scanning is enabled */ static void keypad_scan_event(enum sched_item_id id) { uint8_t row = keypad.row++; - if (row < keypad.rows && row < sizeof(keypad.data) / sizeof(keypad.data[0])) { + if (row < keypad_row_limit()) { /* scan each data row */ - uint16_t data = (keypad.keyMap[row] | keypad.delay[row]) & ((1 << keypad.cols) - 1); - keypad.delay[row] = 0; + uint16_t data = keypad_query_keymap(row) & keypad_data_mask(); /* if mode 3 or 2, generate data change interrupt */ if (keypad.data[row] != data) { @@ -154,17 +187,13 @@ static void keypad_write(const uint16_t pio, const uint8_t byte, bool poke) { case 0x08: case 0x09: case 0x0A: case 0x0B: if (poke) { write8(keypad.data[(pio - 0x10) >> 1 & 15], pio << 3 & 8, byte); - write8(keypad.keyMap[pio >> 1 & 15], pio << 3 & 8, byte); + write8(atomics.keyMap[pio >> 1 & 15], pio << 3 & 8, byte); } break; case 0x10: write8(keypad.gpioEnable, bit_offset, byte); - keypad_intrpt_check(); - break; - case 0x11: - write8(keypad.gpioStatus, bit_offset, keypad.gpioStatus >> bit_offset & ~byte); - keypad_intrpt_check(); break; + case 0x11: /* GPIO status is always 0, no 1 bits to reset */ default: break; /* Escape write sequence if unimplemented */ } @@ -187,8 +216,6 @@ eZ80portrange_t init_keypad(void) { keypad.row = 0; memset(keypad.data, 0, sizeof(keypad.data)); - memset(keypad.keyMap, 0, sizeof(keypad.keyMap)); - memset(keypad.delay, 0, sizeof(keypad.delay)); gui_console_printf("[CEmu] Initialized Keypad...\n"); return device; diff --git a/core/keypad.h b/core/keypad.h index ac32302eb..5026aea32 100644 --- a/core/keypad.h +++ b/core/keypad.h @@ -12,6 +12,9 @@ extern "C" { #include #include +#define KEYPAD_MAX_COLS 16 +#define KEYPAD_MAX_ROWS 16 + typedef struct keypad_state { union { struct { @@ -36,17 +39,15 @@ typedef struct keypad_state { uint8_t row; uint8_t status; uint8_t enable; - uint16_t data[16]; - uint16_t keyMap[16]; - uint16_t delay[16]; - uint32_t gpioStatus; + uint16_t data[KEYPAD_MAX_ROWS]; uint32_t gpioEnable; } keypad_state_t; extern keypad_state_t keypad; eZ80portrange_t init_keypad(void); -void keypad_intrpt_check(void); +void keypad_any_check(void); +void keypad_on_check(void); void keypad_reset(void); bool keypad_restore(FILE *image); bool keypad_save(FILE *image); diff --git a/gui/qt/emuthread.cpp b/gui/qt/emuthread.cpp index ca09bce0a..7205e1e0e 100644 --- a/gui/qt/emuthread.cpp +++ b/gui/qt/emuthread.cpp @@ -60,7 +60,7 @@ EmuThread::EmuThread(QObject *parent) : QThread{parent}, write{CONSOLE_BUFFER_SI } void EmuThread::run() { - while (cpu_check_abort() != CPU_ABORT_EXIT) { + while (!(cpu_check_aborts() & CPU_ABORT_EXIT)) { emu_run(1u); doStuff(); throttleWait(); diff --git a/gui/qt/keypad/qtkeypadbridge.cpp b/gui/qt/keypad/qtkeypadbridge.cpp index e2a22f397..202293e15 100644 --- a/gui/qt/keypad/qtkeypadbridge.cpp +++ b/gui/qt/keypad/qtkeypadbridge.cpp @@ -83,9 +83,6 @@ void QtKeypadBridge::skEvent(QKeyEvent *event, bool press) { } } } - - keypad.gpioEnable |= 0x800; - keypad_intrpt_check(); } void QtKeypadBridge::kEvent(QString text, int key, bool repeat) {