From 73f93b044bd3b38eef2f1c7d107ca54948d1f564 Mon Sep 17 00:00:00 2001 From: beserge Date: Fri, 8 Oct 2021 12:56:19 -0400 Subject: [PATCH 1/7] Fix SetLedRaw overflow bug Off value = rawbrightness + on. On value = startcycle = ledidx << 2. With lots of drivers ledidx can get quite high So eventually off = rawbrightness + on > 4095 and the led gets dimmer. Fixed this by just changing the fullbrightness if to include the on value. --- src/dev/leddriver.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dev/leddriver.h b/src/dev/leddriver.h index 7ca41fa5f..1bbeae556 100644 --- a/src/dev/leddriver.h +++ b/src/dev/leddriver.h @@ -127,7 +127,7 @@ class LedDriverPca9685 const auto on = draw_buffer_[d].leds[ch].on & (0x0FFF); draw_buffer_[d].leds[ch].off = (on + rawBrightness) & (0x0FFF); // full on condition - if(rawBrightness >= 0x0FFF) + if(rawBrightness + on >= 0x0FFF) draw_buffer_[d].leds[ch].on = 0x1000 | on; // set "full on" bit else draw_buffer_[d].leds[ch].on = on; // clear "full on" bit From 20fb63eb52078d6a50bec37c84fad1d048e49752 Mon Sep 17 00:00:00 2001 From: beserge Date: Tue, 12 Oct 2021 15:27:44 -0400 Subject: [PATCH 2/7] Only send buffer if an led is updated --- src/dev/leddriver.h | 51 +++++++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/src/dev/leddriver.h b/src/dev/leddriver.h index 1bbeae556..f8006e76a 100644 --- a/src/dev/leddriver.h +++ b/src/dev/leddriver.h @@ -44,6 +44,8 @@ class LedDriverPca9685 } leds[16]; /** full size in bytes */ static constexpr uint16_t size = 16 * 4 + 1; + /** Whether any led in this buff has an unwritten update */ + bool updated; }; /** Buffer type for the entire DMA buffer. */ using DmaBuffer = PCA9685TransmitBuffer[numDrivers]; @@ -125,12 +127,21 @@ class LedDriverPca9685 const auto ch = GetDriverChannelForLed(ledIndex); // mask away the "full on" bit const auto on = draw_buffer_[d].leds[ch].on & (0x0FFF); - draw_buffer_[d].leds[ch].off = (on + rawBrightness) & (0x0FFF); + const auto off = (on + rawBrightness) & (0x0FFF); + + draw_buffer_[d].updated |= off != draw_buffer_[d].leds[ch].off; + + draw_buffer_[d].leds[ch].off = off; + // full on condition - if(rawBrightness + on >= 0x0FFF) + if(rawBrightness >= 0x0FFF){ + draw_buffer_[d].updated |= !(draw_buffer_[d].leds[ch].on & 0x1000); // updating on? draw_buffer_[d].leds[ch].on = 0x1000 | on; // set "full on" bit - else + } + else{ + draw_buffer_[d].updated |= draw_buffer_[d].leds[ch].on & 0x1000; // updating on? draw_buffer_[d].leds[ch].on = on; // clear "full on" bit + } } /** Swaps the current draw buffer and the current transmit buffer and @@ -173,16 +184,27 @@ class LedDriverPca9685 const auto d = current_driver_idx_; const uint8_t address = PCA9685_I2C_BASE_ADDRESS | addresses_[d]; - const auto status = i2c_.TransmitDma(address, - (uint8_t*)&transmit_buffer_[d], - PCA9685TransmitBuffer::size, - &TxCpltCallback, - this); - if(status != I2CHandle::Result::OK) - { - // TODO: fix this :-) - // Reinit I2C (probably a flag to kill, but hey this works fairly well for now.) - i2c_.Init(i2c_.GetConfig()); + + if(transmit_buffer_[d].updated){ + + transmit_buffer_[d].updated = false; // reset the flag + + // send the bufer + const auto status = i2c_.TransmitDma(address, + (uint8_t*)&transmit_buffer_[d], + PCA9685TransmitBuffer::size, + &TxCpltCallback, + this); + + if(status != I2CHandle::Result::OK) + { + // TODO: fix this :-) + // Reinit I2C (probably a flag to kill, but hey this works fairly well for now.) + i2c_.Init(i2c_.GetConfig()); + } + } + else{ + ContinueTransmission(); } } uint16_t GetStartCycleForLed(int ledIndex) const @@ -207,9 +229,12 @@ class LedDriverPca9685 draw_buffer_[d].registerAddr = PCA9685_LED0; draw_buffer_[d].leds[ch].on = startCycle; draw_buffer_[d].leds[ch].off = startCycle; + draw_buffer_[d].updated = false; transmit_buffer_[d].registerAddr = PCA9685_LED0; transmit_buffer_[d].leds[ch].on = startCycle; transmit_buffer_[d].leds[ch].off = startCycle; + transmit_buffer_[d].leds[ch].off = startCycle; + transmit_buffer_[d].updated = false; } } From 372865c10985b350eb6ee64670fb04e3f60b4348 Mon Sep 17 00:00:00 2001 From: beserge Date: Tue, 12 Oct 2021 15:28:10 -0400 Subject: [PATCH 3/7] clang-format --- src/dev/leddriver.h | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/dev/leddriver.h b/src/dev/leddriver.h index f8006e76a..90c09066a 100644 --- a/src/dev/leddriver.h +++ b/src/dev/leddriver.h @@ -126,21 +126,25 @@ class LedDriverPca9685 const auto d = GetDriverForLed(ledIndex); const auto ch = GetDriverChannelForLed(ledIndex); // mask away the "full on" bit - const auto on = draw_buffer_[d].leds[ch].on & (0x0FFF); - const auto off = (on + rawBrightness) & (0x0FFF); + const auto on = draw_buffer_[d].leds[ch].on & (0x0FFF); + const auto off = (on + rawBrightness) & (0x0FFF); draw_buffer_[d].updated |= off != draw_buffer_[d].leds[ch].off; draw_buffer_[d].leds[ch].off = off; // full on condition - if(rawBrightness >= 0x0FFF){ - draw_buffer_[d].updated |= !(draw_buffer_[d].leds[ch].on & 0x1000); // updating on? - draw_buffer_[d].leds[ch].on = 0x1000 | on; // set "full on" bit + if(rawBrightness >= 0x0FFF) + { + draw_buffer_[d].updated + |= !(draw_buffer_[d].leds[ch].on & 0x1000); // updating on? + draw_buffer_[d].leds[ch].on = 0x1000 | on; // set "full on" bit } - else{ - draw_buffer_[d].updated |= draw_buffer_[d].leds[ch].on & 0x1000; // updating on? - draw_buffer_[d].leds[ch].on = on; // clear "full on" bit + else + { + draw_buffer_[d].updated + |= draw_buffer_[d].leds[ch].on & 0x1000; // updating on? + draw_buffer_[d].leds[ch].on = on; // clear "full on" bit } } @@ -184,17 +188,17 @@ class LedDriverPca9685 const auto d = current_driver_idx_; const uint8_t address = PCA9685_I2C_BASE_ADDRESS | addresses_[d]; - - if(transmit_buffer_[d].updated){ + if(transmit_buffer_[d].updated) + { transmit_buffer_[d].updated = false; // reset the flag - // send the bufer - const auto status = i2c_.TransmitDma(address, - (uint8_t*)&transmit_buffer_[d], - PCA9685TransmitBuffer::size, - &TxCpltCallback, - this); + // send the bufer + const auto status = i2c_.TransmitDma(address, + (uint8_t*)&transmit_buffer_[d], + PCA9685TransmitBuffer::size, + &TxCpltCallback, + this); if(status != I2CHandle::Result::OK) { @@ -203,7 +207,8 @@ class LedDriverPca9685 i2c_.Init(i2c_.GetConfig()); } } - else{ + else + { ContinueTransmission(); } } @@ -229,12 +234,12 @@ class LedDriverPca9685 draw_buffer_[d].registerAddr = PCA9685_LED0; draw_buffer_[d].leds[ch].on = startCycle; draw_buffer_[d].leds[ch].off = startCycle; - draw_buffer_[d].updated = false; + draw_buffer_[d].updated = false; transmit_buffer_[d].registerAddr = PCA9685_LED0; transmit_buffer_[d].leds[ch].on = startCycle; transmit_buffer_[d].leds[ch].off = startCycle; transmit_buffer_[d].leds[ch].off = startCycle; - transmit_buffer_[d].updated = false; + transmit_buffer_[d].updated = false; } } From 3527cdfb52942326114f446b9f2f80e2a2859766 Mon Sep 17 00:00:00 2001 From: beserge Date: Thu, 4 Nov 2021 12:25:16 -0400 Subject: [PATCH 4/7] First pass at direct addressing buffers The fifth transfer fills up the queue and we get stuck on line 147 of i2c.cpp --- src/dev/leddriver.h | 60 ++++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/src/dev/leddriver.h b/src/dev/leddriver.h index 90c09066a..53b4bb5d9 100644 --- a/src/dev/leddriver.h +++ b/src/dev/leddriver.h @@ -45,7 +45,7 @@ class LedDriverPca9685 /** full size in bytes */ static constexpr uint16_t size = 16 * 4 + 1; /** Whether any led in this buff has an unwritten update */ - bool updated; + bool updated[16]; }; /** Buffer type for the entire DMA buffer. */ using DmaBuffer = PCA9685TransmitBuffer[numDrivers]; @@ -129,20 +129,20 @@ class LedDriverPca9685 const auto on = draw_buffer_[d].leds[ch].on & (0x0FFF); const auto off = (on + rawBrightness) & (0x0FFF); - draw_buffer_[d].updated |= off != draw_buffer_[d].leds[ch].off; + draw_buffer_[d].updated[ch] |= off != draw_buffer_[d].leds[ch].off; draw_buffer_[d].leds[ch].off = off; // full on condition if(rawBrightness >= 0x0FFF) { - draw_buffer_[d].updated + draw_buffer_[d].updated[ch] |= !(draw_buffer_[d].leds[ch].on & 0x1000); // updating on? draw_buffer_[d].leds[ch].on = 0x1000 | on; // set "full on" bit } else { - draw_buffer_[d].updated + draw_buffer_[d].updated[ch] |= draw_buffer_[d].leds[ch].on & 0x1000; // updating on? draw_buffer_[d].leds[ch].on = on; // clear "full on" bit } @@ -165,10 +165,14 @@ class LedDriverPca9685 // to keep the led settings (if required) if(persistentBufferContents) { - for(int d = 0; d < numDrivers; d++) - for(int ch = 0; ch < 16; ch++) + for(int d = 0; d < numDrivers; d++){ + for(int ch = 0; ch < 16; ch++){ draw_buffer_[d].leds[ch].off = transmit_buffer_[d].leds[ch].off; + draw_buffer_[d].updated[ch] + = transmit_buffer_[d].updated[ch]; + } + } } // start transmission @@ -189,27 +193,32 @@ class LedDriverPca9685 const auto d = current_driver_idx_; const uint8_t address = PCA9685_I2C_BASE_ADDRESS | addresses_[d]; - if(transmit_buffer_[d].updated) - { - transmit_buffer_[d].updated = false; // reset the flag + for(int i = 0; i < 16; i++){ + uint8_t ch = GetDriverChannelForLed(i); + + if(transmit_buffer_[d].updated[ch]){ + uint8_t regAddr = transmit_buffer_[d].registerAddr; + uint8_t onLow = transmit_buffer_[d].leds[ch].on; + uint8_t onHigh = (transmit_buffer_[d].leds[ch].on >> 8); + uint8_t offLow = transmit_buffer_[d].leds[ch].off; + uint8_t offHigh = (transmit_buffer_[d].leds[ch].off >> 8); + + uint8_t data[5] = {regAddr, onLow, onHigh, offLow, offHigh}; - // send the bufer const auto status = i2c_.TransmitDma(address, - (uint8_t*)&transmit_buffer_[d], - PCA9685TransmitBuffer::size, + data, + 5, &TxCpltCallback, this); - - if(status != I2CHandle::Result::OK) - { - // TODO: fix this :-) - // Reinit I2C (probably a flag to kill, but hey this works fairly well for now.) - i2c_.Init(i2c_.GetConfig()); + if(status != I2CHandle::Result::OK) + { + // TODO: fix this :-) + // Reinit I2C (probably a flag to kill, but hey this works fairly well for now.) + i2c_.Init(i2c_.GetConfig()); + } } - } - else - { - ContinueTransmission(); + + transmit_buffer_[d].updated[i] = false; // reset the flag } } uint16_t GetStartCycleForLed(int ledIndex) const @@ -234,12 +243,12 @@ class LedDriverPca9685 draw_buffer_[d].registerAddr = PCA9685_LED0; draw_buffer_[d].leds[ch].on = startCycle; draw_buffer_[d].leds[ch].off = startCycle; - draw_buffer_[d].updated = false; + draw_buffer_[d].updated[ch] = false; transmit_buffer_[d].registerAddr = PCA9685_LED0; transmit_buffer_[d].leds[ch].on = startCycle; transmit_buffer_[d].leds[ch].off = startCycle; transmit_buffer_[d].leds[ch].off = startCycle; - transmit_buffer_[d].updated = false; + transmit_buffer_[d].updated[ch] = false; } } @@ -270,7 +279,8 @@ class LedDriverPca9685 System::Delay(20); buffer[0] = PCA9685_MODE1; // auto increment on - buffer[1] = 0b00100000; + // buffer[1] = 0b00100000; + buffer[1] = 0b00000000; i2c_.TransmitBlocking(address, buffer, 2, 1); System::Delay(20); buffer[0] = PCA9685_MODE2; From ca8a30a8cf14f4672fe5972cf1703a8249b69cf3 Mon Sep 17 00:00:00 2001 From: beserge Date: Thu, 4 Nov 2021 12:26:57 -0400 Subject: [PATCH 5/7] Formatting --- src/dev/leddriver.h | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/dev/leddriver.h b/src/dev/leddriver.h index 53b4bb5d9..0abffd7be 100644 --- a/src/dev/leddriver.h +++ b/src/dev/leddriver.h @@ -165,8 +165,10 @@ class LedDriverPca9685 // to keep the led settings (if required) if(persistentBufferContents) { - for(int d = 0; d < numDrivers; d++){ - for(int ch = 0; ch < 16; ch++){ + for(int d = 0; d < numDrivers; d++) + { + for(int ch = 0; ch < 16; ch++) + { draw_buffer_[d].leds[ch].off = transmit_buffer_[d].leds[ch].off; draw_buffer_[d].updated[ch] @@ -193,23 +195,22 @@ class LedDriverPca9685 const auto d = current_driver_idx_; const uint8_t address = PCA9685_I2C_BASE_ADDRESS | addresses_[d]; - for(int i = 0; i < 16; i++){ + for(int i = 0; i < 16; i++) + { uint8_t ch = GetDriverChannelForLed(i); - if(transmit_buffer_[d].updated[ch]){ - uint8_t regAddr = transmit_buffer_[d].registerAddr; - uint8_t onLow = transmit_buffer_[d].leds[ch].on; - uint8_t onHigh = (transmit_buffer_[d].leds[ch].on >> 8); - uint8_t offLow = transmit_buffer_[d].leds[ch].off; - uint8_t offHigh = (transmit_buffer_[d].leds[ch].off >> 8); + if(transmit_buffer_[d].updated[ch]) + { + uint8_t regAddr = transmit_buffer_[d].registerAddr; + uint8_t onLow = transmit_buffer_[d].leds[ch].on; + uint8_t onHigh = (transmit_buffer_[d].leds[ch].on >> 8); + uint8_t offLow = transmit_buffer_[d].leds[ch].off; + uint8_t offHigh = (transmit_buffer_[d].leds[ch].off >> 8); - uint8_t data[5] = {regAddr, onLow, onHigh, offLow, offHigh}; + uint8_t data[5] = {regAddr, onLow, onHigh, offLow, offHigh}; - const auto status = i2c_.TransmitDma(address, - data, - 5, - &TxCpltCallback, - this); + const auto status + = i2c_.TransmitDma(address, data, 5, &TxCpltCallback, this); if(status != I2CHandle::Result::OK) { // TODO: fix this :-) @@ -243,12 +244,12 @@ class LedDriverPca9685 draw_buffer_[d].registerAddr = PCA9685_LED0; draw_buffer_[d].leds[ch].on = startCycle; draw_buffer_[d].leds[ch].off = startCycle; - draw_buffer_[d].updated[ch] = false; + draw_buffer_[d].updated[ch] = false; transmit_buffer_[d].registerAddr = PCA9685_LED0; transmit_buffer_[d].leds[ch].on = startCycle; transmit_buffer_[d].leds[ch].off = startCycle; transmit_buffer_[d].leds[ch].off = startCycle; - transmit_buffer_[d].updated[ch] = false; + transmit_buffer_[d].updated[ch] = false; } } From 89acc4016efb4279c73a6da90b0ed0c4492067da Mon Sep 17 00:00:00 2001 From: beserge Date: Mon, 3 Jan 2022 10:51:19 -0500 Subject: [PATCH 6/7] Get transfers working per updated led Still a slight blink to be fixed as we jump over the fullon threshold --- src/dev/leddriver.h | 68 +++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/src/dev/leddriver.h b/src/dev/leddriver.h index 0abffd7be..6138c213f 100644 --- a/src/dev/leddriver.h +++ b/src/dev/leddriver.h @@ -33,17 +33,16 @@ class LedDriverPca9685 /** Buffer Type for a single PCA9685 driver chip. */ struct __attribute__((packed)) PCA9685TransmitBuffer { - /** register address */ - uint8_t registerAddr = PCA9685_LED0; struct __attribute__((packed)) { + /** register address */ + uint8_t regAddr; /** cycle at which to switch on the led */ uint16_t on; /** cycle at which to switch off the led */ uint16_t off; } leds[16]; - /** full size in bytes */ - static constexpr uint16_t size = 16 * 4 + 1; + /** Whether any led in this buff has an unwritten update */ bool updated[16]; }; @@ -163,17 +162,17 @@ class LedDriverPca9685 // copy current transmit buffer contents to the new draw buffer // to keep the led settings (if required) - if(persistentBufferContents) + // and reset led update status + for(int d = 0; d < numDrivers; d++) { - for(int d = 0; d < numDrivers; d++) + for(int ch = 0; ch < 16; ch++) { - for(int ch = 0; ch < 16; ch++) + if(persistentBufferContents) { draw_buffer_[d].leds[ch].off = transmit_buffer_[d].leds[ch].off; - draw_buffer_[d].updated[ch] - = transmit_buffer_[d].updated[ch]; } + draw_buffer_[d].updated[ch] = false; } } @@ -195,22 +194,19 @@ class LedDriverPca9685 const auto d = current_driver_idx_; const uint8_t address = PCA9685_I2C_BASE_ADDRESS | addresses_[d]; - for(int i = 0; i < 16; i++) + for(int i = 0; i < GetNumLeds(); i++) { uint8_t ch = GetDriverChannelForLed(i); if(transmit_buffer_[d].updated[ch]) { - uint8_t regAddr = transmit_buffer_[d].registerAddr; - uint8_t onLow = transmit_buffer_[d].leds[ch].on; - uint8_t onHigh = (transmit_buffer_[d].leds[ch].on >> 8); - uint8_t offLow = transmit_buffer_[d].leds[ch].off; - uint8_t offHigh = (transmit_buffer_[d].leds[ch].off >> 8); - - uint8_t data[5] = {regAddr, onLow, onHigh, offLow, offHigh}; - const auto status - = i2c_.TransmitDma(address, data, 5, &TxCpltCallback, this); + = i2c_.TransmitDma(address, + (uint8_t*)&transmit_buffer_[d].leds[ch], + 5, + NULL, + this); + if(status != I2CHandle::Result::OK) { // TODO: fix this :-) @@ -218,9 +214,9 @@ class LedDriverPca9685 i2c_.Init(i2c_.GetConfig()); } } - - transmit_buffer_[d].updated[i] = false; // reset the flag } + + TxCpltCallback(this, I2CHandle::Result::OK); } uint16_t GetStartCycleForLed(int ledIndex) const { @@ -238,18 +234,18 @@ class LedDriverPca9685 { for(int led = 0; led < GetNumLeds(); led++) { - const auto d = GetDriverForLed(led); - const auto ch = GetDriverChannelForLed(led); - const auto startCycle = GetStartCycleForLed(led); - draw_buffer_[d].registerAddr = PCA9685_LED0; - draw_buffer_[d].leds[ch].on = startCycle; - draw_buffer_[d].leds[ch].off = startCycle; - draw_buffer_[d].updated[ch] = false; - transmit_buffer_[d].registerAddr = PCA9685_LED0; - transmit_buffer_[d].leds[ch].on = startCycle; - transmit_buffer_[d].leds[ch].off = startCycle; - transmit_buffer_[d].leds[ch].off = startCycle; - transmit_buffer_[d].updated[ch] = false; + const auto d = GetDriverForLed(led); + const auto ch = GetDriverChannelForLed(led); + const auto startCycle = GetStartCycleForLed(led); + draw_buffer_[d].leds[ch].regAddr = PCA9685_LED0 + 4 * ch; + draw_buffer_[d].leds[ch].on = startCycle; + draw_buffer_[d].leds[ch].off = startCycle; + draw_buffer_[d].updated[ch] = false; + transmit_buffer_[d].leds[ch].regAddr = PCA9685_LED0 + 4 * ch; + transmit_buffer_[d].leds[ch].on = startCycle; + transmit_buffer_[d].leds[ch].off = startCycle; + transmit_buffer_[d].leds[ch].off = startCycle; + transmit_buffer_[d].updated[ch] = false; } } @@ -280,8 +276,8 @@ class LedDriverPca9685 System::Delay(20); buffer[0] = PCA9685_MODE1; // auto increment on - // buffer[1] = 0b00100000; - buffer[1] = 0b00000000; + buffer[1] = 0b00100000; + // buffer[1] = 0b00000000; i2c_.TransmitBlocking(address, buffer, 2, 1); System::Delay(20); buffer[0] = PCA9685_MODE2; @@ -316,7 +312,7 @@ class LedDriverPca9685 uint8_t addresses_[numDrivers]; dsy_gpio_pin oe_pin_; dsy_gpio oe_pin_gpio_; - // index of the dirver that is currently updated. + // index of the driver that is currently updated. volatile int8_t current_driver_idx_; const uint16_t gamma_table_[256] = { 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, From effaff7d9830bbb406dd988200ce6f38923da564 Mon Sep 17 00:00:00 2001 From: beserge Date: Mon, 3 Jan 2022 11:56:52 -0500 Subject: [PATCH 7/7] Start with update all leds --- src/dev/leddriver.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dev/leddriver.h b/src/dev/leddriver.h index 6138c213f..36d13316e 100644 --- a/src/dev/leddriver.h +++ b/src/dev/leddriver.h @@ -240,12 +240,12 @@ class LedDriverPca9685 draw_buffer_[d].leds[ch].regAddr = PCA9685_LED0 + 4 * ch; draw_buffer_[d].leds[ch].on = startCycle; draw_buffer_[d].leds[ch].off = startCycle; - draw_buffer_[d].updated[ch] = false; + draw_buffer_[d].updated[ch] = true; transmit_buffer_[d].leds[ch].regAddr = PCA9685_LED0 + 4 * ch; transmit_buffer_[d].leds[ch].on = startCycle; transmit_buffer_[d].leds[ch].off = startCycle; transmit_buffer_[d].leds[ch].off = startCycle; - transmit_buffer_[d].updated[ch] = false; + transmit_buffer_[d].updated[ch] = true; } }