Fixes some bugs in the RGB bus driver

Author: kdschlosser

ext_mod/lcd_bus/esp32_include/rgb_bus.h

@@ -71,7 +71,7 @@
             rgb_bus_lock_t copy_lock;
             rgb_bus_event_t copy_task_exit;
             rgb_bus_event_t last_update;
-            rgb_bus_event_t partial_copy;
+            rgb_bus_lock_t tx_color_lock;
             rgb_bus_event_t swap_bufs;
             rgb_bus_lock_t swap_lock;
 

ext_mod/lcd_bus/esp32_src/rgb_bus.c

@@ -216,45 +216,45 @@
         self->panel_io_config.data_width = (size_t) i;
 
     #if CONFIG_LCD_ENABLE_DEBUG_LOG
-        mp_printf("pclk_hz=%lu\n", self->bus_config.pclk_hz);
-        mp_printf("hsync_pulse_width=%lu\n", self->bus_config.hsync_pulse_width);
-        mp_printf("hsync_back_porch=%lu\n", self->bus_config.hsync_back_porch);
-        mp_printf("hsync_front_porch=%lu\n", self->bus_config.hsync_front_porch);
-        mp_printf("vsync_pulse_width=%lu\n", self->bus_config.vsync_pulse_width);
-        mp_printf("vsync_back_porch=%lu\n", self->bus_config.vsync_back_porch);
-        mp_printf("vsync_front_porch=%lu\n", self->bus_config.vsync_front_porch);
-        mp_printf("hsync_idle_low=%d\n", self->bus_config.flags.hsync_idle_low);
-        mp_printf("vsync_idle_low=%d\n", self->bus_config.flags.vsync_idle_low);
-        mp_printf("de_idle_high=%d\n", self->bus_config.flags.de_idle_high);
-        mp_printf("pclk_active_neg=%d\n", self->bus_config.flags.pclk_active_neg);
-        mp_printf("pclk_idle_high=%d\n", self->bus_config.flags.pclk_idle_high);
-        mp_printf("clk_src=%d\n", self->panel_io_config.clk_src);
-        mp_printf("hsync_gpio_num=%d\n", self->panel_io_config.hsync_gpio_num);
-        mp_printf("vsync_gpio_num=%d\n", self->panel_io_config.vsync_gpio_num);
-        mp_printf("de_gpio_num=%d\n", self->panel_io_config.de_gpio_num);
-        mp_printf("pclk_gpio_num=%d\n", self->panel_io_config.pclk_gpio_num);
-        mp_printf("data_gpio_nums[0]=%d\n", self->panel_io_config.data_gpio_nums[0]);
-        mp_printf("data_gpio_nums[1]=%d\n", self->panel_io_config.data_gpio_nums[1]);
-        mp_printf("data_gpio_nums[2]=%d\n", self->panel_io_config.data_gpio_nums[2]);
-        mp_printf("data_gpio_nums[3]=%d\n", self->panel_io_config.data_gpio_nums[3]);
-        mp_printf("data_gpio_nums[4]=%d\n", self->panel_io_config.data_gpio_nums[4]);
-        mp_printf("data_gpio_nums[5]=%d\n", self->panel_io_config.data_gpio_nums[5]);
-        mp_printf("data_gpio_nums[6]=%d\n", self->panel_io_config.data_gpio_nums[6]);
-        mp_printf("data_gpio_nums[7]=%d\n", self->panel_io_config.data_gpio_nums[7]);
-        mp_printf("data_gpio_nums[8]=%d\n", self->panel_io_config.data_gpio_nums[8]);
-        mp_printf("data_gpio_nums[9]=%d\n", self->panel_io_config.data_gpio_nums[9]);
-        mp_printf("data_gpio_nums[10]=%d\n", self->panel_io_config.data_gpio_nums[10]);
-        mp_printf("data_gpio_nums[11]=%d\n", self->panel_io_config.data_gpio_nums[11]);
-        mp_printf("data_gpio_nums[12]=%d\n", self->panel_io_config.data_gpio_nums[12]);
-        mp_printf("data_gpio_nums[13]=%d\n", self->panel_io_config.data_gpio_nums[13]);
-        mp_printf("data_gpio_nums[14]=%d\n", self->panel_io_config.data_gpio_nums[14]);
-        mp_printf("data_gpio_nums[15]=%d\n", self->panel_io_config.data_gpio_nums[15]);
-        mp_printf("sram_trans_align=%d\n", self->panel_io_config.sram_trans_align);
-        mp_printf("psram_trans_align=%d\n", self->panel_io_config.psram_trans_align);
-        mp_printf("refresh_on_demand=%d\n", self->panel_io_config.flags.refresh_on_demand);
-        mp_printf("fb_in_psram=%d\n", self->panel_io_config.flags.fb_in_psram);
-        mp_printf("double_fb=%d\n", self->panel_io_config.flags.double_fb);
-        mp_printf("data_width=%d\n", self->panel_io_config.data_width);
+        mp_printf(&mp_plat_print, "pclk_hz=%lu\n", self->bus_config.pclk_hz);
+        mp_printf(&mp_plat_print, "hsync_pulse_width=%lu\n", self->bus_config.hsync_pulse_width);
+        mp_printf(&mp_plat_print, "hsync_back_porch=%lu\n", self->bus_config.hsync_back_porch);
+        mp_printf(&mp_plat_print, "hsync_front_porch=%lu\n", self->bus_config.hsync_front_porch);
+        mp_printf(&mp_plat_print, "vsync_pulse_width=%lu\n", self->bus_config.vsync_pulse_width);
+        mp_printf(&mp_plat_print, "vsync_back_porch=%lu\n", self->bus_config.vsync_back_porch);
+        mp_printf(&mp_plat_print, "vsync_front_porch=%lu\n", self->bus_config.vsync_front_porch);
+        mp_printf(&mp_plat_print, "hsync_idle_low=%d\n", self->bus_config.flags.hsync_idle_low);
+        mp_printf(&mp_plat_print, "vsync_idle_low=%d\n", self->bus_config.flags.vsync_idle_low);
+        mp_printf(&mp_plat_print, "de_idle_high=%d\n", self->bus_config.flags.de_idle_high);
+        mp_printf(&mp_plat_print, "pclk_active_neg=%d\n", self->bus_config.flags.pclk_active_neg);
+        mp_printf(&mp_plat_print, "pclk_idle_high=%d\n", self->bus_config.flags.pclk_idle_high);
+        mp_printf(&mp_plat_print, "clk_src=%d\n", self->panel_io_config.clk_src);
+        mp_printf(&mp_plat_print, "hsync_gpio_num=%d\n", self->panel_io_config.hsync_gpio_num);
+        mp_printf(&mp_plat_print, "vsync_gpio_num=%d\n", self->panel_io_config.vsync_gpio_num);
+        mp_printf(&mp_plat_print, "de_gpio_num=%d\n", self->panel_io_config.de_gpio_num);
+        mp_printf(&mp_plat_print, "pclk_gpio_num=%d\n", self->panel_io_config.pclk_gpio_num);
+        mp_printf(&mp_plat_print, "data_gpio_nums[0]=%d\n", self->panel_io_config.data_gpio_nums[0]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[1]=%d\n", self->panel_io_config.data_gpio_nums[1]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[2]=%d\n", self->panel_io_config.data_gpio_nums[2]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[3]=%d\n", self->panel_io_config.data_gpio_nums[3]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[4]=%d\n", self->panel_io_config.data_gpio_nums[4]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[5]=%d\n", self->panel_io_config.data_gpio_nums[5]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[6]=%d\n", self->panel_io_config.data_gpio_nums[6]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[7]=%d\n", self->panel_io_config.data_gpio_nums[7]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[8]=%d\n", self->panel_io_config.data_gpio_nums[8]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[9]=%d\n", self->panel_io_config.data_gpio_nums[9]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[10]=%d\n", self->panel_io_config.data_gpio_nums[10]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[11]=%d\n", self->panel_io_config.data_gpio_nums[11]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[12]=%d\n", self->panel_io_config.data_gpio_nums[12]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[13]=%d\n", self->panel_io_config.data_gpio_nums[13]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[14]=%d\n", self->panel_io_config.data_gpio_nums[14]);
+        mp_printf(&mp_plat_print, "data_gpio_nums[15]=%d\n", self->panel_io_config.data_gpio_nums[15]);
+        mp_printf(&mp_plat_print, "sram_trans_align=%d\n", self->panel_io_config.sram_trans_align);
+        mp_printf(&mp_plat_print, "psram_trans_align=%d\n", self->panel_io_config.psram_trans_align);
+        mp_printf(&mp_plat_print, "refresh_on_demand=%d\n", self->panel_io_config.flags.refresh_on_demand);
+        mp_printf(&mp_plat_print, "fb_in_psram=%d\n", self->panel_io_config.flags.fb_in_psram);
+        mp_printf(&mp_plat_print, "double_fb=%d\n", self->panel_io_config.flags.double_fb);
+        mp_printf(&mp_plat_print, "data_width=%d\n", self->panel_io_config.data_width);
     #endif
 
         self->panel_io_handle.get_lane_count = &rgb_get_lane_count;
@@ -274,10 +274,30 @@
         mp_lcd_rgb_bus_obj_t *self = (mp_lcd_rgb_bus_obj_t *)obj;
 
     #if CONFIG_LCD_ENABLE_DEBUG_LOG
-        mp_printf("rgb_del(self)\n");
+        mp_printf(&mp_plat_print, "rgb_del(self)\n");
     #endif
 
+        if (self->view1 != NULL || self->view2 != NULL) {
+            mp_raise_msg(&mp_type_MemoryError, MP_ERROR_TEXT("Framebuffers have not been released"));
+            return mp_const_none;
+        }
+
+        rgb_bus_lock_acquire(&self->tx_color_lock, -1);
+        self->partial_buf = NULL;
+        rgb_bus_event_set(&self->copy_task_exit);
+        rgb_bus_lock_release(&self->copy_lock);
+        rgb_bus_lock_release(&self->tx_color_lock);
+
         mp_lcd_err_t ret = esp_lcd_panel_del(self->panel_handle);
+
+        rgb_bus_lock_delete(&self->copy_lock);
+        rgb_bus_lock_delete(&self->tx_color_lock);
+        rgb_bus_lock_delete(&self->swap_lock);
+
+        rgb_bus_event_delete(&self->swap_bufs);
+        rgb_bus_event_delete(&self->last_update);
+        rgb_bus_event_delete(&self->copy_task_exit);
+
         return ret;
     }
 
@@ -287,7 +307,7 @@
         LCD_UNUSED(param);
 
     #if CONFIG_LCD_ENABLE_DEBUG_LOG
-        mp_printf("rgb_rx_param(self, lcd_cmd=%d, param, param_size=%d)\n", lcd_cmd, param_size);
+        mp_printf(&mp_plat_print, "rgb_rx_param(self, lcd_cmd=%d, param, param_size=%d)\n", lcd_cmd, param_size);
     #else
         LCD_UNUSED(lcd_cmd);
         LCD_UNUSED(param_size);
@@ -302,7 +322,7 @@
         LCD_UNUSED(param);
 
     #if CONFIG_LCD_ENABLE_DEBUG_LOG
-        mp_printf("rgb_tx_param(self, lcd_cmd=%d, param, param_size=%d)\n", lcd_cmd, param_size);
+        mp_printf(&mp_plat_print, "rgb_tx_param(self, lcd_cmd=%d, param, param_size=%d)\n", lcd_cmd, param_size);
     #else
         LCD_UNUSED(lcd_cmd);
         LCD_UNUSED(param_size);
@@ -327,13 +347,13 @@
             heap_caps_free(item_buf);
             self->view1 = NULL;
         #if CONFIG_LCD_ENABLE_DEBUG_LOG
-            mp_printf("rgb_free_framebuffer(self, buf=1)\n");
+            mp_printf(&mp_plat_print, "rgb_free_framebuffer(self, buf=1)\n");
         #endif
         } else if (array_buf == self->view2) {
             heap_caps_free(item_buf);
             self->view2 = NULL;
         #if CONFIG_LCD_ENABLE_DEBUG_LOG
-            mp_printf("rgb_free_framebuffer(self, buf=2)\n");
+            mp_printf(&mp_plat_print, "rgb_free_framebuffer(self, buf=2)\n");
         #endif
         } else {
             mp_raise_msg(&mp_type_MemoryError, MP_ERROR_TEXT("No matching buffer found"));
@@ -344,7 +364,7 @@
     mp_obj_t rgb_allocate_framebuffer(mp_obj_t obj, uint32_t size, uint32_t caps)
     {
     #if CONFIG_LCD_ENABLE_DEBUG_LOG
-        mp_printf("rgb_allocate_framebuffer(self, size=%lu, caps=%lu)\n", size, caps);
+        mp_printf(&mp_plat_print, "rgb_allocate_framebuffer(self, size=%lu, caps=%lu)\n", size, caps);
     #endif
 
         mp_lcd_rgb_bus_obj_t *self = (mp_lcd_rgb_bus_obj_t *)obj;
@@ -380,7 +400,7 @@
         LCD_UNUSED(cmd_bits);
         LCD_UNUSED(param_bits);
     #if CONFIG_LCD_ENABLE_DEBUG_LOG
-        mp_printf("rgb_init(self, width=%i, height=%i, bpp=%d, buffer_size=%lu, rgb565_byte_swap=%d)\n", width, height, bpp, buffer_size, rgb565_byte_swap);
+        mp_printf(&mp_plat_print, "rgb_init(self, width=%i, height=%i, bpp=%d, buffer_size=%lu, rgb565_byte_swap=%d)\n", width, height, bpp, buffer_size, rgb565_byte_swap);
     #endif
         mp_lcd_rgb_bus_obj_t *self = (mp_lcd_rgb_bus_obj_t *)obj;
 
@@ -422,10 +442,10 @@
         self->panel_io_config.flags.double_fb = 1;
 
     #if CONFIG_LCD_ENABLE_DEBUG_LOG
-        mp_printf("h_res=%lu\n", self->panel_io_config.timings.h_res);
-        mp_printf("v_res=%lu\n", self->panel_io_config.timings.v_res);
-        mp_printf("bits_per_pixel=%d\n", self->panel_io_config.bits_per_pixel);
-        mp_printf("rgb565_byte_swap=%d\n", self->rgb565_byte_swap);
+        mp_printf(&mp_plat_print, "h_res=%lu\n", self->panel_io_config.timings.h_res);
+        mp_printf(&mp_plat_print, "v_res=%lu\n", self->panel_io_config.timings.v_res);
+        mp_printf(&mp_plat_print, "bits_per_pixel=%d\n", self->panel_io_config.bits_per_pixel);
+        mp_printf(&mp_plat_print, "rgb565_byte_swap=%d\n", self->rgb565_byte_swap);
     #endif
         mp_lcd_err_t ret = esp_lcd_new_rgb_panel(&self->panel_io_config, &self->panel_handle);
         if (ret != 0) {
@@ -459,9 +479,9 @@
         self->idle_fb = rgb_panel->fbs[0];
 
         rgb_bus_lock_init(&self->copy_lock);
+        rgb_bus_lock_init(&self->tx_color_lock);
         rgb_bus_event_init(&self->copy_task_exit);
         rgb_bus_event_init(&self->last_update);
-        rgb_bus_event_init(&self->partial_copy);
         rgb_bus_event_init(&self->swap_bufs);
         rgb_bus_lock_init(&self->swap_lock);
 
@@ -478,7 +498,7 @@
         mp_lcd_rgb_bus_obj_t *self = (mp_lcd_rgb_bus_obj_t *)obj;
         *lane_count = (uint8_t)self->panel_io_config.data_width;
     #if CONFIG_LCD_ENABLE_DEBUG_LOG
-        mp_printf("rgb_get_lane_count(self)-> %d\n", (uint8_t)self->panel_io_config.data_width);
+        mp_printf(&mp_plat_print, "rgb_get_lane_count(self)-> %d\n", (uint8_t)self->panel_io_config.data_width);
     #endif
 
         return LCD_OK;
@@ -488,12 +508,15 @@
     mp_lcd_err_t rgb_tx_color(mp_obj_t obj, int lcd_cmd, void *color, size_t color_size, int x_start, int y_start, int x_end, int y_end, uint8_t rotation, bool last_update)
     {
     #if CONFIG_LCD_ENABLE_DEBUG_LOG
-        mp_printf("rgb_tx_color(self, lcd_cmd=%d, color, color_size=%d, x_start=%d, y_start=%d, x_end=%d, y_end=%d)\n", lcd_cmd, color_size, x_start, y_start, x_end, y_end);
+        mp_printf(&mp_plat_print, "rgb_tx_color(self, lcd_cmd=%d, color, color_size=%d, x_start=%d, y_start=%d, x_end=%d, y_end=%d)\n", lcd_cmd, color_size, x_start, y_start, x_end, y_end);
     #endif
 
         LCD_UNUSED(color_size);
 
         mp_lcd_rgb_bus_obj_t *self = (mp_lcd_rgb_bus_obj_t *)obj;
+
+        rgb_bus_lock_acquire(&self->tx_color_lock, -1);
+
         self->partial_buf = (uint8_t *)color;
         self->x_start = x_start;
         self->y_start = y_start;