Better reasoned allocations

Author: tannewt

ports/espressif/common-hal/mipidsi/Bus.c

@@ -14,6 +14,10 @@ void common_hal_mipidsi_bus_construct(mipidsi_bus_obj_t *self, mp_uint_t frequen
     self->num_data_lanes = num_lanes;
     self->bus_handle = NULL;
 
+    if (self->use_count > 0) {
+        mp_raise_ValueError_varg(MP_ERROR_TEXT("%q in use"), MP_QSTR_mipidsi);
+    }
+
     esp_ldo_channel_handle_t ldo_mipi_phy = NULL;
     esp_ldo_channel_config_t ldo_mipi_phy_config = {
         .chan_id = 3,
@@ -33,6 +37,9 @@ void common_hal_mipidsi_bus_construct(mipidsi_bus_obj_t *self, mp_uint_t frequen
 }
 
 void common_hal_mipidsi_bus_deinit(mipidsi_bus_obj_t *self) {
+    if (self->use_count > 0) {
+        mp_raise_ValueError_varg(MP_ERROR_TEXT("%q in use"), MP_QSTR_Bus);
+    }
     if (common_hal_mipidsi_bus_deinited(self)) {
         return;
     }
@@ -50,3 +57,13 @@ void common_hal_mipidsi_bus_deinit(mipidsi_bus_obj_t *self) {
 bool common_hal_mipidsi_bus_deinited(mipidsi_bus_obj_t *self) {
     return self->bus_handle == NULL;
 }
+
+void mipidsi_bus_increment_use_count(mipidsi_bus_obj_t *self) {
+    self->use_count++;
+}
+void mipidsi_bus_decrement_use_count(mipidsi_bus_obj_t *self) {
+    self->use_count--;
+    if (self->use_count == 0) {
+        common_hal_mipidsi_bus_deinit(self);
+    }
+}

ports/espressif/common-hal/mipidsi/Bus.h

@@ -14,4 +14,8 @@ typedef struct {
     mp_uint_t frequency;
     esp_lcd_dsi_bus_handle_t bus_handle;
     uint8_t num_data_lanes;
+    uint8_t use_count; // Up to 4 displays
 } mipidsi_bus_obj_t;
+
+void mipidsi_bus_increment_use_count(mipidsi_bus_obj_t *self);
+void mipidsi_bus_decrement_use_count(mipidsi_bus_obj_t *self);

ports/espressif/common-hal/mipidsi/Display.c

@@ -67,7 +67,7 @@ void common_hal_mipidsi_display_construct(mipidsi_display_obj_t *self,
         color_format = LCD_COLOR_FMT_RGB888;
     } else {
         common_hal_mipidsi_display_deinit(self);
-        mp_raise_ValueError(MP_ERROR_TEXT("Color depth must be 16 or 24"));
+        mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q"), MP_QSTR_color_depth);
     }
 
     // Create the DPI panel for sending pixel data
@@ -173,6 +173,7 @@ void common_hal_mipidsi_display_construct(mipidsi_display_obj_t *self,
             common_hal_digitalio_digitalinout_set_value(&self->backlight_inout, on);
         }
     }
+    mipidsi_bus_increment_use_count(self->bus);
 }
 
 void common_hal_mipidsi_display_deinit(mipidsi_display_obj_t *self) {
@@ -202,6 +203,7 @@ void common_hal_mipidsi_display_deinit(mipidsi_display_obj_t *self) {
         self->dbi_io_handle = NULL;
     }
 
+    mipidsi_bus_decrement_use_count(self->bus);
     self->bus = NULL;
     self->framebuffer = NULL;
 }

shared-bindings/mipidsi/Bus.c

@@ -28,6 +28,13 @@
 //|         :param int num_lanes: the number of data lanes to use (default 2, range 1-4)
 //|         """
 //|
+//
+//
+// All MCUs we support only have one DSI bus but it can be shared between multiple displays. One
+// display may live longer than the VM, so we need to allocate the bus outside the VM. To simplify
+// memory tracking, we use a global object for the bus.
+//
+static mipidsi_bus_obj_t _mipidsi_bus_obj;
 
 static mp_obj_t mipidsi_bus_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) {
     enum { ARG_frequency, ARG_num_lanes };
@@ -38,7 +45,8 @@ static mp_obj_t mipidsi_bus_make_new(const mp_obj_type_t *type, size_t n_args, s
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
     mp_arg_parse_all_kw_array(n_args, n_kw, all_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
-    mipidsi_bus_obj_t *self = mp_obj_malloc(mipidsi_bus_obj_t, &mipidsi_bus_type);
+    _mipidsi_bus_obj.base.type = &mipidsi_bus_type;
+    mipidsi_bus_obj_t *self = &_mipidsi_bus_obj;
 
     mp_uint_t frequency = (mp_uint_t)mp_arg_validate_int_min(args[ARG_frequency].u_int, 1, MP_QSTR_frequency);
     uint8_t num_lanes = (uint8_t)mp_arg_validate_int_range(args[ARG_num_lanes].u_int, 1, 4, MP_QSTR_num_lanes);
@@ -62,28 +70,8 @@ static mp_obj_t mipidsi_bus_deinit(mp_obj_t self_in) {
 
 static MP_DEFINE_CONST_FUN_OBJ_1(mipidsi_bus_deinit_obj, mipidsi_bus_deinit);
 
-//|     def __enter__(self) -> Bus:
-//|         """No-op used by Context Managers."""
-//|         ...
-//|
-//  Provided by context manager helper.
-
-//|     def __exit__(self) -> None:
-//|         """Automatically deinitializes the hardware when exiting a context. See
-//|         :ref:`lifetime-and-contextmanagers` for more info."""
-//|         ...
-//|
-static mp_obj_t mipidsi_bus_obj___exit__(size_t n_args, const mp_obj_t *args) {
-    (void)n_args;
-    common_hal_mipidsi_bus_deinit(args[0]);
-    return mp_const_none;
-}
-static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mipidsi_bus___exit___obj, 4, 4, mipidsi_bus_obj___exit__);
-
 static const mp_rom_map_elem_t mipidsi_bus_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&mipidsi_bus_deinit_obj) },
-    { MP_ROM_QSTR(MP_QSTR___enter__), MP_ROM_PTR(&default___enter___obj) },
-    { MP_ROM_QSTR(MP_QSTR___exit__), MP_ROM_PTR(&mipidsi_bus___exit___obj) },
 };
 static MP_DEFINE_CONST_DICT(mipidsi_bus_locals_dict, mipidsi_bus_locals_dict_table);
 

shared-bindings/mipidsi/Display.c

@@ -124,7 +124,7 @@ static mp_obj_t mipidsi_display_make_new(const mp_obj_type_t *type, size_t n_arg
     mp_uint_t height = (mp_uint_t)mp_arg_validate_int_min(args[ARG_height].u_int, 0, MP_QSTR_height);
     mp_uint_t color_depth = args[ARG_color_depth].u_int;
 
-    if (color_depth != 8 && color_depth != 16 && color_depth != 24) {
+    if (color_depth != 16 && color_depth != 24) {
         mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q"), MP_QSTR_color_depth);
     }
 

shared-module/displayio/__init__.c

@@ -41,6 +41,10 @@
 #include "shared-module/aurora_epaper/aurora_framebuffer.h"
 #endif
 
+#if CIRCUITPY_MIPIDSI
+#include "shared-bindings/mipidsi/Display.h"
+#endif
+
 #ifdef BOARD_USE_INTERNAL_SPI
 #include "supervisor/spi_flash_api.h"
 #endif
@@ -64,7 +68,7 @@ displayio_buffer_transform_t null_transform = {
     .transpose_xy = false
 };
 
-#if CIRCUITPY_RGBMATRIX || CIRCUITPY_IS31FL3741 || CIRCUITPY_VIDEOCORE || CIRCUITPY_PICODVI
+#if CIRCUITPY_RGBMATRIX || CIRCUITPY_IS31FL3741 || CIRCUITPY_VIDEOCORE || CIRCUITPY_PICODVI || CIRCUITPY_MIPIDSI
 static bool any_display_uses_this_framebuffer(mp_obj_base_t *obj) {
     for (uint8_t i = 0; i < CIRCUITPY_DISPLAY_LIMIT; i++) {
         if (displays[i].display_base.type == &framebufferio_framebufferdisplay_type) {
@@ -180,6 +184,10 @@ static void common_hal_displayio_release_displays_impl(bool keep_primary) {
         } else if (bus_type == &picodvi_framebuffer_type) {
             common_hal_picodvi_framebuffer_deinit(&display_buses[i].picodvi);
         #endif
+        #if CIRCUITPY_MIPIDSI
+        } else if (bus_type == &mipidsi_display_type) {
+            common_hal_mipidsi_display_deinit(&display_buses[i].mipidsi);
+        #endif
         }
         display_buses[i].bus_base.type = &mp_type_NoneType;
     }
@@ -333,6 +341,13 @@ void reset_displays(void) {
             // Set to None, gets deinit'd up by display_base
             display_buses[i].bus_base.type = &mp_type_NoneType;
         #endif
+        #if CIRCUITPY_MIPIDSI
+        } else if (display_bus_type == &mipidsi_display_type) {
+            mipidsi_display_obj_t *display = &display_buses[i].mipidsi;
+            if (!any_display_uses_this_framebuffer(&display->base)) {
+                common_hal_mipidsi_display_deinit(display);
+            }
+        #endif
         } else {
             // Not an active display bus.
             continue;