From 2cc6514583be7137a58005418740eb58a30a0389 Mon Sep 17 00:00:00 2001 From: Phil Howard Date: Thu, 26 Aug 2021 16:35:05 +0100 Subject: [PATCH 1/2] Slow set/clr_bit writes a little We chased a bug with handling/clearing interrupts on Encoder into the depths of madness, finding that a Debug build would magically fix the bug. Turns out it was probably just us being a little aggressive with the poor little MS51-based Encoder driver. * Fix delays to be more delayey. * Replace big 'ol loop and boolean with straight up checks and an early exit- the bit-addressed regs are never going to change --- drivers/ioexpander/ioexpander.cpp | 46 ++++++++++++------------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/drivers/ioexpander/ioexpander.cpp b/drivers/ioexpander/ioexpander.cpp index dec135a5..7a7450c3 100644 --- a/drivers/ioexpander/ioexpander.cpp +++ b/drivers/ioexpander/ioexpander.cpp @@ -756,27 +756,19 @@ namespace pimoroni { } void IOExpander::set_bits(uint8_t reg, uint8_t bits) { - // Set the specified bits (using a mask) in a register. - - // Deal with special case registers first - bool reg_in_bit_addressed_regs = false; - for(uint8_t i = 0; i < NUM_BIT_ADDRESSED_REGISTERS; i++) { - if(BIT_ADDRESSED_REGS[i] == reg) { + if(reg == reg::P0 || reg == reg::P1 || reg == reg::P2 || reg == reg::P3) { for(uint8_t bit = 0; bit < 8; bit++) { - if(bits & (1 << bit)) + if(bits & (1 << bit)) { i2c->reg_write_uint8(address, reg, 0b1000 | (bit & 0b111)); + sleep_us(50); + } } - reg_in_bit_addressed_regs = true; - break; - } + return; } - // Now deal with any other registers - if(!reg_in_bit_addressed_regs) { - uint8_t value = i2c->reg_read_uint8(address, reg); - sleep_us(10); - i2c->reg_write_uint8(address, reg, value | bits); - } + uint8_t value = i2c->reg_read_uint8(address, reg); + sleep_us(50); + i2c->reg_write_uint8(address, reg, value | bits); } void IOExpander::set_bit(uint8_t reg, uint8_t bit) { @@ -785,24 +777,20 @@ namespace pimoroni { } void IOExpander::clr_bits(uint8_t reg, uint8_t bits) { - bool reg_in_bit_addressed_regs = false; - for(uint8_t i = 0; i < NUM_BIT_ADDRESSED_REGISTERS; i++) { - if(BIT_ADDRESSED_REGS[i] == reg) { - for(uint8_t bit = 0; bit < 8; bit++) { - if(bits & (1 << bit)) - i2c->reg_write_uint8(address, reg, 0b0000 | (bit & 0b111)); + if(reg == reg::P0 || reg == reg::P1 || reg == reg::P2 || reg == reg::P3) { + for(uint8_t bit = 0; bit < 8; bit++) { + if(bits & (1 << bit)) { + i2c->reg_write_uint8(address, reg, 0b0000 | (bit & 0b111)); + sleep_us(50); } - reg_in_bit_addressed_regs = true; - break; } + return; } // Now deal with any other registers - if(!reg_in_bit_addressed_regs) { - uint8_t value = i2c->reg_read_uint8(address, reg); - sleep_us(10); - i2c->reg_write_uint8(address, reg, value & ~bits); - } + uint8_t value = i2c->reg_read_uint8(address, reg); + sleep_us(50); + i2c->reg_write_uint8(address, reg, value & ~bits); } void IOExpander::clr_bit(uint8_t reg, uint8_t bit) { From b5c7add90ddde58a582c19b91061920cb721c2e8 Mon Sep 17 00:00:00 2001 From: Phil Howard Date: Thu, 26 Aug 2021 18:14:39 +0100 Subject: [PATCH 2/2] APA102: Extra clocks to flush pixels This covers an edge case where pixels are updated intermittently - such as the once that happens when clearing before a Python soft reset. Under normal circumstances users should `start` the LED strip and allow it to continuously update. --- drivers/plasma/apa102.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/plasma/apa102.cpp b/drivers/plasma/apa102.cpp index 5ba6d4ef..9b4d2713 100644 --- a/drivers/plasma/apa102.cpp +++ b/drivers/plasma/apa102.cpp @@ -51,6 +51,16 @@ void APA102::update(bool blocking) { dma_channel_set_read_addr(dma_channel, buffer, true); if (!blocking) return; while(dma_channel_is_busy(dma_channel)) {}; // Block waiting for DMA finish + // This is necessary to prevent a single LED remaining lit when clearing and updating. + // This code will only run in *blocking* mode since it's assumed non-blocking will be continuously updating anyway. + // Yes this will slow down LED updates... don't use blocking mode unless you're clearing LEDs before shutdown, + // or you *really* want to avoid actively driving your APA102s for some reason. + for(auto x = 0u; x < (num_leds / 16) + 1; x++) { + pio->txf[sm] = 0x00000000; + // Some delay is needed, since the PIO is async + // and this could be happening during a destructor/MicroPython soft reset + sleep_ms(1); // Chosen by fair dice roll + } } bool APA102::start(uint fps) {