diff --git a/components/esp32-p4-function-ev-board/src/video.cpp b/components/esp32-p4-function-ev-board/src/video.cpp index c8f4beb65a..e1d998490a 100644 --- a/components/esp32-p4-function-ev-board/src/video.cpp +++ b/components/esp32-p4-function-ev-board/src/video.cpp @@ -148,9 +148,11 @@ bool Esp32P4FunctionEvBoard::initialize_lcd() { // (esp_lcd_panel_draw_bitmap) through it corrupts the RGB565 channel order on // this board — colors come out brighter/greener and alpha blends render // wrong, while the bytes in the frame buffer are correct. The plain CPU copy - // path renders correctly and matches the m5stack-tab5 BSP, which also does - // not enable DMA2D on IDF >= 6. (An earlier "blank screen without DMA2D" was - // actually the RST_LCD/PWM jumper wiring, not DMA2D.) + // path renders correctly and matches the m5stack-tab5 BSP's ILI9881 variant, + // which skips DMA2D on IDF >= 6 for the same reason (its ST71xx variants + // enable it without issue, as does the esp32-p4-nano's JD9365). (An earlier + // "blank screen without DMA2D" was actually the RST_LCD/PWM jumper wiring, + // not DMA2D.) } // Send the panel controller's vendor init sequence over DBI (command mode), diff --git a/components/m5stack-tab5/Kconfig b/components/m5stack-tab5/Kconfig index b76c0eade4..fe8beb41a4 100644 --- a/components/m5stack-tab5/Kconfig +++ b/components/m5stack-tab5/Kconfig @@ -27,4 +27,18 @@ menu "M5Stack Tab5 Configuration" help Size of the stack used for the audio processing task. + config M5STACK_TAB5_ST7121_HW_ROTATION + bool "ST7121: apply 0/180 rotation in the panel (EXPERIMENTAL)" + default n + help + Route 0/180-degree display rotation to the ST7121 panel itself (MADCTL + GS/SS scan-direction flip) instead of the PPA/software rotation in the + flush path. EXPERIMENTAL: verified NOT working on at least some ST7121 + Tab5 units - the 180-degree orientation renders corrupted, most likely + because the TDDI gate/source mux tables programmed at init (command + 0xAC block) are matched to the normal scan direction and a MADCTL GS + flip alone reorders gate scanning without swapping them. Leave disabled + (the default) to keep the known-good PPA rotation for all orientations; + enable only to experiment on your unit. + endmenu diff --git a/components/m5stack-tab5/include/m5stack-tab5.hpp b/components/m5stack-tab5/include/m5stack-tab5.hpp index 669ecffb30..9aa3a247e8 100644 --- a/components/m5stack-tab5/include/m5stack-tab5.hpp +++ b/components/m5stack-tab5/include/m5stack-tab5.hpp @@ -23,6 +23,7 @@ #include #include +#include #include #include @@ -285,6 +286,10 @@ class M5StackTab5 : public BaseComponent { /// \note This method queues the panel transfer asynchronously and may return /// before the write has completed. void write_lcd_lines(int xs, int ys, int xe, int ye, const uint8_t *data, uint32_t user_data); + // Issue one draw_bitmap and block until the DPI copy completes (or a + // bounded timeout). Caller must hold panel_op_mutex_. Returns false if the + // draw was rejected/failed (no completion will arrive). + bool draw_and_wait(int x1, int y1, int x2, int y2, const void *data); ///////////////////////////////////////////////////////////////////////////// // Audio System @@ -944,6 +949,44 @@ class M5StackTab5 : public BaseComponent { esp_lcd_panel_handle_t panel{nullptr}; // color handle } lcd_handles_{}; + // Publication gate for the LCD state that flush() / write_lcd_lines() / + // on_display_rotation() read from other threads (lcd_handles_, + // dpi_framebuffer_ + dpi_framebuffer_bytes_, display_driver_, + // display_controller_). Those fields are plain (non-atomic) and are written + // by initialize_lcd() on the init thread; if the LVGL display already exists + // (initialize_display() called first) the LVGL thread can be flushing + // concurrently, so the readers must not touch them until they are all + // written. initialize_lcd() only runs while this flag is false (it refuses + // to re-initialize once the gate has opened — clearing the flag would not + // wait for readers that already observed true), writes every field, applies + // the initial panel rotation, and store-releases it true as its final + // publication step; the readers load-acquire it and bail out while it is + // false. The release/acquire pair makes all of the writes happen-before any + // read that observes true, and the flag never transitions true -> false. + std::atomic lcd_initialized_{false}; + + // Serializes every panel draw. esp_lcd_panel_draw_bitmap() is asynchronous and + // single-flight when the DMA2D hook is enabled (a second call while one is in + // flight returns ESP_ERR_INVALID_STATE), and flush() and the public, + // cross-thread write_lcd_lines() both issue draws. Holding this mutex across + // the draw AND its completion wait means only one transfer is ever in flight, + // so a direct write cannot make an LVGL flush's draw fail (which would leave + // LVGL waiting forever) and a direct write's completion cannot be mistaken for + // an LVGL flush completion. + std::mutex panel_op_mutex_; + // Signalled from the on_color_trans_done ISR for each completed draw; the + // issuing draw (under panel_op_mutex_) waits on it, making draws synchronous. + SemaphoreHandle_t draw_done_sem_{nullptr}; + + // The DPI panel's (PSRAM) framebuffer, queried from esp_lcd once the panel + // is created. flush() uses it to rotate LVGL draw buffers directly into the + // scanned-out framebuffer with the PPA, skipping the intermediate scratch + // buffer + draw_bitmap copy (which doubles the PSRAM traffic and can starve + // the DSI scan-out DMA into FIFO underruns / on-screen streaking). Null when + // unavailable, in which case flush() falls back to the scratch-buffer path. + void *dpi_framebuffer_{nullptr}; + size_t dpi_framebuffer_bytes_{0}; + // Display controller detection DisplayController display_controller_{DisplayController::UNKNOWN}; @@ -952,6 +995,22 @@ class M5StackTab5 : public BaseComponent { esp_err_t (*original_panel_init_)(esp_lcd_panel_t *panel){nullptr}; void flush(lv_display_t *disp, const lv_area_t *area, uint8_t *px_map); + // Called by espp::Display (LV_EVENT_RESOLUTION_CHANGED) whenever the LVGL + // display rotation changes; routes the rotation to the display driver + // (MADCTL) when the active panel can honor it in hardware. + void on_display_rotation(const DisplayRotation &rotation); + // Gate-free core of on_display_rotation(): routes the rotation to the + // display driver (MADCTL) when the active panel honors it in hardware. + // Callers must guarantee display_driver_ / display_controller_ are safe to + // read: on_display_rotation() does so via its lcd_initialized_ acquire + // load; initialize_lcd() calls this directly on the init thread (which + // wrote those fields) BEFORE opening the gate, so the initial scan + // direction is programmed before any flush() can skip the PPA rotation. + void apply_panel_rotation(const DisplayRotation &rotation); + // Whether the active display controller applies the given LVGL rotation in + // panel hardware (via the display driver's set_rotation()/MADCTL), making + // buffer rotation (PPA / software) in flush() unnecessary. + bool panel_handles_rotation(lv_display_rotation_t rotation) const; static bool notify_lvgl_flush_ready(esp_lcd_panel_handle_t panel, esp_lcd_dpi_panel_event_data_t *edata, void *user_ctx); diff --git a/components/m5stack-tab5/src/camera.cpp b/components/m5stack-tab5/src/camera.cpp index 276cae81e8..7fe01fb50f 100644 --- a/components/m5stack-tab5/src/camera.cpp +++ b/components/m5stack-tab5/src/camera.cpp @@ -245,6 +245,12 @@ bool M5StackTab5::initialize_camera(const camera_frame_callback_t &callback, // hardware pass. The callback receives this preview buffer. ppa_client_config_t ppa_cfg = {}; ppa_cfg.oper_type = PPA_OPERATION_SRM; + // Throttle the PPA's AXI bursts (default 128 bytes): full-length PPA bursts + // against PSRAM are known to starve the DSI panel's continuous framebuffer + // scan-out DMA and underrun its FIFO, streaking the display (see the display + // PPA client in video.cpp and lvgl/lvgl#9590). The camera PPA runs every + // frame concurrently with display flushes, so keep its bursts short too. + ppa_cfg.data_burst_length = PPA_DATA_BURST_LENGTH_64; if (ppa_register_client(&ppa_cfg, &camera_ppa_client_) != ESP_OK) { logger_.error("Could not register the camera PPA client"); stop_camera(); diff --git a/components/m5stack-tab5/src/video.cpp b/components/m5stack-tab5/src/video.cpp old mode 100755 new mode 100644 index 719efe1092..afdb5d47b1 --- a/components/m5stack-tab5/src/video.cpp +++ b/components/m5stack-tab5/src/video.cpp @@ -11,7 +11,10 @@ #include #include +#include + #include +#include #include #include #include @@ -21,6 +24,74 @@ using namespace std::chrono_literals; namespace espp { +// espp::DisplayRotation and lv_display_rotation_t enumerate the same four +// quarter-turns in the same order, and espp::Display itself converts between +// them by value (static_cast, see display.hpp). The rotation logic below leans +// on that correspondence — flush() keys off the LVGL enum while +// on_display_rotation() receives the espp one — so pin the mapping down +// explicitly and convert through these helpers only. +static_assert(static_cast(DisplayRotation::LANDSCAPE) == LV_DISPLAY_ROTATION_0 && + static_cast(DisplayRotation::PORTRAIT) == LV_DISPLAY_ROTATION_90 && + static_cast(DisplayRotation::LANDSCAPE_INVERTED) == + LV_DISPLAY_ROTATION_180 && + static_cast(DisplayRotation::PORTRAIT_INVERTED) == LV_DISPLAY_ROTATION_270, + "espp::DisplayRotation and lv_display_rotation_t must stay value-compatible"); +static constexpr lv_display_rotation_t to_lv_rotation(DisplayRotation rotation) { + return static_cast(rotation); +} +static constexpr DisplayRotation to_display_rotation(lv_display_rotation_t rotation) { + return static_cast(rotation); +} + +// The entire video path is RGB565-only: the DPI panel is configured for +// RGB565 in initialize_lcd(), flush()'s PPA rotation uses +// PPA_SRM_COLOR_MODE_RGB565 for both its input and output, and every buffer +// is sized in sizeof(Pixel) = 2-byte pixels. LVGL must therefore render +// RGB565 as well (LV_COLOR_DEPTH 16) — with any other color depth the flush +// callback would receive pixels these fixed color modes and sizes +// misinterpret (wrong colors at best, buffer overruns at worst). Refuse to +// build such a configuration instead of failing at runtime; supporting +// another depth means plumbing the active format through the DPI config, the +// PPA color modes, and the buffer sizing together. +static_assert(LV_COLOR_DEPTH == 16 && sizeof(M5StackTab5::Pixel) == sizeof(uint16_t), + "The Tab5 video path (DPI panel config, PPA rotation color modes, buffer " + "sizing) is RGB565-only; configure LVGL with LV_COLOR_DEPTH 16"); + +// Number of framebuffers the DPI panel is created with (esp_lcd_dpi_panel_config_t::num_fbs, +// used for every controller variant below). The direct-to-framebuffer PPA +// rotation in flush() caches THE single framebuffer pointer at init and writes +// into it unconditionally; with more than one framebuffer the driver flips +// which buffer is scanned out, and rotating into a fixed one would +// intermittently update a non-visible buffer. That optimization is therefore +// only implemented for exactly one framebuffer: raising this value requires +// first teaching the direct-to-framebuffer path to track the active +// framebuffer (or removing it in favor of the always-correct scratch-buffer +// path), and a static_assert in initialize_lcd() enforces that at compile +// time rather than letting the stale-pointer bug ship. +static constexpr uint8_t kNumDpiFramebuffers = 1; + +// Alignment the PPA requires for its output buffer in external (PSRAM) memory: +// both the buffer pointer and the buffer size must be multiples of the data +// cache line size. On the ESP32-P4 external memory sits behind the L2 cache, +// whose line size is a Kconfig choice (CONFIG_CACHE_L2_CACHE_LINE_SIZE, 64 or +// 128 bytes) that cpu_start.c programs into the cache HAL at boot — so this +// public compile-time constant equals exactly the value the PPA driver itself +// validates against at runtime (ppa_check_buffer_alignment() checks +// s_platform.buf_alignment_size, which it obtains from that same cache HAL via +// the private esp_cache_get_alignment(MALLOC_CAP_SPIRAM | MALLOC_CAP_DMA, +// ...)). Using the Kconfig constant keeps this file off the private +// esp_private/esp_cache_private.h header, which can move or break across IDF +// releases. +#if defined(CONFIG_CACHE_L2_CACHE_LINE_SIZE) && (CONFIG_CACHE_L2_CACHE_LINE_SIZE > 0) +static constexpr size_t kPpaOutBufferAlignment = CONFIG_CACHE_L2_CACHE_LINE_SIZE; +#else +// Fallback for IDF configurations that do not expose the symbol (it has +// shipped with ESP32-P4 support from the start, so this is belt-and-braces): +// use the largest L2 cache line the P4 supports so the alignment is never +// under-estimated — over-aligning is always safe here. +static constexpr size_t kPpaOutBufferAlignment = 128; +#endif + M5StackTab5::DisplayController M5StackTab5::detect_display_controller() { auto &i2c = internal_i2c(); @@ -92,6 +163,45 @@ M5StackTab5::DisplayController M5StackTab5::detect_display_controller() { bool M5StackTab5::initialize_lcd() { logger_.info("Initializing M5Stack Tab5 LCD (MIPI-DSI, {}x{})", display_width_, display_height_); + // Re-initialization is NOT supported once the publication gate has opened: + // clearing lcd_initialized_ here would not wait for readers that already + // passed their acquire-load of true — a concurrent flush() or + // write_lcd_lines() could then race the rebuild of lcd_handles_ / + // display_driver_ / the framebuffer state it is still using. Rather than + // add reader/writer synchronization to the hot flush path for a re-init + // this BSP never needs (the panel hardware is fixed at boot), refuse. + if (lcd_initialized_.load(std::memory_order_acquire)) { + logger_.warn("LCD already initialized; re-initialization is not supported — skipping"); + return true; + } + + // The publication gate is provably closed here: this is either the first + // call or a retry after a failed attempt, and a failed attempt never + // reaches the final store-release. No reader can therefore have observed + // true, so the LCD state below (lcd_handles_, dpi_framebuffer_ + + // dpi_framebuffer_bytes_, display_driver_, display_controller_) can be + // built without racing them: even if the LVGL display already exists + // (initialize_display() called first) and its thread is already pumping + // flush(), flush()/write_lcd_lines()/on_display_rotation() all load-acquire + // this flag and no-op while it is false, so none of them can observe a + // partially initialized panel or a torn framebuffer-pointer/size pair. The + // gate is store-released true as the final step of this function, after + // every field has been written and the initial panel rotation applied. + + // Start every attempt from a clean slate for the state that is otherwise + // only assigned on success paths below. A failed attempt never opened the + // gate, so no reader has seen these, but it may still have left them set — + // and a retry does not necessarily rewrite them (the framebuffer caching + // below deliberately leaves them untouched when the query or alignment + // check fails). Without this reset such a retry could open the gate with a + // stale framebuffer pointer from the previous attempt still cached, and + // flush()'s direct-to-framebuffer PPA path would then write through it. + // (display_driver_ / display_controller_ need no reset here: display_driver_ + // is .reset() unconditionally below and both are rewritten together before + // the gate can open.) + dpi_framebuffer_ = nullptr; + dpi_framebuffer_bytes_ = 0; + if (!ioexp_0x43_) { if (!initialize_io_expanders()) { logger_.error("Failed to init IO expanders for LCD reset"); @@ -206,13 +316,7 @@ bool M5StackTab5::initialize_lcd() { dpi_cfg.virtual_channel = 0; dpi_cfg.dpi_clk_src = MIPI_DSI_DPI_CLK_SRC_DEFAULT; dpi_cfg.dpi_clock_freq_mhz = 60; -#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(6, 0, 0) - dpi_cfg.in_color_format = LCD_COLOR_FMT_RGB565; - dpi_cfg.out_color_format = LCD_COLOR_FMT_RGB565; -#else - dpi_cfg.pixel_format = LCD_COLOR_PIXEL_FORMAT_RGB565; -#endif - dpi_cfg.num_fbs = 1; + dpi_cfg.num_fbs = kNumDpiFramebuffers; dpi_cfg.video_timing.h_size = display_width_; dpi_cfg.video_timing.v_size = display_height_; dpi_cfg.video_timing.hsync_back_porch = 140; @@ -234,13 +338,7 @@ bool M5StackTab5::initialize_lcd() { // (M5Stack/esp-bsp value) shrinks the blanking window and desyncs the touch // scan, so the panel shows but touch never reports. Keep this at 70 MHz. dpi_cfg.dpi_clock_freq_mhz = 70; -#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(6, 0, 0) - dpi_cfg.in_color_format = LCD_COLOR_FMT_RGB565; - dpi_cfg.out_color_format = LCD_COLOR_FMT_RGB565; -#else - dpi_cfg.pixel_format = LCD_COLOR_PIXEL_FORMAT_RGB565; -#endif - dpi_cfg.num_fbs = 1; + dpi_cfg.num_fbs = kNumDpiFramebuffers; dpi_cfg.video_timing.h_size = display_width_; dpi_cfg.video_timing.v_size = display_height_; dpi_cfg.video_timing.hsync_back_porch = 40; @@ -259,13 +357,7 @@ bool M5StackTab5::initialize_lcd() { // against the pixel clock; use the M5GFX reference 70 MHz and porch set // (they differ from the ST7123's: VBP 24 / VPW 20 / VFP 200). dpi_cfg.dpi_clock_freq_mhz = 70; -#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(6, 0, 0) - dpi_cfg.in_color_format = LCD_COLOR_FMT_RGB565; - dpi_cfg.out_color_format = LCD_COLOR_FMT_RGB565; -#else - dpi_cfg.pixel_format = LCD_COLOR_PIXEL_FORMAT_RGB565; -#endif - dpi_cfg.num_fbs = 1; + dpi_cfg.num_fbs = kNumDpiFramebuffers; dpi_cfg.video_timing.h_size = display_width_; dpi_cfg.video_timing.v_size = display_height_; dpi_cfg.video_timing.hsync_back_porch = 40; @@ -280,6 +372,18 @@ bool M5StackTab5::initialize_lcd() { } if (lcd_handles_.panel == nullptr) { + // The DPI pixel format is set here - in exactly one place, shared by all + // controller variants - and is always RGB565: the whole video path is + // RGB565-only (see the static_assert at the top of this file), and no + // Kconfig or build flag selects any other panel format. The preprocessor + // branch below only picks the IDF-version-specific field names for that + // one format, never a different depth. +#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(6, 0, 0) + dpi_cfg.in_color_format = LCD_COLOR_FMT_RGB565; + dpi_cfg.out_color_format = LCD_COLOR_FMT_RGB565; +#else + dpi_cfg.pixel_format = LCD_COLOR_PIXEL_FORMAT_RGB565; +#endif logger_.info("Creating DPI panel with resolution {}x{}", dpi_cfg.video_timing.h_size, dpi_cfg.video_timing.v_size); ret = esp_lcd_new_panel_dpi(lcd_handles_.mipi_dsi_bus, &dpi_cfg, &lcd_handles_.panel); @@ -360,12 +464,161 @@ bool M5StackTab5::initialize_lcd() { .on_color_trans_done = &M5StackTab5::notify_lvgl_flush_ready, .on_refresh_done = nullptr, }; + // Completion semaphore for the serialized synchronous draws (see + // draw_and_wait()); created before the callback that gives it can fire. + if (draw_done_sem_ == nullptr) { + draw_done_sem_ = xSemaphoreCreateBinary(); + if (draw_done_sem_ == nullptr) { + logger_.error("Failed to create the draw-completion semaphore"); + return false; + } + } ret = esp_lcd_dpi_panel_register_event_callbacks(lcd_handles_.panel, &cbs, this); if (ret != ESP_OK) { logger_.error("Failed to register panel event callback: {}", esp_err_to_name(ret)); return false; } +#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(6, 0, 0) + // Use the 2D-DMA engine for esp_lcd_panel_draw_bitmap copies into the DPI + // framebuffer. On ESP-IDF < 6.0 this was requested with the + // esp_lcd_dpi_panel_config_t::flags.use_dma2d flag (set above); IDF 6.0 + // removed the flag in favor of this explicit call. Without it every + // draw_bitmap is a CPU memcpy through the cache — for a full 720x1280 RGB565 + // frame that is ~1.8 MB read + ~1.8 MB written + a ~1.8 MB cache writeback + // per flush, all PSRAM traffic that competes with the DPI panel's continuous + // ~140 MB/s framebuffer scan-out DMA. Starving that scan-out DMA underruns + // the DSI bridge FIFO, which shows up as streaks/tears along the panel's + // scan-line axis (the driver logs "underrun happens" when it detects this). + // The DMA2D copy runs without the CPU touching the data and completes via + // the same on_color_trans_done callback registered above. + // + // Gate it per controller, though: DMA2D is a color-processing engine, not a + // plain copy, and this repository documents (from hardware testing) that + // routing draw_bitmap through it corrupts the RGB565 channel order on + // ILI9881C-family DSI panels — colors render brighter/greener and alpha + // blends come out wrong even though the framebuffer bytes are correct (see + // components/esp32-p4-function-ev-board/src/video.cpp, ILI9881C/EK79007, + // and components/esp32-p4-nano/src/video.cpp, which enables DMA2D only for + // its JD9365 panel for the same reason). The Tab5's original revision uses + // that same ILI9881 family, so it keeps the always-correct CPU copy path + // and accepts the extra PSRAM traffic. The ST7121/ST7123 TDDI variants keep + // DMA2D: no such corruption has been reported for them, they are the units + // on which the underrun streaking this call addresses was reproduced, and + // M5Stack's own Tab5 demo firmware ships with DMA2D enabled on production + // units that are predominantly ST71xx. + if (display_controller_ != DisplayController::ILI9881) { + ret = esp_lcd_dpi_panel_enable_dma2d(lcd_handles_.panel); + if (ret != ESP_OK) { + // Not fatal: draw_bitmap falls back to the (slower) CPU copy. + logger_.warn("Could not enable DMA2D for DPI draw_bitmap ({}); using CPU copies", + esp_err_to_name(ret)); + } + } else { + logger_.info("ILI9881 variant: keeping CPU draw_bitmap copies (DMA2D corrupts RGB565 " + "channel order on ILI9881C-family panels)"); + } +#endif + + // Cache the DPI framebuffer address so flush() can rotate directly into it + // with the PPA (see flush()). The panel scans this buffer out continuously; + // the espp draw/flush path itself never writes it with the CPU. + // + // This optimization is only valid with a single DPI framebuffer: caching one + // fixed pointer assumes the panel scans that same buffer forever. With + // num_fbs > 1 the driver flips between buffers and the PPA could rotate into + // one that is not being scanned out. Multi-framebuffer operation is + // intentionally unsupported until this path learns to track the active + // framebuffer, and the static_assert below turns raising + // kNumDpiFramebuffers without doing that work into a compile-time error + // instead of an intermittent visual glitch. + static_assert(kNumDpiFramebuffers == 1, + "The direct-to-framebuffer PPA rotation caches a single framebuffer pointer; " + "with multiple DPI framebuffers it must track the active one instead"); + { + void *fb = nullptr; + if (esp_lcd_dpi_panel_get_frame_buffer(lcd_handles_.panel, kNumDpiFramebuffers, &fb) == + ESP_OK && + fb != nullptr) { + const size_t fb_bytes = static_cast(display_width_) * display_height_ * sizeof(Pixel); + // The PPA requires its output buffer pointer and size to be aligned to + // the data cache line (see kPpaOutBufferAlignment). The esp_lcd driver + // allocates the DPI framebuffer DMA-aligned, and 720*1280*2 is a + // multiple of any such line size, but verify rather than assume — if it + // does not hold, flush() simply keeps the scratch-buffer path. + const size_t cache_align = kPpaOutBufferAlignment; + // cppcheck-suppress [knownConditionTrueFalse, moduloofone] // cache_align + // is a Kconfig compile-time constant (64 or 128); cppcheck's --force + // explores a configuration where the macro folds to 1, trivializing the + // checks. On real configurations both checks are meaningful. + if ((reinterpret_cast(fb) % cache_align) == 0 && (fb_bytes % cache_align) == 0) { + dpi_framebuffer_ = fb; + dpi_framebuffer_bytes_ = fb_bytes; + } else { + logger_.warn("DPI framebuffer not cache-line aligned; PPA will rotate via scratch buffer"); + } + } else { + logger_.warn("Could not query the DPI framebuffer; PPA will rotate via scratch buffer"); + } + } + + // Program the panel's initial scan direction BEFORE publishing the LCD + // state, so the rotation decision flush() makes via + // panel_handles_rotation() is valid from the very first frame that can + // reach the panel. Ordering matters in the reversed init order + // (initialize_display() first): an already-running LVGL thread may flush + // the instant the gate opens, and if the gate opened before this call such + // a flush could skip the PPA rotation (panel_handles_rotation() true) while + // the panel's MADCTL still held the old scan direction. Applying the + // rotation first closes that window. on_display_rotation() itself no-ops + // while the gate is closed, so the init path calls the gate-free + // apply_panel_rotation() helper directly — safe, because this is the same + // thread that wrote display_driver_/display_controller_ above (no + // synchronization needed against itself) and any concurrent flush() still + // no-ops on the closed gate. Per init order: + // - Documented order (initialize_lcd() before initialize_display()): + // display_ is still null here, so no LVGL display or flush callback + // exists yet and espp::Display does not run an LVGL handler task of its + // own — nothing can race this call. It is also redundant-but-harmless in + // this order: the espp::Display constructor will call + // lv_display_set_rotation() with the initial rotation, which synchronously + // fires the LV_EVENT_RESOLUTION_CHANGED handler (in LVGL, + // update_resolution() sends the event as a direct call) and thus + // on_display_rotation() again before any flush can run. + // - Reversed order (initialize_display() first): any earlier rotation + // callback no-op'd on the closed gate (and display_driver_ did not exist + // yet), so (re)apply the current LVGL rotation now that the driver is up. + // Even if the application is already pumping LVGL on another thread, this + // cannot corrupt an in-flight flush: the MADCTL write travels on the DSI + // command channel, which never touches the DPI framebuffer or its DMA and + // is arbitrated against the video stream in hardware (see + // apply_panel_rotation()). + // (With CONFIG_M5STACK_TAB5_ST7121_HW_ROTATION disabled — the default — + // apply_panel_rotation() is a no-op and this call does nothing at all.) + // Use the configured initial `rotation` rather than querying LVGL here: + // in the reversed init order the LVGL update task may already be running, + // and lv_display_get_rotation() from this (init) thread would read LVGL + // state without the application's LVGL lock (LV_USE_OS == LV_OS_NONE). Any + // rotation the app sets later is applied by on_display_rotation() (the LVGL + // rotation callback, which runs on the GUI task), so the panel still tracks + // runtime changes; this call only establishes the initial MADCTL. + apply_panel_rotation(rotation); + + // Publish the fully initialized LCD state — the gate opens LAST. Every + // field the cross-thread readers touch (lcd_handles_, dpi_framebuffer_ + + // dpi_framebuffer_bytes_, display_driver_, display_controller_) has been + // written above and the panel's initial MADCTL rotation has been applied, + // so the release store here makes all of that visible to any + // flush()/write_lcd_lines()/on_display_rotation() call that load-acquires + // the flag as true. Until this store, those readers no-op (flush() just + // signals lv_display_flush_ready()), which is what closes the + // reversed-init-order races: with initialize_display() called first and the + // application already pumping LVGL on another thread, a concurrent flush() + // can no longer pass a plain lcd_handles_.panel null-check mid-build and + // read a not-yet-init'd panel, a torn framebuffer-pointer/size pair, or a + // panel whose scan direction does not yet match panel_handles_rotation(). + lcd_initialized_.store(true, std::memory_order_release); + logger_.info("M5Stack Tab5 LCD initialization completed successfully"); return true; } @@ -373,7 +626,7 @@ bool M5StackTab5::initialize_lcd() { // Scratch buffer that holds the hardware-rotated frame produced by the PPA // before it is handed to the panel (see flush()). Kept in PSRAM and aligned to // the data-cache line size, which the PPA requires for its output buffer. -static uint16_t *third_buffer = nullptr; +static M5StackTab5::Pixel *third_buffer = nullptr; static size_t third_buffer_bytes = 0; // PPA (Pixel Processing Accelerator) client used to rotate the frame in // hardware instead of on the CPU (lv_draw_sw_rotate). CPU rotation of a @@ -388,7 +641,11 @@ bool M5StackTab5::initialize_display(size_t pixel_buffer_size) { Display::LvglConfig{.width = display_width_, .height = display_height_, .flush_callback = std::bind_front(&M5StackTab5::flush, this), - .rotation_callback = nullptr, // DisplayDriver::rotate, + // Forward LVGL rotation changes to the display driver so + // panels that can rotate in hardware (ST7121) do so via + // MADCTL instead of the PPA / software rotation in flush(). + .rotation_callback = + std::bind_front(&M5StackTab5::on_display_rotation, this), .rotation = rotation}, Display::OledConfig{ .set_brightness_callback = @@ -413,6 +670,15 @@ bool M5StackTab5::initialize_display(size_t pixel_buffer_size) { if (g_ppa_client == nullptr) { ppa_client_config_t ppa_cfg = {}; ppa_cfg.oper_type = PPA_OPERATION_SRM; + // Throttle the PPA's AXI bursts (default PPA_DATA_BURST_LENGTH_128). The + // DPI panel continuously scans its PSRAM framebuffer at ~140 MB/s; on the + // ESP32-P4 a PPA client running full-length bursts against the same PSRAM + // is known to starve that scan-out DMA and underrun the DSI bridge FIFO, + // which appears as streaks along the panel's scan-line axis even with + // 200 MHz hex PSRAM (see lvgl/lvgl#9590 - shorter PPA bursts fix the + // artifacts at a small cost in PPA throughput; LVGL's own PPA integration + // exposes the same knob as LV_PPA_BURST_LENGTH). + ppa_cfg.data_burst_length = PPA_DATA_BURST_LENGTH_64; esp_err_t perr = ppa_register_client(&ppa_cfg, &g_ppa_client); if (perr != ESP_OK) { logger_.warn("Could not register PPA client ({}); rotation will fall back to software", @@ -420,12 +686,12 @@ bool M5StackTab5::initialize_display(size_t pixel_buffer_size) { g_ppa_client = nullptr; } } - // Align to 128 bytes: the PPA output buffer in external (PSRAM) memory must - // be aligned to the L1 and L2 cache line size, and the ESP32-P4's L2 line is - // 128 bytes. Both the pointer and the size must be a multiple of it. - static constexpr size_t kCacheAlign = 128; - size_t required_bytes = pixel_buffer_size * sizeof(uint16_t); - required_bytes = (required_bytes + kCacheAlign - 1) / kCacheAlign * kCacheAlign; + // The PPA output buffer in external (PSRAM) memory must be aligned to the + // data cache line size: both the pointer and the size must be a multiple of + // it (see kPpaOutBufferAlignment). + const size_t cache_align = kPpaOutBufferAlignment; + size_t required_bytes = pixel_buffer_size * sizeof(Pixel); + required_bytes = (required_bytes + cache_align - 1) / cache_align * cache_align; // Reuse the existing scratch buffer if it is already the right size; otherwise // free it first so a repeated initialize_display() call (e.g. re-init with a @@ -436,8 +702,15 @@ bool M5StackTab5::initialize_display(size_t pixel_buffer_size) { third_buffer = nullptr; } third_buffer_bytes = required_bytes; - third_buffer = (uint16_t *)heap_caps_aligned_alloc(kCacheAlign, third_buffer_bytes, - MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT); + // Request MALLOC_CAP_SPIRAM | MALLOC_CAP_DMA: the buffer is a PPA (DMA) + // output target, and these are exactly the caps kPpaOutBufferAlignment is + // derived for. The heap layer supports this combination for external + // memory (esp_heap_adjust_alignment_to_hw() applies the cache-line + // alignment/size the caps require, then maps the request onto the PSRAM + // heap), so on the ESP32-P4 - whose PSRAM is DMA-capable - it yields a + // DMA-usable PSRAM allocation. + third_buffer = static_cast(heap_caps_aligned_alloc( + cache_align, third_buffer_bytes, MALLOC_CAP_SPIRAM | MALLOC_CAP_DMA)); if (third_buffer == nullptr) { // The scratch buffer is required for display rotation - both the PPA path // and the software fallback rotate into it - so without it a non-zero @@ -454,7 +727,8 @@ bool M5StackTab5::initialize_display(size_t pixel_buffer_size) { } size_t M5StackTab5::rotated_display_width() const { - auto rotation = lv_display_get_rotation(lv_display_get_default()); + auto rotation = + lv_display_get_rotation(display_ ? display_->get_lvgl_display() : lv_display_get_default()); switch (rotation) { // swap case LV_DISPLAY_ROTATION_90: @@ -469,7 +743,8 @@ size_t M5StackTab5::rotated_display_width() const { } size_t M5StackTab5::rotated_display_height() const { - auto rotation = lv_display_get_rotation(lv_display_get_default()); + auto rotation = + lv_display_get_rotation(display_ ? display_->get_lvgl_display() : lv_display_get_default()); switch (rotation) { // swap case LV_DISPLAY_ROTATION_90: @@ -483,17 +758,140 @@ size_t M5StackTab5::rotated_display_height() const { } } +bool M5StackTab5::panel_handles_rotation(lv_display_rotation_t rotation) const { +#if !CONFIG_M5STACK_TAB5_ST7121_HW_ROTATION + // Panel-side rotation is DISABLED by default: hardware testing showed the + // 180-degree MADCTL GS/SS scan flip renders corrupted on (at least some) + // ST7121 units - the TDDI gate/source mux tables programmed at init (the + // 0xAC block) are matched to the normal scan direction, and a MADCTL GS + // flip alone reorders gate scanning without swapping them. All orientations + // therefore use the PPA/flush-time rotation (the pre-existing, known-good + // path). Enable CONFIG_M5STACK_TAB5_ST7121_HW_ROTATION to experiment. + (void)rotation; + return false; +#else + // Only the ST7121 variant routes rotation to the panel (the ILI9881 and + // ST7123 keep the historical PPA/flush-time rotation path). The Tab5 panels + // are MIPI-DSI DPI (video mode) panels: the host streams a fixed 720x1280 + // raster, so the panel cannot swap axes for 90/270 (no MADCTL MV/GRAM + // addressing in video mode) and those still need the frame rotated into the + // framebuffer (PPA, or software fallback). The gate/source scan-direction + // flips (MADCTL GS/SS) are applied by the panel itself through the espp + // display driver's set_rotation() for 0 and 180 - EXPERIMENTAL, see the + // Kconfig help. + if (display_controller_ != DisplayController::ST7121) { + return false; + } + return rotation == LV_DISPLAY_ROTATION_0 || rotation == LV_DISPLAY_ROTATION_180; +#endif +} + +void M5StackTab5::on_display_rotation(const DisplayRotation &rotation) { +#if !CONFIG_M5STACK_TAB5_ST7121_HW_ROTATION + // Panel-side rotation disabled (see panel_handles_rotation()): make this a + // complete no-op so NO runtime MADCTL write ever reaches the panel - the + // scan state stays exactly as the init sequence programmed it. + (void)rotation; +#else + // Acquire-load the publication gate (paired with the release store in + // initialize_lcd()) before reading display_driver_ / display_controller_: + // with initialize_display() called first, LVGL rotation events can arrive + // while initialize_lcd() is still writing those fields. Bailing out here is + // harmless — initialize_lcd() applies the current LVGL rotation via + // apply_panel_rotation() before it opens the gate, so the panel state is + // already consistent for the first flush() that observes the gate open. + if (!lcd_initialized_.load(std::memory_order_acquire)) { + return; + } + apply_panel_rotation(rotation); +#endif +} + +void M5StackTab5::apply_panel_rotation(const DisplayRotation &rotation) { +#if !CONFIG_M5STACK_TAB5_ST7121_HW_ROTATION + // Panel-side rotation disabled (see panel_handles_rotation()): complete + // no-op so NO MADCTL write ever reaches the panel. + (void)rotation; +#else + if (!display_driver_ || display_controller_ != DisplayController::ST7121) { + // Other variants keep the PPA/flush-time rotation path; nothing to do. + return; + } + // Ordering: in its normal invocation — the espp::Display rotation callback, + // via on_display_rotation() — this runs on the LVGL thread: + // LV_EVENT_RESOLUTION_CHANGED is sent synchronously from inside + // lv_display_set_rotation(), so it strictly precedes the + // invalidation-driven flush() calls for the new orientation, and the MADCTL + // state below is always consistent with the decision flush() makes via + // panel_handles_rotation() (the same predicate, keyed on the same rotation + // value via to_lv_rotation()). It is additionally called once directly from + // initialize_lcd() (an init-thread call, not the LVGL thread, deliberately + // BEFORE the lcd_initialized_ gate opens) to program the initial scan + // direction; see the safety analysis at that call site. This helper is + // gate-free: callers must guarantee display_driver_/display_controller_ are + // safe to read (on_display_rotation() does so via its acquire load; the + // init path wrote them on the same thread). + // + // Synchronization with the display pipeline: the MADCTL write goes out on + // the DSI generic/DBI command channel (esp_lcd_panel_io_tx_param -> + // mipi_dsi_hal_host_gen_write_dcs_command), which the DSI host peripheral + // arbitrates against the DPI video stream in hardware. It never touches the + // DPI framebuffer or its DMA, so it cannot corrupt a previous flush()'s + // in-flight draw_bitmap copy. The panel may latch the new scan direction + // mid scan-out, which can show as (at most) a single transient frame; that + // is accepted here, since a rotation change is a full-screen visual + // discontinuity anyway and LVGL follows it immediately with a full + // invalidate/redraw of the new orientation. + if (panel_handles_rotation(to_lv_rotation(rotation))) { + // 0 / 180: the panel applies the rotation itself (scan-direction flip via + // MADCTL); flush() writes unrotated buffers at unrotated coordinates. + display_driver_->set_rotation(rotation); + } else { + // 90 / 270: a DPI video-mode panel cannot swap axes, so restore the + // natural scan direction and let the PPA rotation in flush() do the full + // transform. + display_driver_->set_rotation(DisplayRotation::LANDSCAPE); + } +#endif +} + +bool M5StackTab5::draw_and_wait(int x1, int y1, int x2, int y2, const void *data) { + // Caller holds panel_op_mutex_. Drain any stale completion first (defensive: + // a prior transfer that timed out could leave the binary semaphore signalled), + // issue the draw, then block until on_color_trans_done gives the semaphore. + if (draw_done_sem_ != nullptr) { + xSemaphoreTake(draw_done_sem_, 0); + } + esp_err_t err = esp_lcd_panel_draw_bitmap(lcd_handles_.panel, x1, y1, x2, y2, data); + if (err != ESP_OK) { + // Rejected (e.g. ESP_ERR_INVALID_STATE: a prior draw still in flight) or + // failed - no completion callback will arrive, so do not wait for one. + logger_.warn_rate_limited("draw_bitmap failed: {}", esp_err_to_name(err)); + return false; + } + if (draw_done_sem_ != nullptr) { + // Bounded wait so a missing completion cannot wedge the caller forever. + xSemaphoreTake(draw_done_sem_, pdMS_TO_TICKS(200)); + } + return true; +} + void M5StackTab5::write_lcd_lines(int xs, int ys, int xe, int ye, const uint8_t *data, uint32_t user_data) { (void)user_data; - if (lcd_handles_.panel == nullptr || data == nullptr) { + // Acquire-load the publication gate (paired with the release store in + // initialize_lcd()) instead of a plain lcd_handles_.panel null-check: this + // runs on the caller's thread (e.g. the camera task) and must not race the + // init thread's writes or draw into a panel that is not fully initialized. + if (!lcd_initialized_.load(std::memory_order_acquire) || data == nullptr) { return; } if (xs < 0 || ys < 0 || xe < xs || ye < ys) { logger_.error("write_lcd_lines: Bad region: ({},{}) to ({},{})", xs, ys, xe, ye); return; } - esp_lcd_panel_draw_bitmap(lcd_handles_.panel, xs, ys, xe + 1, ye + 1, data); + std::lock_guard lock(panel_op_mutex_); + draw_and_wait(xs, ys, xe + 1, ye + 1, data); } void M5StackTab5::brightness(float brightness) { @@ -524,7 +922,15 @@ void M5StackTab5::flush(lv_display_t *disp, const lv_area_t *area, uint8_t *px_m // interrupt is handled separately in notify_lvgl_flush_ready(). Blocking work // (the PPA rotation and esp_lcd_panel_draw_bitmap) is therefore safe here. - if (lcd_handles_.panel == nullptr) { + // Acquire-load the publication gate before touching any LCD state. This + // pairs with the release store at the end of initialize_lcd(): observing + // true guarantees lcd_handles_ (incl. a fully init'd panel, DMA2D-enabled + // on the ST71xx variants), the dpi_framebuffer_/dpi_framebuffer_bytes_ + // pair, display_driver_ and display_controller_ are all completely written + // and will not be written again (the gate never closes once open — + // initialize_lcd() refuses to re-run). Until then (LCD not yet initialized) + // just tell LVGL the flush is done and drop the frame. + if (!lcd_initialized_.load(std::memory_order_acquire)) { lv_display_flush_ready(disp); return; } @@ -534,34 +940,53 @@ void M5StackTab5::flush(lv_display_t *disp, const lv_area_t *area, uint8_t *px_m int offsety1 = area->y1; int offsety2 = area->y2; - auto rotation = lv_display_get_rotation(lv_display_get_default()); + // Use the display actually being flushed, not the global default display. + auto rotation = lv_display_get_rotation(disp); // Cache the rotation for the camera task, which must not call LVGL from its // own thread. flush() runs on the LVGL (GUI) thread, so reading it here is // safe; the camera task reads the cached atomic instead. camera_display_rotation_.store(static_cast(rotation), std::memory_order_relaxed); - if (rotation > LV_DISPLAY_ROTATION_0 && third_buffer != nullptr) { + // When the panel itself applies the rotation (ST7121, 180 degrees via the + // display driver's MADCTL scan flip - see on_display_rotation()), the + // logical frame is written to the framebuffer unrotated and at unrotated + // coordinates; the panel flips the whole frame at scan-out, which lands each + // partial area exactly where LVGL's rotated mapping expects it. + if (rotation > LV_DISPLAY_ROTATION_0 && !panel_handles_rotation(rotation) && + third_buffer != nullptr) { int32_t ww = lv_area_get_width(area); int32_t hh = lv_area_get_height(area); lv_color_format_t cf = lv_display_get_color_format(disp); - bool rotated = false; - if (g_ppa_client != nullptr) { - // Hardware rotation via the PPA. The LVGL rotation maps directly onto the - // PPA rotation angle (LVGL 90 -> PPA 90, 180 -> 180, 270 -> 270); this is - // the mapping verified on hardware. For 90/270 the output picture - // width/height are swapped. If a different panel comes out turned the - // wrong way, swap the 90 and 270 cases here. + // Map the logical (LVGL) area to physical panel coordinates up front: the + // direct-to-framebuffer PPA path needs the rotated destination offsets + // before the PPA runs, and the fallback paths need them for draw_bitmap. + // Rotate a local copy — LVGL handed us a const pointer, so do not mutate + // its area in place. + lv_area_t rotated_area = *area; + lv_display_rotate_area(disp, &rotated_area); + offsetx1 = rotated_area.x1; + offsetx2 = rotated_area.x2; + offsety1 = rotated_area.y1; + offsety2 = rotated_area.y2; + if (g_ppa_client != nullptr && dpi_framebuffer_ != nullptr) { + // Hardware rotation via the PPA, writing the rotated block DIRECTLY into + // the DPI panel's framebuffer at the rotated offset (the PPA output + // supports placing a block inside a larger picture). This halves the + // PSRAM traffic of the previous scratch-buffer approach (PPA write + + // draw_bitmap read + write), which matters because the DSI scan-out DMA + // is reading the same PSRAM continuously and underruns - visible as + // streaks - when the flush path hogs the bandwidth. + // + // The LVGL rotation maps directly onto the PPA rotation angle (LVGL 90 + // -> PPA 90, 180 -> 180, 270 -> 270); this is the mapping verified on + // hardware. If a different panel comes out turned the wrong way, swap + // the 90 and 270 cases here. ppa_srm_rotation_angle_t angle = PPA_SRM_ROTATION_ANGLE_0; - uint32_t out_w = ww, out_h = hh; if (rotation == LV_DISPLAY_ROTATION_90) { angle = PPA_SRM_ROTATION_ANGLE_90; - out_w = hh; - out_h = ww; } else if (rotation == LV_DISPLAY_ROTATION_180) { angle = PPA_SRM_ROTATION_ANGLE_180; } else if (rotation == LV_DISPLAY_ROTATION_270) { angle = PPA_SRM_ROTATION_ANGLE_270; - out_w = hh; - out_h = ww; } ppa_srm_oper_config_t srm = {}; srm.in.buffer = px_map; @@ -570,22 +995,46 @@ void M5StackTab5::flush(lv_display_t *disp, const lv_area_t *area, uint8_t *px_m srm.in.block_w = ww; srm.in.block_h = hh; srm.in.srm_cm = PPA_SRM_COLOR_MODE_RGB565; - srm.out.buffer = third_buffer; - srm.out.buffer_size = third_buffer_bytes; - srm.out.pic_w = out_w; - srm.out.pic_h = out_h; + srm.out.buffer = dpi_framebuffer_; + srm.out.buffer_size = dpi_framebuffer_bytes_; + srm.out.pic_w = display_width_; + srm.out.pic_h = display_height_; + srm.out.block_offset_x = offsetx1; + srm.out.block_offset_y = offsety1; srm.out.srm_cm = PPA_SRM_COLOR_MODE_RGB565; srm.rotation_angle = angle; srm.scale_x = 1.0f; srm.scale_y = 1.0f; srm.mode = PPA_TRANS_MODE_BLOCKING; - // On failure third_buffer holds stale/partial data; leave rotated=false so - // we fall through to the software rotation below rather than flushing it. - rotated = (ppa_do_scale_rotate_mirror(g_ppa_client, &srm) == ESP_OK); + // Serialize the framebuffer write with draw_bitmap (write_lcd_lines / + // the non-PPA flush tail) so two engines never write the panel FB at once. + esp_err_t ppa_err; + { + std::lock_guard lock(panel_op_mutex_); + ppa_err = ppa_do_scale_rotate_mirror(g_ppa_client, &srm); + } + if (ppa_err == ESP_OK) { + // The rotated pixels are already in the scanned-out framebuffer and + // the PPA driver performed the cache maintenance (write-back of the + // source window, invalidate of the destination window). The espp + // flush path never dirties the framebuffer with the CPU (draw_bitmap + // copies run on the DMA2D engine on the ST71xx variants, and the CPU + // copy path — used by the ILI9881 variant and as the DMA2D fallback — + // writes back the cache before returning), so there is nothing left + // to sync and + // no draw_bitmap call is needed: signal LVGL directly. + lv_display_flush_ready(disp); + return; + } + // On failure fall through to the scratch-buffer software rotation: the + // framebuffer may hold a partial block, but the fallback redraws the + // full area at the same destination. } - if (!rotated) { - // Software fallback: the PPA client failed to register or the PPA - // operation failed. Rotates into third_buffer, fully overwriting it. + { + // Fallback: the PPA client failed to register, the framebuffer could + // not be queried, or the PPA operation failed. Rotate on the CPU into + // the scratch buffer, fully overwriting it, and hand it to draw_bitmap + // below. uint32_t w_stride = lv_draw_buf_width_to_stride(ww, cf); uint32_t h_stride = lv_draw_buf_width_to_stride(hh, cf); if (rotation == LV_DISPLAY_ROTATION_180) { @@ -600,17 +1049,18 @@ void M5StackTab5::flush(lv_display_t *disp, const lv_area_t *area, uint8_t *px_m } } px_map = reinterpret_cast(third_buffer); - lv_display_rotate_area(disp, const_cast(area)); - offsetx1 = area->x1; - offsetx2 = area->x2; - offsety1 = area->y1; - offsety2 = area->y2; } - // pass the draw buffer to the DPI panel driver - esp_lcd_panel_draw_bitmap(lcd_handles_.panel, offsetx1, offsety1, offsetx2 + 1, offsety2 + 1, - px_map); - // For DPI panels, the notification will come through the callback + // Pass the draw buffer to the DPI panel driver. Serialized + synchronous (see + // draw_and_wait): only one panel transfer is ever in flight, so a concurrent + // write_lcd_lines() cannot make this draw fail. Always signal LVGL afterwards + // - even if the draw was rejected/failed - so LVGL never waits forever for a + // completion that will not come. + { + std::lock_guard lock(panel_op_mutex_); + draw_and_wait(offsetx1, offsety1, offsetx2 + 1, offsety2 + 1, px_map); + } + lv_display_flush_ready(disp); } bool IRAM_ATTR M5StackTab5::notify_lvgl_flush_ready(esp_lcd_panel_handle_t panel, @@ -621,12 +1071,16 @@ bool IRAM_ATTR M5StackTab5::notify_lvgl_flush_ready(esp_lcd_panel_handle_t panel return false; } - // This is called from ISR context, so we need to be careful about what we do - // Just notify LVGL that the flush is ready - avoid logging or other complex operations - if (tab5->display_) { - tab5->display_->notify_flush_ready(); + // ISR context: just release the draw-completion semaphore. The draw that + // issued this transfer (flush() or write_lcd_lines(), holding + // panel_op_mutex_) is waiting on it and will signal LVGL itself. This keeps + // a direct write_lcd_lines() completion from being mistaken for an LVGL + // flush completion. + BaseType_t higher_priority_task_woken = pdFALSE; + if (tab5->draw_done_sem_ != nullptr) { + xSemaphoreGiveFromISR(tab5->draw_done_sem_, &higher_priority_task_woken); } - return false; + return higher_priority_task_woken == pdTRUE; } void M5StackTab5::dsi_write_command(uint8_t cmd, std::span params,