From 4def0af768e319c1ee17cc97a14d11853862723d Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Thu, 23 Jul 2026 14:41:24 -0500 Subject: [PATCH 1/3] feat(m5stack-tab5): Update tab5 video and example to have larger display buffers and use the PPU for faster refresh and display rotation --- components/m5stack-tab5/CMakeLists.txt | 2 +- .../example/main/m5stack_tab5_example.cpp | 10 +- components/m5stack-tab5/src/video.cpp | 114 ++++++++++++++---- 3 files changed, 101 insertions(+), 25 deletions(-) diff --git a/components/m5stack-tab5/CMakeLists.txt b/components/m5stack-tab5/CMakeLists.txt index d75353614..f0aeae866 100755 --- a/components/m5stack-tab5/CMakeLists.txt +++ b/components/m5stack-tab5/CMakeLists.txt @@ -1,6 +1,6 @@ idf_component_register( INCLUDE_DIRS "include" SRC_DIRS "src" - REQUIRES driver esp_driver_i2s esp_driver_sdmmc esp_driver_spi esp_lcd fatfs base_component bmi270 codec display display_drivers gt911 i2c ina226 input_drivers interrupt pi4ioe5v rx8130ce st7123touch task touch + REQUIRES driver esp_driver_i2s esp_driver_ppa esp_driver_sdmmc esp_driver_spi esp_lcd fatfs base_component bmi270 codec display display_drivers gt911 i2c ina226 input_drivers interrupt pi4ioe5v rx8130ce st7123touch task touch REQUIRED_IDF_TARGETS "esp32p4" ) diff --git a/components/m5stack-tab5/example/main/m5stack_tab5_example.cpp b/components/m5stack-tab5/example/main/m5stack_tab5_example.cpp index 56ca14169..b3f0106a8 100644 --- a/components/m5stack-tab5/example/main/m5stack_tab5_example.cpp +++ b/components/m5stack-tab5/example/main/m5stack_tab5_example.cpp @@ -81,9 +81,15 @@ extern "C" void app_main(void) { return; } - // initialize the display with a pixel buffer (Tab5 is 1280x720 with 2 bytes per pixel) + // initialize the display with a full-frame pixel buffer (Tab5 is 1280x720 with + // 2 bytes per pixel). A full-frame LVGL draw buffer (allocated in PSRAM, see + // initialize_display) lets a full-screen redraw - e.g. a rotation change - + // flush in a single pass instead of ~72 ten-line strips, so the screen + // repaints at once rather than wiping progressively. Partial updates still + // only render/flush their dirty area, so the larger buffer costs nothing + // there (just PSRAM). logger.info("Initializing display..."); - auto pixel_buffer_size = tab5.display_width() * 10; // tab5.display_height(); + auto pixel_buffer_size = tab5.display_width() * tab5.display_height(); if (!tab5.initialize_display(pixel_buffer_size)) { logger.error("Failed to initialize display!"); return; diff --git a/components/m5stack-tab5/src/video.cpp b/components/m5stack-tab5/src/video.cpp index 665f0035b..a5424be8e 100755 --- a/components/m5stack-tab5/src/video.cpp +++ b/components/m5stack-tab5/src/video.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -271,7 +272,16 @@ bool M5StackTab5::initialize_lcd() { return true; } +// 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 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 +// full-frame PSRAM buffer is slow and, because the transpose strides PSRAM, +// asymmetric between 90 and 270 degrees; the PPA is fast and symmetric. +static ppa_client_handle_t g_ppa_client = nullptr; bool M5StackTab5::initialize_display(size_t pixel_buffer_size) { logger_.info("Initializing LVGL display with pixel buffer size: {} pixels", pixel_buffer_size); @@ -289,13 +299,41 @@ bool M5StackTab5::initialize_display(size_t pixel_buffer_size) { Display::DynamicMemoryConfig{ .pixel_buffer_size = pixel_buffer_size, .double_buffered = true, - .allocation_flags = MALLOC_CAP_8BIT | MALLOC_CAP_DMA, + // Allocate the LVGL draw buffers in PSRAM: a full-frame double + // buffer is ~3.7 MB, far larger than internal RAM, and the DPI + // panel's draw_bitmap reads the source straight from PSRAM (the + // esp32-p4-function-ev-board uses the same PSRAM buffers). + .allocation_flags = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT, }, Logger::Verbosity::WARN); } - third_buffer = (uint16_t *)heap_caps_malloc(pixel_buffer_size * sizeof(uint16_t), - MALLOC_CAP_8BIT | MALLOC_CAP_DMA); + // Register a PPA client for hardware rotation, and allocate the rotation + // scratch buffer that the PPA writes into. The PPA output buffer (external / + // PSRAM memory) must be aligned to the data cache line size, and both the + // pointer and the size must be a multiple of it. + if (g_ppa_client == nullptr) { + ppa_client_config_t ppa_cfg = {}; + ppa_cfg.oper_type = PPA_OPERATION_SRM; + 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", + esp_err_to_name(perr)); + 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; + third_buffer_bytes = pixel_buffer_size * sizeof(uint16_t); + third_buffer_bytes = (third_buffer_bytes + kCacheAlign - 1) / kCacheAlign * kCacheAlign; + third_buffer = (uint16_t *)heap_caps_aligned_alloc(kCacheAlign, third_buffer_bytes, + MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT); + if (third_buffer == nullptr) { + logger_.error("Could not allocate the rotation scratch buffer ({} bytes)", third_buffer_bytes); + third_buffer_bytes = 0; + } logger_.info("LVGL display initialized"); return true; @@ -381,28 +419,60 @@ void IRAM_ATTR M5StackTab5::flush(lv_display_t *disp, const lv_area_t *area, uin int offsety2 = area->y2; auto rotation = lv_display_get_rotation(lv_display_get_default()); - if (rotation > LV_DISPLAY_ROTATION_0) { - /* SW rotation */ + if (rotation > LV_DISPLAY_ROTATION_0 && 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); - 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) { - lv_draw_sw_rotate(px_map, third_buffer, hh, ww, h_stride, h_stride, LV_DISPLAY_ROTATION_180, - cf); - } else if (rotation == LV_DISPLAY_ROTATION_90) { - // printf("%ld %ld\n", w_stride, h_stride); - lv_draw_sw_rotate(px_map, third_buffer, ww, hh, w_stride, h_stride, LV_DISPLAY_ROTATION_90, - cf); - // // rotate_copy_pixel((uint16_t*)color_map, (uint16_t*)third_buffer, offsetx1, offsety1, - // // offsetx2, offsety2, LV_HOR_RES, LV_VER_RES, 270); - // rotate_copy_pixel((uint16_t*)color_map, (uint16_t*)third_buffer, 0, 0, offsetx2 - offsetx1, - // offsety2 - offsety1, offsetx2 - offsetx1 + 1, offsety2 - offsety1 + 1, - // 270); - } else if (rotation == LV_DISPLAY_ROTATION_270) { - lv_draw_sw_rotate(px_map, third_buffer, ww, hh, w_stride, h_stride, LV_DISPLAY_ROTATION_270, - cf); + if (g_ppa_client != nullptr) { + // Hardware rotation via the PPA. NOTE: the PPA rotates COUNTER-clockwise + // while LVGL's rotation is clockwise, so LVGL 90 -> PPA 270 and LVGL 270 + // -> PPA 90 (180 is the same). If the rotated image comes out turned the + // wrong way, swap the 90/270 mapping here. For 90/270 the output picture + // width/height are swapped. + 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; + srm.in.pic_w = ww; + srm.in.pic_h = hh; + 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.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; + ppa_do_scale_rotate_mirror(g_ppa_client, &srm); + } else { + // Software fallback (used only if the PPA client failed to register) + 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) { + lv_draw_sw_rotate(px_map, third_buffer, hh, ww, h_stride, h_stride, LV_DISPLAY_ROTATION_180, + cf); + } else if (rotation == LV_DISPLAY_ROTATION_90) { + lv_draw_sw_rotate(px_map, third_buffer, ww, hh, w_stride, h_stride, LV_DISPLAY_ROTATION_90, + cf); + } else if (rotation == LV_DISPLAY_ROTATION_270) { + lv_draw_sw_rotate(px_map, third_buffer, ww, hh, w_stride, h_stride, LV_DISPLAY_ROTATION_270, + cf); + } } px_map = reinterpret_cast(third_buffer); lv_display_rotate_area(disp, const_cast(area)); From c06b5793d82fe5bb64ec9323ec8dafb8d176d109 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Thu, 23 Jul 2026 15:51:57 -0500 Subject: [PATCH 2/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- components/m5stack-tab5/src/video.cpp | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/components/m5stack-tab5/src/video.cpp b/components/m5stack-tab5/src/video.cpp index a5424be8e..cc3b7b803 100755 --- a/components/m5stack-tab5/src/video.cpp +++ b/components/m5stack-tab5/src/video.cpp @@ -326,13 +326,21 @@ bool M5StackTab5::initialize_display(size_t pixel_buffer_size) { // 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; - third_buffer_bytes = pixel_buffer_size * sizeof(uint16_t); - third_buffer_bytes = (third_buffer_bytes + kCacheAlign - 1) / kCacheAlign * kCacheAlign; - third_buffer = (uint16_t *)heap_caps_aligned_alloc(kCacheAlign, third_buffer_bytes, - MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT); - if (third_buffer == nullptr) { - logger_.error("Could not allocate the rotation scratch buffer ({} bytes)", third_buffer_bytes); - third_buffer_bytes = 0; + size_t required_bytes = pixel_buffer_size * sizeof(uint16_t); + required_bytes = (required_bytes + kCacheAlign - 1) / kCacheAlign * kCacheAlign; + + if (third_buffer == nullptr || third_buffer_bytes != required_bytes) { + if (third_buffer != nullptr) { + free(third_buffer); + 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); + if (third_buffer == nullptr) { + logger_.error("Could not allocate the rotation scratch buffer ({} bytes)", third_buffer_bytes); + third_buffer_bytes = 0; + } } logger_.info("LVGL display initialized"); From cd1f5e1b98f1cf79d70e1828497c757494808423 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Fri, 24 Jul 2026 09:15:14 -0500 Subject: [PATCH 3/3] fix(m5stack-tab5): address PR review on the PPU display-rotation path Review feedback on the PPA rotation in video.cpp: - initialize_display() reused/overwrote the rotation scratch buffer on every call, leaking it on re-init. Reuse it when the size matches, otherwise free it before reallocating. - If the scratch-buffer allocation failed, initialize_display() still returned true, leaving rotation silently broken (flush would skip rotation). The buffer is required for both the PPA and the software rotation, so fail init instead. - The PPA-rotation comment claimed LVGL 90 -> PPA 270 / 270 -> PPA 90 while the code (verified on hardware) maps 90 -> 90 and 270 -> 270. Correct the comment. - ppa_do_scale_rotate_mirror()'s return value was ignored, so a failed rotation would flush stale/partial scratch-buffer data to the panel. Check it and fall back to the software rotation (which fully overwrites the buffer) on failure. - flush() was marked IRAM_ATTR and commented as possibly ISR-invoked, which conflicts with its blocking PPA op. It is actually LVGL's flush callback and runs in the LVGL task context (the DPI transfer-done ISR is the separate notify_lvgl_flush_ready), so drop IRAM_ATTR and correct the comment; blocking work is safe there. Co-Authored-By: Claude Opus 4.8 (1M context) --- components/m5stack-tab5/src/video.cpp | 41 ++++++++++++++++++--------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/components/m5stack-tab5/src/video.cpp b/components/m5stack-tab5/src/video.cpp index cc3b7b803..6ffd982d2 100755 --- a/components/m5stack-tab5/src/video.cpp +++ b/components/m5stack-tab5/src/video.cpp @@ -329,17 +329,25 @@ bool M5StackTab5::initialize_display(size_t pixel_buffer_size) { size_t required_bytes = pixel_buffer_size * sizeof(uint16_t); required_bytes = (required_bytes + kCacheAlign - 1) / kCacheAlign * kCacheAlign; + // 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 + // different pixel buffer size) cannot leak the previous allocation. if (third_buffer == nullptr || third_buffer_bytes != required_bytes) { if (third_buffer != nullptr) { - free(third_buffer); + heap_caps_free(third_buffer); 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); if (third_buffer == nullptr) { - logger_.error("Could not allocate the rotation scratch buffer ({} bytes)", third_buffer_bytes); + // 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 + // rotation (settable at runtime) would silently flush incorrect output. + // Fail initialization rather than come up with rotation quietly broken. + logger_.error("Could not allocate the rotation scratch buffer ({} bytes)", required_bytes); third_buffer_bytes = 0; + return false; } } @@ -412,9 +420,11 @@ float M5StackTab5::brightness() const { // DSI write helpers // ----------------- -void IRAM_ATTR M5StackTab5::flush(lv_display_t *disp, const lv_area_t *area, uint8_t *px_map) { - // Note: This function may be called from ISR context via DPI callback - // Avoid using floating-point operations, logging, or other coprocessor functions +void M5StackTab5::flush(lv_display_t *disp, const lv_area_t *area, uint8_t *px_map) { + // This is LVGL's flush callback; it runs in the LVGL task context (from + // lv_display_flush / the LVGL timer), not from an ISR - the DPI transfer-done + // 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) { lv_display_flush_ready(disp); @@ -431,12 +441,13 @@ void IRAM_ATTR M5StackTab5::flush(lv_display_t *disp, const lv_area_t *area, uin 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. NOTE: the PPA rotates COUNTER-clockwise - // while LVGL's rotation is clockwise, so LVGL 90 -> PPA 270 and LVGL 270 - // -> PPA 90 (180 is the same). If the rotated image comes out turned the - // wrong way, swap the 90/270 mapping here. For 90/270 the output picture - // width/height are swapped. + // 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. 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) { @@ -466,9 +477,13 @@ void IRAM_ATTR M5StackTab5::flush(lv_display_t *disp, const lv_area_t *area, uin srm.scale_x = 1.0f; srm.scale_y = 1.0f; srm.mode = PPA_TRANS_MODE_BLOCKING; - ppa_do_scale_rotate_mirror(g_ppa_client, &srm); - } else { - // Software fallback (used only if the PPA client failed to register) + // 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); + } + if (!rotated) { + // Software fallback: the PPA client failed to register or the PPA + // operation failed. Rotates into third_buffer, fully overwriting it. 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) {