From e2460b546e5c2634c1a13895fca136e76193500e Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Fri, 31 Jul 2026 05:49:39 -0500 Subject: [PATCH] MUI: stop holding the SPI bus across the whole UI cycle (#11278) tft_task_handler held spiLock for the entire LVGL cycle. Most of that cycle is timer work and rendering into the draw buffer, which issues no SPI at all - but on boards where the TFT, SD card and LoRa radio share one bus (T-Deck), every radio operation on the main loop still waited it out. That is tens to hundreds of milliseconds whenever the UI animates, felt as mesh RX/TX latency. device-ui now takes the lock around its own transfers instead (meshtastic/device-ui#356), so the coarse hold here can go and the bus is contended only during real traffic. Lend it spiLock through a reentrant adapter. device-ui nests its guards - SdFsCard::usedBytes() calls cardSize() and freeBytes(), each of which takes the lock - while spiLock is a plain binary semaphore that would self-deadlock on the second take, so track the owning task and only touch the underlying lock on the outermost acquire. Requires the device-ui pin bump included here. Tested on T-Deck: flush, touch, panel init, SD detect, powersave sleep and wake all exercised; LoRa RX decoding under a live UI, no deadlocks, no watchdog resets. Also builds seeed-sensecap-indicator-tft. Co-authored-by: Manuel <71137295+mverch67@users.noreply.github.com> --- platformio.ini | 2 +- src/graphics/tftSetup.cpp | 72 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/platformio.ini b/platformio.ini index 040da932c..c357bc1f0 100644 --- a/platformio.ini +++ b/platformio.ini @@ -128,7 +128,7 @@ lib_deps = [device-ui_base] lib_deps = # renovate: datasource=git-refs depName=meshtastic/device-ui packageName=https://github.com/meshtastic/device-ui gitBranch=master - https://github.com/meshtastic/device-ui/archive/bcb327f058469282408eed93e27249d6447de9c0.zip + https://github.com/meshtastic/device-ui/archive/85bd4ff2210f7d431cbd496db691d6c19c5018a2.zip ; Common libs for environmental measurements in telemetry module [environmental_base] diff --git a/src/graphics/tftSetup.cpp b/src/graphics/tftSetup.cpp index 95b721e67..0ab868f94 100644 --- a/src/graphics/tftSetup.cpp +++ b/src/graphics/tftSetup.cpp @@ -8,9 +8,13 @@ #include "comms/PacketServer.h" #include "graphics/DeviceScreen.h" #include "graphics/driver/DisplayDriverConfig.h" +#include "util/ISpiLock.h" #ifdef ARCH_PORTDUINO #include "PortduinoGlue.h" +#endif + +#if defined(ARCH_PORTDUINO) || !defined(HAS_FREE_RTOS) #include #include #include @@ -30,12 +34,72 @@ CallbackObserver endSleepObserver = CallbackObserver(deviceScreen, &DeviceScreen::wakeUp); #endif +/** + * Lends spiLock to device-ui so it can guard its own bus traffic. + * + * This used to be a coarse hold around the whole UI cycle in tft_task_handler(). On + * boards where the TFT, SD card and LoRa radio share one SPI bus (T-Deck), that blocked + * every radio operation on the main loop for an entire render+flush - tens to hundreds + * of milliseconds while the UI animates, felt as mesh RX/TX latency. Most of that time + * is LVGL timer work and rendering into the draw buffer, which touches no SPI at all. + * + * device-ui now takes the lock only around real transfers, so the bus is free during the + * CPU-only majority of a redraw. + * + * Reentrant because ISpiLock requires it: device-ui guards at method granularity, so a + * caller that already holds the bus can reach another guarded method, while spiLock is a + * plain binary semaphore that would self-deadlock on the second take. Track the owning + * thread and only touch the underlying lock on the outermost acquire. + * + * Only the owner writes owner/depth, and only while holding the lock, so no additional + * synchronization is needed: a thread that does not own it either reads some other + * thread's id or the unowned sentinel, and in both cases correctly goes on to block. + */ +class ReentrantSpiLock : public ISpiLock +{ + public: + void lock(void) override + { + ThreadId self = currentThread(); + if (depth && owner == self) { + depth++; + return; + } + spiLock->lock(); + owner = self; + depth = 1; + } + + void unlock(void) override + { + if (--depth == 0) { + owner = ThreadId(); + spiLock->unlock(); + } + } + + private: + // Portduino builds have no FreeRTOS, but device-ui runs there too, so the + // reentrancy has to be portable rather than an ESP32-only path. +#ifdef HAS_FREE_RTOS + using ThreadId = TaskHandle_t; + static ThreadId currentThread(void) { return xTaskGetCurrentTaskHandle(); } +#else + using ThreadId = std::thread::id; + static ThreadId currentThread(void) { return std::this_thread::get_id(); } +#endif + + ThreadId owner = ThreadId(); + uint32_t depth = 0; +}; + +static ReentrantSpiLock reentrantSpiLock; + void tft_task_handler(void *param = nullptr) { while (true) { - spiLock->lock(); + // No lock held here on purpose - device-ui guards its own SPI access. deviceScreen->task_handler(); - spiLock->unlock(); deviceScreen->sleep(); } } @@ -43,7 +107,7 @@ void tft_task_handler(void *param = nullptr) void tftSetup(void) { #ifndef ARCH_PORTDUINO - deviceScreen = &DeviceScreen::create(); + deviceScreen = &DeviceScreen::create(reentrantSpiLock); PacketAPI::create(PacketServer::init()); deviceScreen->init(new PacketClient); #else @@ -126,7 +190,7 @@ void tftSetup(void) .i2c{.i2c_addr = (uint8_t)portduino_config.touchscreenI2CAddr}}); } } - deviceScreen = &DeviceScreen::create(&displayConfig); + deviceScreen = &DeviceScreen::create(&displayConfig, &reentrantSpiLock); PacketAPI::create(PacketServer::init()); deviceScreen->init(new PacketClient); } else {