Add Core1 refresh recovery and shared SPI arbitration

CMakeLists.txt

@@ -48,6 +48,7 @@ add_executable(basic1
     basic1.cpp
     lib/input_manager.cpp
     lib/game_launcher.cpp
+    lib/shared_spi_bus.c
     lib/serial_uploader.cpp
     games/tic_tac_toe.cpp
     games/demo_game.cpp

basic1.cpp

@@ -59,6 +59,7 @@ extern "C" {
 #include "lua_game_loader.h"
 #include "serial_uploader.h"
 #include "scene_stack.h"
+#include "shared_spi_bus.h"
 
 
 // Binary info for RP2350 - ensures proper boot image structure
@@ -75,6 +76,28 @@ volatile bool refresh_requested = false;
 volatile bool refresh_in_progress = false;
 const uint8_t* volatile refresh_buffer = nullptr;
 LowLevelDisplay* volatile refresh_display = nullptr;
+volatile uint32_t core1_heartbeat = 0;
+
+void core1_entry();
+
+static void restart_core1_refresh_worker() {
+    printf("Attempting Core1 restart...\n");
+
+    // Stop Core1 and clear in-flight refresh state.
+    multicore_reset_core1();
+    refresh_requested = false;
+    refresh_in_progress = false;
+    refresh_buffer = nullptr;
+    refresh_display = nullptr;
+    core1_heartbeat = 0;
+
+    // Recover shared SPI lock state after hard core reset.
+    shared_spi_bus_force_recover();
+
+    multicore_launch_core1(core1_entry);
+    sleep_ms(20);
+    printf("Core1 restart complete\n");
+}
 
 /**
  * @brief Core 1 entry point - handles display refresh operations
@@ -86,19 +109,29 @@ void core1_entry() {
     printf("Core 1 started - handling display refreshes\n");
     
     while (1) {
+        core1_heartbeat++;
+
         // Wait for refresh request
-        if (refresh_requested && refresh_buffer && refresh_display) {
+        if (refresh_requested) {
+            if (refresh_buffer && refresh_display) {
                 // refresh_in_progress is already set by Core 0 to lock the buffer
                 
                 // Get local copies for safe access
                 LowLevelDisplay* display = refresh_display;
                 const uint8_t* buffer = refresh_buffer;
                 
-            // Perform the refresh operation (may be slow for e-ink)
+                // Perform refresh with shared SPI bus lock to avoid SD/display collisions.
+                shared_spi_bus_lock();
                 display->draw_buffer(buffer);
                 display->refresh();
+                shared_spi_bus_unlock();
+            } else {
+                // Recovery guard: never leave Core 0 stuck waiting forever if a
+                // malformed/partial request is observed across cores.
+                printf("Core1: dropped malformed refresh request\n");
+            }
 
-            // Clear flags
+            // Clear flags in all cases to avoid deadlock on Core 0.
             refresh_requested = false;
             refresh_in_progress = false; // Unlock buffer for Core 0
         }
@@ -467,6 +500,9 @@ int main()
     printf("\n=== %s Demo ===\n", BOARD_NAME);
     printf("Starting dual-core system...\n");
 
+    // Initialize shared SPI lock before SD/display operations start.
+    shared_spi_bus_init();
+    
     // Create display abstraction using factory method
     // The factory handles all board-specific configuration internally
     LowLevelDisplay* display = LowLevelDisplay::create((DisplayType)DISPLAY_TYPE_SELECTED, V_WIDTH, V_HEIGHT);
@@ -485,11 +521,11 @@ int main()
         return -1;
     }
 
-    // Enable dirty rectangle optimization for ST7796 displays
+    // Enable dirty/partial refresh optimization for ST7796.
     if (display->get_type() == DISPLAY_TYPE_ST7796) {
         LowLevelDisplayST7796* st7796_display = static_cast<LowLevelDisplayST7796*>(display);
         st7796_display->enable_dirty_rect(true);
-        printf("Dirty rectangle optimization enabled (4 quadrants: TL/TR/BL/BR split)\n");
+        printf("Dirty rectangle optimization enabled\n");
     }
 
     // Launch Core 1 for display refresh handling
@@ -607,7 +643,7 @@ int main()
     // Draw launcher menu
     launcher.draw();
     
-    // Refresh the screen with the launcher menu (async on Core 1)
+    // Initial refresh queued on Core 1 (async for all display types in this test mode).
     refresh_screen_async(bit_buffer, display);
     printf("Initial screen refresh queued on Core 1\n");
     
@@ -698,6 +734,10 @@ int main()
     bool needs_refresh = false;  // Track if screen needs redraw
     bool dirty_rect_opt_state = (display->get_type() == DISPLAY_TYPE_ST7796);
     SceneStack scene_stack;
+    bool force_sync_tft_refresh = false;
+    bool core1_restart_attempted = false;
+    uint32_t last_seen_core1_heartbeat = core1_heartbeat;
+    uint32_t last_core1_heartbeat_ms = to_ms_since_boot(get_absolute_time());
     bool swipe_candidate_active = false;
     int16_t swipe_start_x = 0;
     int16_t swipe_start_y = 0;
@@ -705,6 +745,33 @@ int main()
     int16_t swipe_last_y = 0;
 
     while (1) {
+        // Core1 liveness watchdog:
+        // If refresh work is pending/in-progress but Core1 heartbeat stops advancing,
+        // fall back to synchronous TFT refresh on Core0 to avoid frozen UI.
+        uint32_t hb_now = core1_heartbeat;
+        uint32_t now_ms = to_ms_since_boot(get_absolute_time());
+        if (hb_now != last_seen_core1_heartbeat) {
+            last_seen_core1_heartbeat = hb_now;
+            last_core1_heartbeat_ms = now_ms;
+            core1_restart_attempted = false;
+        } else if (!force_sync_tft_refresh &&
+                   (refresh_in_progress || pending_refresh || needs_refresh) &&
+                   (now_ms - last_core1_heartbeat_ms) > 500) {
+            if (!core1_restart_attempted) {
+                core1_restart_attempted = true;
+                restart_core1_refresh_worker();
+                last_seen_core1_heartbeat = core1_heartbeat;
+                last_core1_heartbeat_ms = to_ms_since_boot(get_absolute_time());
+                pending_refresh = true;
+            } else {
+                force_sync_tft_refresh = true;
+                refresh_requested = false;
+                refresh_in_progress = false;
+                pending_refresh = true;
+                printf("Core1 heartbeat stalled after restart; switching TFT refresh to synchronous fallback\n");
+            }
+        }
+
         // 0. Process serial uploads (for rapid game iteration)
         serial_uploader.process(is_refresh_in_progress());
 
@@ -933,15 +1000,9 @@ int main()
             if (time_since_last_frame >= TARGET_FRAME_TIME_MS) {
                 // Only draw if Core 1 is finished with the buffer
                 if (!is_refresh_in_progress()) {
-                    // Update dirty rectangle optimization based on continuous updates
+                    // Keep dirty rectangle optimization enabled for TFT.
                     if (display->get_type() == DISPLAY_TYPE_ST7796) {
-                        bool wants_opt = false;
+                        bool wants_opt = true;
-                        if (scene_stack.is(SceneId::GAME)) {
-                            Game* g = launcher.get_selected_game();
-                            if (g && g->wants_frame_updates()) {
-                                wants_opt = true;
-                            }
-                        }
 
                         if (dirty_rect_opt_state != wants_opt) {
                             LowLevelDisplayST7796* st7796_display = static_cast<LowLevelDisplayST7796*>(display);
@@ -979,16 +1040,13 @@ int main()
                         }
                     }
 
-                    // TFT fix:
-                    // Run refresh synchronously on Core 0 for ST7796/ST7789.
-                    // We observed intermittent frozen screens with async Core 1 refresh
-                    // after SD/Lua activity even though scene/game logic continued.
-                    // E-paper keeps async refresh because its refresh latency is high.
                     bool refresh_started = false;
-                    if (display->get_type() == DISPLAY_TYPE_ST7796 || display->get_type() == DISPLAY_TYPE_ST7789) {
+                    if (force_sync_tft_refresh &&
+                        (display->get_type() == DISPLAY_TYPE_ST7796 || display->get_type() == DISPLAY_TYPE_ST7789)) {
                         refresh_screen(bit_buffer, display);
                         refresh_started = true;
                     } else {
+                        // Async refresh test path.
                         refresh_started = refresh_screen_async(bit_buffer, display);
                     }
 

lib/sd_card/sd_card.c

@@ -5,6 +5,7 @@
 #include "sd_card.h"
 #include "hardware/gpio.h"
 #include "board_config.h"
+#include "shared_spi_bus.h"
 #include "ff.h"  // FatFS
 #include <string.h>
 #include <stdio.h>
@@ -431,6 +432,9 @@ bool sd_card_init_with_board_config(void) {
 uint sd_card_set_spi_speed(void) {
     if (!g_config) return 0;
 
+    // SD file operations run with exclusive ownership of shared SPI bus.
+    shared_spi_bus_lock();
+
     // Save current speed and set to SD card speed
     uint current_speed = spi_get_baudrate(g_config->spi);
     spi_set_baudrate(g_config->spi, 12500 * 1000);  // 12.5 MHz for SD card
@@ -438,10 +442,20 @@ uint sd_card_set_spi_speed(void) {
 }
 
 void sd_card_restore_spi_speed(uint baudrate) {
-    if (!g_config || baudrate == 0) return;
+    if (!g_config) {
+        shared_spi_bus_unlock();
+        return;
+    }
+
+    if (baudrate != 0) {
         spi_set_baudrate(g_config->spi, baudrate);
     }
 
+    // Leave SD deselected before releasing bus.
+    gpio_put(g_config->gpio_cs, 1);
+    shared_spi_bus_unlock();
+}
+
 bool sd_card_test_fatfs(void) {
     if (!g_card_info.initialized) {
         printf("SD Card not initialized\n");

lib/shared_spi_bus.c

@@ -0,0 +1,65 @@
+#include "shared_spi_bus.h"
+#include "pico/mutex.h"
+#include "pico/multicore.h"
+#include <stdbool.h>
+
+static mutex_t g_spi_bus_mutex;
+static bool g_spi_bus_initialized = false;
+static uint32_t g_core_depth[2] = {0, 0};
+
+void shared_spi_bus_init(void) {
+    if (g_spi_bus_initialized) {
+        return;
+    }
+
+    mutex_init(&g_spi_bus_mutex);
+    g_spi_bus_initialized = true;
+}
+
+void shared_spi_bus_lock(void) {
+    if (!g_spi_bus_initialized) {
+        shared_spi_bus_init();
+    }
+
+    int core = get_core_num();
+    if (core < 0 || core > 1) {
+        core = 0;
+    }
+
+    // Re-entrant lock on same core (needed for nested SD helpers).
+    if (g_core_depth[core] > 0) {
+        g_core_depth[core]++;
+        return;
+    }
+
+    mutex_enter_blocking(&g_spi_bus_mutex);
+    g_core_depth[core] = 1;
+}
+
+void shared_spi_bus_unlock(void) {
+    if (!g_spi_bus_initialized) {
+        return;
+    }
+
+    int core = get_core_num();
+    if (core < 0 || core > 1) {
+        core = 0;
+    }
+
+    if (g_core_depth[core] == 0) {
+        return;
+    }
+
+    g_core_depth[core]--;
+    if (g_core_depth[core] == 0) {
+        mutex_exit(&g_spi_bus_mutex);
+    }
+}
+
+void shared_spi_bus_force_recover(void) {
+    // Used after Core1 reset to avoid stale mutex/depth state.
+    mutex_init(&g_spi_bus_mutex);
+    g_core_depth[0] = 0;
+    g_core_depth[1] = 0;
+    g_spi_bus_initialized = true;
+}

lib/shared_spi_bus.h

@@ -0,0 +1,18 @@
+#ifndef SHARED_SPI_BUS_H
+#define SHARED_SPI_BUS_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// Cross-core SPI bus lock for shared SD/display SPI usage.
+void shared_spi_bus_init(void);
+void shared_spi_bus_lock(void);
+void shared_spi_bus_unlock(void);
+void shared_spi_bus_force_recover(void);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif // SHARED_SPI_BUS_H