From 1117edae63df7b66f1575980bf54d6836f9a8aa8 Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Sat, 24 Oct 2020 17:13:10 +0200 Subject: Fix erroneous claim of board.I2C() by deinited display. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the end of a session that called displayio.release_displays() (and did not initialize a new display), a board.I2C() bus that was previously used by a display would wrongly be considered still in use. While I can’t think of any unrecoverable problem this would cause in the next session, it violates the assumption that a soft reboot resets everything not needed by displays, potentially leading to confusion. By itself, this change does not fix the problem yet - rather, it introduces the same issue as in #3581 for SPI. This needs to be solved in the same way for I2C and SPI. --- shared-module/board/__init__.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'shared-module') diff --git a/shared-module/board/__init__.c b/shared-module/board/__init__.c index 39b68a0f1..856103b7b 100644 --- a/shared-module/board/__init__.c +++ b/shared-module/board/__init__.c @@ -139,7 +139,7 @@ void reset_board_busses(void) { bool display_using_i2c = false; #if CIRCUITPY_DISPLAYIO for (uint8_t i = 0; i < CIRCUITPY_DISPLAY_LIMIT; i++) { - if (displays[i].i2cdisplay_bus.bus == i2c_singleton) { + if (displays[i].bus_base.type == &displayio_i2cdisplay_type && displays[i].i2cdisplay_bus.bus == i2c_singleton) { display_using_i2c = true; break; } -- cgit v1.2.3 From f4f80e07ca3dc2f5a93484e5342540837a94a505 Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Sat, 24 Oct 2020 17:23:17 +0200 Subject: Fix lost board.SPI and board.I2C after being used by display. Fixes #3581. Pins were marked as never_reset by common_hal_displayio_fourwire_construct() and common_hal_sharpdisplay_framebuffer_construct(), but these marks were never removed, so at the end of a session after displayio.release_displays(), {spi|i2c}_singleton would be set to NULL but the pins would not be reset. In the next session, board.SPI() and board.I2C() were unable to reconstruct the object because the pins were still in use. For symmetry with creation of the singleton, add deinitialization before setting it to NULL in reset_board_busses(). This makes the pins resettable, so that reset_port(), moved behind it, then resets them. --- main.c | 4 +++- shared-module/board/__init__.c | 14 +++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) (limited to 'shared-module') diff --git a/main.c b/main.c index b43b3b8c8..80b163f60 100755 --- a/main.c +++ b/main.c @@ -234,10 +234,12 @@ void cleanup_after_vm(supervisor_allocation* heap) { common_hal_canio_reset(); #endif - reset_port(); + // reset_board_busses() first because it may release pins from the never_reset state, so that + // reset_port() can reset them. #if CIRCUITPY_BOARD reset_board_busses(); #endif + reset_port(); reset_board(); reset_status_led(); } diff --git a/shared-module/board/__init__.c b/shared-module/board/__init__.c index 856103b7b..967b430d2 100644 --- a/shared-module/board/__init__.c +++ b/shared-module/board/__init__.c @@ -145,8 +145,11 @@ void reset_board_busses(void) { } } #endif - if (!display_using_i2c) { - i2c_singleton = NULL; + if (i2c_singleton != NULL) { + if (!display_using_i2c) { + common_hal_busio_i2c_deinit(i2c_singleton); + i2c_singleton = NULL; + } } #endif #if BOARD_SPI @@ -169,9 +172,10 @@ void reset_board_busses(void) { // make sure SPI lock is not held over a soft reset if (spi_singleton != NULL) { common_hal_busio_spi_unlock(spi_singleton); - } - if (!display_using_spi) { - spi_singleton = NULL; + if (!display_using_spi) { + common_hal_busio_spi_deinit(spi_singleton); + spi_singleton = NULL; + } } #endif #if BOARD_UART -- cgit v1.2.3 From 99a3750a2cfd1da8cdd266ca9a0367f0c3a54092 Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Sat, 24 Oct 2020 17:50:11 +0200 Subject: Fix lost board.SPI and board.I2C after explicitly deiniting them. After calling board.SPI().deinit(), calling board.SPI() again would return the unusable deinited object and there was no way of getting it back into an initialized state until the end of the session. --- shared-bindings/board/__init__.c | 10 ++++++++-- shared-module/board/__init__.c | 10 ++++------ 2 files changed, 12 insertions(+), 8 deletions(-) (limited to 'shared-module') diff --git a/shared-bindings/board/__init__.c b/shared-bindings/board/__init__.c index 6837fc41c..717698408 100644 --- a/shared-bindings/board/__init__.c +++ b/shared-bindings/board/__init__.c @@ -28,6 +28,12 @@ #include "py/runtime.h" #include "shared-bindings/board/__init__.h" +#if BOARD_I2C +#include "shared-bindings/busio/I2C.h" +#endif +#if BOARD_SPI +#include "shared-bindings/busio/SPI.h" +#endif //| """Board specific pin names //| @@ -45,7 +51,7 @@ #if BOARD_I2C mp_obj_t board_i2c(void) { mp_obj_t singleton = common_hal_board_get_i2c(); - if (singleton != NULL) { + if (singleton != NULL && !common_hal_busio_i2c_deinited(singleton)) { return singleton; } assert_pin_free(DEFAULT_I2C_BUS_SDA); @@ -69,7 +75,7 @@ MP_DEFINE_CONST_FUN_OBJ_0(board_i2c_obj, board_i2c); #if BOARD_SPI mp_obj_t board_spi(void) { mp_obj_t singleton = common_hal_board_get_spi(); - if (singleton != NULL) { + if (singleton != NULL && !common_hal_busio_spi_deinited(singleton)) { return singleton; } assert_pin_free(DEFAULT_SPI_BUS_SCK); diff --git a/shared-module/board/__init__.c b/shared-module/board/__init__.c index 967b430d2..2f1c34e56 100644 --- a/shared-module/board/__init__.c +++ b/shared-module/board/__init__.c @@ -55,9 +55,8 @@ mp_obj_t common_hal_board_get_i2c(void) { } mp_obj_t common_hal_board_create_i2c(void) { - if (i2c_singleton != NULL) { - return i2c_singleton; - } + // All callers have either already verified this or come so early that it can't be otherwise. + assert(i2c_singleton == NULL || common_hal_busio_i2c_deinited(i2c_singleton)); busio_i2c_obj_t *self = &i2c_obj; self->base.type = &busio_i2c_type; @@ -79,9 +78,8 @@ mp_obj_t common_hal_board_get_spi(void) { } mp_obj_t common_hal_board_create_spi(void) { - if (spi_singleton != NULL) { - return spi_singleton; - } + // All callers have either already verified this or come so early that it can't be otherwise. + assert(spi_singleton == NULL || common_hal_busio_spi_deinited(spi_singleton)); busio_spi_obj_t *self = &spi_obj; self->base.type = &busio_spi_type; -- cgit v1.2.3 From 0d1649880f1c23ec6c9c0a4834f3712a8495f9d8 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Mon, 26 Oct 2020 16:58:00 -0700 Subject: Add grayscale EInk support --- shared-bindings/displayio/EPaperDisplay.c | 35 +++++++++++++++++++++++++------ shared-bindings/displayio/EPaperDisplay.h | 2 +- shared-module/displayio/ColorConverter.c | 9 ++++---- shared-module/displayio/EPaperDisplay.c | 15 ++++++++----- shared-module/displayio/EPaperDisplay.h | 1 + shared-module/displayio/Palette.h | 1 + shared-module/displayio/display_core.c | 1 + 7 files changed, 48 insertions(+), 16 deletions(-) (limited to 'shared-module') diff --git a/shared-bindings/displayio/EPaperDisplay.c b/shared-bindings/displayio/EPaperDisplay.c index 8be4ee4c4..e0326d9c8 100644 --- a/shared-bindings/displayio/EPaperDisplay.c +++ b/shared-bindings/displayio/EPaperDisplay.c @@ -49,7 +49,19 @@ //| Most people should not use this class directly. Use a specific display driver instead that will //| contain the startup and shutdown sequences at minimum.""" //| -//| def __init__(self, display_bus: _DisplayBus, start_sequence: ReadableBuffer, stop_sequence: ReadableBuffer, *, width: int, height: int, ram_width: int, ram_height: int, colstart: int = 0, rowstart: int = 0, rotation: int = 0, set_column_window_command: Optional[int] = None, set_row_window_command: Optional[int] = None, single_byte_bounds: bool = False, write_black_ram_command: int, black_bits_inverted: bool = False, write_color_ram_command: Optional[int] = None, color_bits_inverted: bool = False, highlight_color: int = 0x000000, refresh_display_command: int, refresh_time: float = 40, busy_pin: Optional[microcontroller.Pin] = None, busy_state: bool = True, seconds_per_frame: float = 180, always_toggle_chip_select: bool = False) -> None: +//| def __init__(self, display_bus: _DisplayBus, +//| start_sequence: ReadableBuffer, stop_sequence: ReadableBuffer, *, +//| width: int, height: int, ram_width: int, ram_height: int, +//| colstart: int = 0, rowstart: int = 0, rotation: int = 0, +//| set_column_window_command: Optional[int] = None, +//| set_row_window_command: Optional[int] = None, single_byte_bounds: bool = False, +//| write_black_ram_command: int, black_bits_inverted: bool = False, +//| write_color_ram_command: Optional[int] = None, +//| color_bits_inverted: bool = False, highlight_color: int = 0x000000, +//| refresh_display_command: int, refresh_time: float = 40, +//| busy_pin: Optional[microcontroller.Pin] = None, busy_state: bool = True, +//| seconds_per_frame: float = 180, always_toggle_chip_select: bool = False, +//| grayscale: bool = False) -> None: //| """Create a EPaperDisplay object on the given display bus (`displayio.FourWire` or `displayio.ParallelBus`). //| //| The ``start_sequence`` and ``stop_sequence`` are bitpacked to minimize the ram impact. Every @@ -84,11 +96,18 @@ //| :param microcontroller.Pin busy_pin: Pin used to signify the display is busy //| :param bool busy_state: State of the busy pin when the display is busy //| :param float seconds_per_frame: Minimum number of seconds between screen refreshes -//| :param bool always_toggle_chip_select: When True, chip select is toggled every byte""" +//| :param bool always_toggle_chip_select: When True, chip select is toggled every byte +//| :param bool grayscale: When true, the color ram is the low bit of 2-bit grayscale""" //| ... //| STATIC mp_obj_t displayio_epaperdisplay_make_new(const mp_obj_type_t *type, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { - enum { ARG_display_bus, ARG_start_sequence, ARG_stop_sequence, ARG_width, ARG_height, ARG_ram_width, ARG_ram_height, ARG_colstart, ARG_rowstart, ARG_rotation, ARG_set_column_window_command, ARG_set_row_window_command, ARG_set_current_column_command, ARG_set_current_row_command, ARG_write_black_ram_command, ARG_black_bits_inverted, ARG_write_color_ram_command, ARG_color_bits_inverted, ARG_highlight_color, ARG_refresh_display_command, ARG_refresh_time, ARG_busy_pin, ARG_busy_state, ARG_seconds_per_frame, ARG_always_toggle_chip_select }; + enum { ARG_display_bus, ARG_start_sequence, ARG_stop_sequence, ARG_width, ARG_height, + ARG_ram_width, ARG_ram_height, ARG_colstart, ARG_rowstart, ARG_rotation, + ARG_set_column_window_command, ARG_set_row_window_command, ARG_set_current_column_command, + ARG_set_current_row_command, ARG_write_black_ram_command, ARG_black_bits_inverted, + ARG_write_color_ram_command, ARG_color_bits_inverted, ARG_highlight_color, + ARG_refresh_display_command, ARG_refresh_time, ARG_busy_pin, ARG_busy_state, + ARG_seconds_per_frame, ARG_always_toggle_chip_select, ARG_grayscale }; static const mp_arg_t allowed_args[] = { { MP_QSTR_display_bus, MP_ARG_REQUIRED | MP_ARG_OBJ }, { MP_QSTR_start_sequence, MP_ARG_REQUIRED | MP_ARG_OBJ }, @@ -115,6 +134,7 @@ STATIC mp_obj_t displayio_epaperdisplay_make_new(const mp_obj_type_t *type, size { MP_QSTR_busy_state, MP_ARG_BOOL | MP_ARG_KW_ONLY, {.u_bool = true} }, { MP_QSTR_seconds_per_frame, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = MP_OBJ_NEW_SMALL_INT(180)} }, { MP_QSTR_always_toggle_chip_select, MP_ARG_BOOL | MP_ARG_KW_ONLY, {.u_bool = false} }, + { MP_QSTR_grayscale, MP_ARG_BOOL | MP_ARG_KW_ONLY, {.u_bool = false} }, }; mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); @@ -151,11 +171,14 @@ STATIC mp_obj_t displayio_epaperdisplay_make_new(const mp_obj_type_t *type, size self, display_bus, start_bufinfo.buf, start_bufinfo.len, stop_bufinfo.buf, stop_bufinfo.len, - args[ARG_width].u_int, args[ARG_height].u_int, args[ARG_ram_width].u_int, args[ARG_ram_height].u_int, args[ARG_colstart].u_int, args[ARG_rowstart].u_int, rotation, + args[ARG_width].u_int, args[ARG_height].u_int, args[ARG_ram_width].u_int, args[ARG_ram_height].u_int, + args[ARG_colstart].u_int, args[ARG_rowstart].u_int, rotation, args[ARG_set_column_window_command].u_int, args[ARG_set_row_window_command].u_int, args[ARG_set_current_column_command].u_int, args[ARG_set_current_row_command].u_int, - args[ARG_write_black_ram_command].u_int, args[ARG_black_bits_inverted].u_bool, write_color_ram_command, args[ARG_color_bits_inverted].u_bool, highlight_color, args[ARG_refresh_display_command].u_int, refresh_time, - busy_pin, args[ARG_busy_state].u_bool, seconds_per_frame, args[ARG_always_toggle_chip_select].u_bool + args[ARG_write_black_ram_command].u_int, args[ARG_black_bits_inverted].u_bool, write_color_ram_command, + args[ARG_color_bits_inverted].u_bool, highlight_color, args[ARG_refresh_display_command].u_int, refresh_time, + busy_pin, args[ARG_busy_state].u_bool, seconds_per_frame, + args[ARG_always_toggle_chip_select].u_bool, args[ARG_grayscale].u_bool ); return self; diff --git a/shared-bindings/displayio/EPaperDisplay.h b/shared-bindings/displayio/EPaperDisplay.h index e4b81c883..352de899a 100644 --- a/shared-bindings/displayio/EPaperDisplay.h +++ b/shared-bindings/displayio/EPaperDisplay.h @@ -44,7 +44,7 @@ void common_hal_displayio_epaperdisplay_construct(displayio_epaperdisplay_obj_t* uint16_t set_column_window_command, uint16_t set_row_window_command, uint16_t set_current_column_command, uint16_t set_current_row_command, uint16_t write_black_ram_command, bool black_bits_inverted, uint16_t write_color_ram_command, bool color_bits_inverted, uint32_t highlight_color, uint16_t refresh_display_command, mp_float_t refresh_time, - const mcu_pin_obj_t* busy_pin, bool busy_state, mp_float_t seconds_per_frame, bool always_toggle_chip_select); + const mcu_pin_obj_t* busy_pin, bool busy_state, mp_float_t seconds_per_frame, bool always_toggle_chip_select, bool grayscale); bool common_hal_displayio_epaperdisplay_refresh(displayio_epaperdisplay_obj_t* self); diff --git a/shared-module/displayio/ColorConverter.c b/shared-module/displayio/ColorConverter.c index dc64da03d..03ec99ceb 100644 --- a/shared-module/displayio/ColorConverter.c +++ b/shared-module/displayio/ColorConverter.c @@ -165,9 +165,9 @@ void displayio_colorconverter_convert(displayio_colorconverter_t *self, const _d g8 = MIN(255,g8 + (randg&0x03)); } else { int bitmask = 0xFF >> colorspace->depth; - b8 = MIN(255,b8 + (randb&bitmask)); - r8 = MIN(255,r8 + (randr&bitmask)); - g8 = MIN(255,g8 + (randg&bitmask)); + b8 = MIN(255,b8 + (randb & bitmask)); + r8 = MIN(255,r8 + (randr & bitmask)); + g8 = MIN(255,g8 + (randg & bitmask)); } pixel = r8 << 16 | g8 << 8 | b8; } @@ -196,7 +196,8 @@ void displayio_colorconverter_convert(displayio_colorconverter_t *self, const _d return; } else if (colorspace->grayscale && colorspace->depth <= 8) { uint8_t luma = displayio_colorconverter_compute_luma(pixel); - output_color->pixel = luma >> (8 - colorspace->depth); + size_t bitmask = (1 << colorspace->depth) - 1; + output_color->pixel = (luma >> colorspace->grayscale_bit) & bitmask; output_color->opaque = true; return; } diff --git a/shared-module/displayio/EPaperDisplay.c b/shared-module/displayio/EPaperDisplay.c index 46c7ea82e..514b99a13 100644 --- a/shared-module/displayio/EPaperDisplay.c +++ b/shared-module/displayio/EPaperDisplay.c @@ -49,7 +49,7 @@ void common_hal_displayio_epaperdisplay_construct(displayio_epaperdisplay_obj_t* uint16_t set_column_window_command, uint16_t set_row_window_command, uint16_t set_current_column_command, uint16_t set_current_row_command, uint16_t write_black_ram_command, bool black_bits_inverted, uint16_t write_color_ram_command, bool color_bits_inverted, uint32_t highlight_color, uint16_t refresh_display_command, mp_float_t refresh_time, - const mcu_pin_obj_t* busy_pin, bool busy_state, mp_float_t seconds_per_frame, bool chip_select) { + const mcu_pin_obj_t* busy_pin, bool busy_state, mp_float_t seconds_per_frame, bool chip_select, bool grayscale) { if (highlight_color != 0x000000) { self->core.colorspace.tricolor = true; self->core.colorspace.tricolor_hue = displayio_colorconverter_compute_hue(highlight_color); @@ -72,6 +72,7 @@ void common_hal_displayio_epaperdisplay_construct(displayio_epaperdisplay_obj_t* self->refreshing = false; self->milliseconds_per_frame = seconds_per_frame * 1000; self->chip_select = chip_select ? CHIP_SELECT_TOGGLE_EVERY_BYTE : CHIP_SELECT_UNTOUCHED; + self->grayscale = grayscale; self->start_sequence = start_sequence; self->start_sequence_len = start_sequence_len; @@ -230,17 +231,16 @@ bool displayio_epaperdisplay_refresh_area(displayio_epaperdisplay_obj_t* self, c uint32_t mask[mask_length]; uint8_t passes = 1; - if (self->core.colorspace.tricolor) { + if (self->core.colorspace.tricolor || self->grayscale) { passes = 2; } for (uint8_t pass = 0; pass < passes; pass++) { uint16_t remaining_rows = displayio_area_height(&clipped); - // added false parameter at end for SH1107_addressing quirk if (self->set_row_window_command != NO_COMMAND) { displayio_display_core_set_region_to_update(&self->core, self->set_column_window_command, self->set_row_window_command, self->set_current_column_command, self->set_current_row_command, - false, self->chip_select, &clipped, false); + false, self->chip_select, &clipped, false /* SH1107_addressing */); } uint8_t write_command = self->write_black_ram_command; @@ -270,8 +270,13 @@ bool displayio_epaperdisplay_refresh_area(displayio_epaperdisplay_obj_t* self, c memset(buffer, 0, buffer_size * sizeof(buffer[0])); self->core.colorspace.grayscale = true; + self->core.colorspace.grayscale_bit = 7; if (pass == 1) { - self->core.colorspace.grayscale = false; + if (self->grayscale) { // 4-color grayscale + self->core.colorspace.grayscale_bit = 6; + } else { // Tri-color + self->core.colorspace.grayscale = false; + } } displayio_display_core_fill_area(&self->core, &subrectangle, mask, buffer); diff --git a/shared-module/displayio/EPaperDisplay.h b/shared-module/displayio/EPaperDisplay.h index 3b9f6e368..4103fe5fc 100644 --- a/shared-module/displayio/EPaperDisplay.h +++ b/shared-module/displayio/EPaperDisplay.h @@ -55,6 +55,7 @@ typedef struct { bool black_bits_inverted; bool color_bits_inverted; bool refreshing; + bool grayscale; display_chip_select_behavior_t chip_select; } displayio_epaperdisplay_obj_t; diff --git a/shared-module/displayio/Palette.h b/shared-module/displayio/Palette.h index da72f250f..993912cc5 100644 --- a/shared-module/displayio/Palette.h +++ b/shared-module/displayio/Palette.h @@ -37,6 +37,7 @@ typedef struct { uint8_t bytes_per_cell; uint8_t tricolor_hue; uint8_t tricolor_luma; + uint8_t grayscale_bit; // The lowest grayscale bit. Normally 8 - depth. bool grayscale; bool tricolor; bool pixels_in_byte_share_row; diff --git a/shared-module/displayio/display_core.c b/shared-module/displayio/display_core.c index 411f9f373..57d33b565 100644 --- a/shared-module/displayio/display_core.c +++ b/shared-module/displayio/display_core.c @@ -48,6 +48,7 @@ void displayio_display_core_construct(displayio_display_core_t* self, uint16_t color_depth, bool grayscale, bool pixels_in_byte_share_row, uint8_t bytes_per_cell, bool reverse_pixels_in_byte, bool reverse_bytes_in_word) { self->colorspace.depth = color_depth; self->colorspace.grayscale = grayscale; + self->colorspace.grayscale_bit = 8 - color_depth; self->colorspace.pixels_in_byte_share_row = pixels_in_byte_share_row; self->colorspace.bytes_per_cell = bytes_per_cell; self->colorspace.reverse_pixels_in_byte = reverse_pixels_in_byte; -- cgit v1.2.3