rgbmatrix: update protomatter

and re-organize so that esp32 s2/s3 don't do as much at reset

.. it's not necessary (because most data is in esp-idf managed memory)
and doing this saves me from having to debug why reconstruct isn't working
properly on that platform.

This needs to be tested on other platforms again before being merged!

Author: jepler

lib/protomatter

@@ -31,7 +31,7 @@
 #include "peripherals/timer.h"
 
 void *common_hal_rgbmatrix_timer_allocate(rgbmatrix_rgbmatrix_obj_t *self) {
-    const timer_config_t config = {
+    static const timer_config_t config = {
         .alarm_en = false,
         .counter_en = false,
         .intr_type = TIMER_INTR_LEVEL,
@@ -43,6 +43,7 @@ void *common_hal_rgbmatrix_timer_allocate(rgbmatrix_rgbmatrix_obj_t *self) {
     timer_index_t *timer = malloc(sizeof(timer_index_t));
     bool res = peripherals_timer_init(&config, timer);
     if (!res) {
+        free(timer);
         return NULL;
     }
     peripherals_timer_never_reset(timer);
@@ -52,6 +53,11 @@ void *common_hal_rgbmatrix_timer_allocate(rgbmatrix_rgbmatrix_obj_t *self) {
 extern bool _PM_esp32timerCallback(void *arg);
 
 void common_hal_rgbmatrix_timer_enable(void *ptr) {
+    timer_index_t *timer = (timer_index_t *)ptr;
+    if (timer->idx == TIMER_MAX) {
+        return;
+    }
+    timer_start(timer->group, timer->idx);
 }
 
 void common_hal_rgbmatrix_timer_disable(void *ptr) {
@@ -60,8 +66,6 @@ void common_hal_rgbmatrix_timer_disable(void *ptr) {
         return;
     }
     timer_pause(timer->group, timer->idx);
-    timer_disable_intr(timer->group, timer->idx);
-    timer_isr_callback_remove(timer->group, timer->idx);
 }
 
 void common_hal_rgbmatrix_timer_free(void *ptr) {
@@ -70,6 +74,8 @@ void common_hal_rgbmatrix_timer_free(void *ptr) {
         return;
     }
     common_hal_rgbmatrix_timer_disable(ptr);
+    timer_disable_intr(timer->group, timer->idx);
+    timer_isr_callback_remove(timer->group, timer->idx);
     peripherals_timer_deinit(timer);
-    timer->idx = TIMER_MAX;
+    free(timer);
 }

ports/espressif/common-hal/rgbmatrix/RGBMatrix.c

@@ -70,10 +70,12 @@ CIRCUITPY_USB = 0
 else ifeq ($(IDF_TARGET),esp32s2)
 # Modules
 CIRCUITPY_BLEIO = 0
+CIRCUITPY_RGBMATRIX_USES_SUPERVISOR_ALLOCATION = 0
 
 else ifeq ($(IDF_TARGET),esp32s3)
 # Modules
 CIRCUITPY_PARALLELDISPLAY = 0
+CIRCUITPY_RGBMATRIX_USES_SUPERVISOR_ALLOCATION = 0
 endif
 
 # No room for dualbank on boards with 2MB flash

ports/espressif/mpconfigport.mk

@@ -385,6 +385,8 @@ CFLAGS += -DCIRCUITPY_RP2PIO=$(CIRCUITPY_RP2PIO)
 
 CIRCUITPY_RGBMATRIX ?= 0
 CFLAGS += -DCIRCUITPY_RGBMATRIX=$(CIRCUITPY_RGBMATRIX)
+CIRCUITPY_RGBMATRIX_USES_SUPERVISOR_ALLOCATION ?= 1
+CFLAGS += -DCIRCUITPY_RGBMATRIX_USES_SUPERVISOR_ALLOCATION=$(CIRCUITPY_RGBMATRIX_USES_SUPERVISOR_ALLOCATION)
 
 CIRCUITPY_ROTARYIO ?= 1
 CFLAGS += -DCIRCUITPY_ROTARYIO=$(CIRCUITPY_ROTARYIO)

py/circuitpy_mpconfig.mk

@@ -62,13 +62,16 @@ STATIC void claim_and_never_reset_pins(mp_obj_t seq) {
 }
 
 STATIC void preflight_pins_or_throw(uint8_t clock_pin, uint8_t *rgb_pins, uint8_t rgb_pin_count, bool allow_inefficient) {
-    uint32_t port = clock_pin / 32;
-    uint32_t bit_mask = 1 << (clock_pin % 32);
-
     if (rgb_pin_count <= 0 || rgb_pin_count % 6 != 0 || rgb_pin_count > 30) {
         mp_raise_ValueError_varg(translate("The length of rgb_pins must be 6, 12, 18, 24, or 30"));
     }
 
+// Most ports have a strict requirement for how the rgbmatrix pins are laid
+// out; these two micros don't. Special-case it here.
+    #if !defined(CONFIG_IDF_TARGET_ESP32S3) && !defined(CONFIG_IDF_TARGET_ESP32S2)
+    uint32_t port = clock_pin / 32;
+    uint32_t bit_mask = 1 << (clock_pin % 32);
+
     for (uint8_t i = 0; i < rgb_pin_count; i++) {
         uint32_t pin_port = rgb_pins[i] / 32;
 
@@ -130,6 +133,7 @@ STATIC void preflight_pins_or_throw(uint8_t clock_pin, uint8_t *rgb_pins, uint8_
             translate("Pinout uses %d bytes per element, which consumes more than the ideal %d bytes.  If this cannot be avoided, pass allow_inefficient=True to the constructor"),
             bytes_per_element, ideal_bytes_per_element);
     }
+    #endif
 }
 
 //|     def __init__(
@@ -236,20 +240,14 @@ STATIC mp_obj_t rgbmatrix_rgbmatrix_make_new(const mp_obj_type_t *type, size_t n
 
     preflight_pins_or_throw(clock_pin, rgb_pins, rgb_count, true);
 
-    mp_obj_t framebuffer = args[ARG_framebuffer].u_obj;
-    if (framebuffer == mp_const_none) {
-        int bufsize = 2 * width * computed_height;
-        framebuffer = mp_obj_new_bytearray_of_zeros(bufsize);
-    }
-
     common_hal_rgbmatrix_rgbmatrix_construct(self,
         width,
         bit_depth,
         rgb_count, rgb_pins,
         addr_count, addr_pins,
         clock_pin, latch_pin, output_enable_pin,
         args[ARG_doublebuffer].u_bool,
-        framebuffer, tile, args[ARG_serpentine].u_bool, NULL);
+        args[ARG_framebuffer].u_obj, tile, args[ARG_serpentine].u_bool, NULL);
 
     claim_and_never_reset_pins(args[ARG_rgb_list].u_obj);
     claim_and_never_reset_pins(args[ARG_addr_list].u_obj);
@@ -353,7 +351,6 @@ STATIC MP_DEFINE_CONST_DICT(rgbmatrix_rgbmatrix_locals_dict, rgbmatrix_rgbmatrix
 
 STATIC void rgbmatrix_rgbmatrix_get_bufinfo(mp_obj_t self_in, mp_buffer_info_t *bufinfo) {
     rgbmatrix_rgbmatrix_obj_t *self = (rgbmatrix_rgbmatrix_obj_t *)self_in;
-    check_for_deinit(self);
 
     *bufinfo = self->bufinfo;
 }

shared-bindings/rgbmatrix/RGBMatrix.c

@@ -34,7 +34,7 @@ extern const mp_obj_type_t rgbmatrix_RGBMatrix_type;
 void common_hal_rgbmatrix_rgbmatrix_construct(rgbmatrix_rgbmatrix_obj_t *self, int width, int bit_depth, uint8_t rgb_count, uint8_t *rgb_pins, uint8_t addr_count, uint8_t *addr_pins, uint8_t clock_pin, uint8_t latch_pin, uint8_t oe_pin, bool doublebuffer, mp_obj_t framebuffer, int8_t tile, bool serpentine, void *timer);
 void common_hal_rgbmatrix_rgbmatrix_deinit(rgbmatrix_rgbmatrix_obj_t *);
 void rgbmatrix_rgbmatrix_collect_ptrs(rgbmatrix_rgbmatrix_obj_t *);
-void common_hal_rgbmatrix_rgbmatrix_reconstruct(rgbmatrix_rgbmatrix_obj_t *self, mp_obj_t framebuffer);
+void common_hal_rgbmatrix_rgbmatrix_reconstruct(rgbmatrix_rgbmatrix_obj_t *self);
 void common_hal_rgbmatrix_rgbmatrix_set_paused(rgbmatrix_rgbmatrix_obj_t *self, bool paused);
 bool common_hal_rgbmatrix_rgbmatrix_get_paused(rgbmatrix_rgbmatrix_obj_t *self);
 void common_hal_rgbmatrix_rgbmatrix_refresh(rgbmatrix_rgbmatrix_obj_t *self);

shared-bindings/rgbmatrix/RGBMatrix.h

@@ -42,6 +42,8 @@
 
 extern Protomatter_core *_PM_protoPtr;
 
+STATIC void common_hal_rgbmatrix_rgbmatrix_construct1(rgbmatrix_rgbmatrix_obj_t *self, mp_obj_t framebuffer);
+
 void common_hal_rgbmatrix_rgbmatrix_construct(rgbmatrix_rgbmatrix_obj_t *self, int width, int bit_depth, uint8_t rgb_count, uint8_t *rgb_pins, uint8_t addr_count, uint8_t *addr_pins, uint8_t clock_pin, uint8_t latch_pin, uint8_t oe_pin, bool doublebuffer, mp_obj_t framebuffer, int8_t tile, bool serpentine, void *timer) {
     self->width = width;
     self->bit_depth = bit_depth;
@@ -64,15 +66,11 @@ void common_hal_rgbmatrix_rgbmatrix_construct(rgbmatrix_rgbmatrix_obj_t *self, i
     self->width = width;
     self->bufsize = 2 * width * common_hal_rgbmatrix_rgbmatrix_get_height(self);
 
-    common_hal_rgbmatrix_rgbmatrix_reconstruct(self, framebuffer);
+    common_hal_rgbmatrix_rgbmatrix_construct1(self, framebuffer);
 }
 
-void common_hal_rgbmatrix_rgbmatrix_reconstruct(rgbmatrix_rgbmatrix_obj_t *self, mp_obj_t framebuffer) {
-    self->paused = 1;
-
-    common_hal_rgbmatrix_timer_disable(self->timer);
-    if (framebuffer) {
-        self->framebuffer = framebuffer;
+STATIC void common_hal_rgbmatrix_rgbmatrix_construct1(rgbmatrix_rgbmatrix_obj_t *self, mp_obj_t framebuffer) {
+    if (framebuffer != mp_const_none) {
         mp_get_buffer_raise(self->framebuffer, &self->bufinfo, MP_BUFFER_READ);
         if (mp_get_buffer(self->framebuffer, &self->bufinfo, MP_BUFFER_RW)) {
             self->bufinfo.typecode = 'H' | MP_OBJ_ARRAY_TYPECODE_FLAG_RW;
@@ -82,16 +80,11 @@ void common_hal_rgbmatrix_rgbmatrix_reconstruct(rgbmatrix_rgbmatrix_obj_t *self,
         // verify that the matrix is big enough
         mp_get_index(mp_obj_get_type(self->framebuffer), self->bufinfo.len, MP_OBJ_NEW_SMALL_INT(self->bufsize - 1), false);
     } else {
-        common_hal_rgbmatrix_free_impl(self->bufinfo.buf);
-        common_hal_rgbmatrix_free_impl(self->protomatter.rgbPins);
-        common_hal_rgbmatrix_free_impl(self->protomatter.addr);
-        common_hal_rgbmatrix_free_impl(self->protomatter.screenData);
-
-        self->framebuffer = NULL;
         self->bufinfo.buf = common_hal_rgbmatrix_allocator_impl(self->bufsize);
         self->bufinfo.len = self->bufsize;
         self->bufinfo.typecode = 'H' | MP_OBJ_ARRAY_TYPECODE_FLAG_RW;
     }
+    self->framebuffer = framebuffer;
 
     memset(&self->protomatter, 0, sizeof(self->protomatter));
     ProtomatterStatus stat = _PM_init(&self->protomatter,
@@ -115,6 +108,9 @@ void common_hal_rgbmatrix_rgbmatrix_reconstruct(rgbmatrix_rgbmatrix_obj_t *self,
 
     if (stat != PROTOMATTER_OK) {
         common_hal_rgbmatrix_rgbmatrix_deinit(self);
+        if (!gc_alloc_possible()) {
+            return;
+        }
         switch (stat) {
             case PROTOMATTER_ERR_PINS:
                 raise_ValueError_invalid_pin();
@@ -148,35 +144,65 @@ STATIC void free_pin_seq(uint8_t *seq, int count) {
     }
 }
 
-void common_hal_rgbmatrix_rgbmatrix_deinit(rgbmatrix_rgbmatrix_obj_t *self) {
-    if (self->timer) {
-        common_hal_rgbmatrix_timer_free(self->timer);
-        self->timer = 0;
-    }
+extern int pm_row_count;
+STATIC void common_hal_rgbmatrix_rgbmatrix_deinit1(rgbmatrix_rgbmatrix_obj_t *self) {
+    common_hal_rgbmatrix_timer_disable(self->timer);
 
     if (_PM_protoPtr == &self->protomatter) {
         _PM_protoPtr = NULL;
     }
 
-    free_pin_seq(self->rgb_pins, self->rgb_count);
-    free_pin_seq(self->addr_pins, self->addr_count);
-    free_pin(&self->clock_pin);
-    free_pin(&self->latch_pin);
-    free_pin(&self->oe_pin);
-
     if (self->protomatter.rgbPins) {
         _PM_deallocate(&self->protomatter);
     }
+
     memset(&self->protomatter, 0, sizeof(self->protomatter));
 
     // If it was supervisor-allocated, it is supervisor-freed and the pointer
     // is zeroed, otherwise the pointer is just zeroed
-    _PM_free(self->bufinfo.buf);
-    self->base.type = NULL;
+    if (self->bufinfo.buf) {
+        common_hal_rgbmatrix_free_impl(self->bufinfo.buf);
+        self->bufinfo.buf = NULL;
+    }
+
 
-    // If a framebuffer was passed in to the constructor, NULL the reference
+    // If a framebuffer was passed in to the constructor, clear the reference
     // here so that it will become GC'able
-    self->framebuffer = NULL;
+    self->framebuffer = mp_const_none;
+}
+
+void common_hal_rgbmatrix_rgbmatrix_deinit(rgbmatrix_rgbmatrix_obj_t *self) {
+    common_hal_rgbmatrix_rgbmatrix_deinit1(self);
+    if (self->timer) {
+        common_hal_rgbmatrix_timer_free(self->timer);
+        self->timer = 0;
+    }
+
+    free_pin_seq(self->rgb_pins, self->rgb_count);
+    free_pin_seq(self->addr_pins, self->addr_count);
+    free_pin(&self->clock_pin);
+    free_pin(&self->latch_pin);
+    free_pin(&self->oe_pin);
+
+    self->base.type = &mp_type_NoneType;
+}
+
+void common_hal_rgbmatrix_rgbmatrix_reconstruct(rgbmatrix_rgbmatrix_obj_t *self) {
+    if (self->framebuffer != mp_const_none) {
+        memset(&self->bufinfo, 0, sizeof(self->bufinfo));
+    }
+    #if CIRCUITPY_RGBMATRIX_USES_SUPERVISOR_ALLOCATION
+    common_hal_rgbmatrix_rgbmatrix_set_paused(self, true);
+    common_hal_rgbmatrix_rgbmatrix_deinit1(self);
+    common_hal_rgbmatrix_rgbmatrix_construct1(self, mp_const_none);
+    #endif
+    if (self->bufinfo.buf == NULL) {
+        self->bufinfo.buf = common_hal_rgbmatrix_allocator_impl(self->bufsize);
+        self->bufinfo.len = self->bufsize;
+        self->bufinfo.typecode = 'H' | MP_OBJ_ARRAY_TYPECODE_FLAG_RW;
+    }
+    memset(self->bufinfo.buf, 0, self->bufinfo.len);
+    common_hal_rgbmatrix_rgbmatrix_set_paused(self, false);
 }
 
 void rgbmatrix_rgbmatrix_collect_ptrs(rgbmatrix_rgbmatrix_obj_t *self) {

shared-module/rgbmatrix/RGBMatrix.c

@@ -31,7 +31,15 @@
 #include "py/misc.h"
 #include "supervisor/memory.h"
 
+#if defined(CONFIG_IDF_TARGET_ESP32S3) || defined(CONFIG_IDF_TARGET_ESP32S2)
+// Use DMA-capable RAM (not PSRAM) for framebuffer:
+#include "components/heap/include/esp_heap_caps.h"
+#define _PM_allocate(x) heap_caps_malloc(x, MALLOC_CAP_DMA | MALLOC_CAP_8BIT)
+#define _PM_free(x) heap_caps_free(x)
+#else
 #define _PM_allocate common_hal_rgbmatrix_allocator_impl
 #define _PM_free(x) (common_hal_rgbmatrix_free_impl((x)), (x) = NULL, (void)0)
+#endif
+
 extern void *common_hal_rgbmatrix_allocator_impl(size_t sz);
 extern void common_hal_rgbmatrix_free_impl(void *);

shared-module/rgbmatrix/allocator.h

@@ -191,7 +191,7 @@ void supervisor_display_move_memory(void) {
         #if CIRCUITPY_RGBMATRIX
         if (display_buses[i].rgbmatrix.base.type == &rgbmatrix_RGBMatrix_type) {
             rgbmatrix_rgbmatrix_obj_t *pm = &display_buses[i].rgbmatrix;
-            common_hal_rgbmatrix_rgbmatrix_reconstruct(pm, NULL);
+            common_hal_rgbmatrix_rgbmatrix_reconstruct(pm);
         }
         #endif
         #if CIRCUITPY_SHARPDISPLAY