From 03e8ecb921a5c5b8debf307019f3b03966f2ecc0 Mon Sep 17 00:00:00 2001 From: lundril Date: Wed, 1 Jan 2025 16:23:13 +0100 Subject: [PATCH] LED: Fix artefacts. (#61) * LED: Fix artefacts. Basic idea to change the state of the Pins: 1) Tri-State all (will TURN OFF all LEDs) 2) Define correct High/Low value for Pins 3) Drive pins which need driving (will TURN ON correct LEDs) * leddrv: Different implementation. Fix artefacts when using high brightness. Highter drive strength if more than 5 LEDs need to be turned on. * Support 4 different brightness levels. --- src/leddrv.c | 209 +++++++++++++++++++++++++++++++-------------------- src/leddrv.h | 1 - src/main.c | 37 +++++---- 3 files changed, 146 insertions(+), 101 deletions(-) diff --git a/src/leddrv.c b/src/leddrv.c index 4a428ba..37c3e4b 100644 --- a/src/leddrv.c +++ b/src/leddrv.c @@ -2,7 +2,7 @@ #define LED_PINCOUNT (23) -volatile int drive_strength; +static uint32_t g_pindrive_strong; typedef enum { FLOATING, @@ -10,123 +10,159 @@ typedef enum { HIGH, } tristate_t; -typedef struct { - uint32_t *port_buf; - uint32_t *cfg_buf; - uint32_t pin; -} pinctrl_t; +typedef struct pinbank { + volatile uint8_t *base; -static void gpio_buf_set(pinctrl_t pinctl, tristate_t state) + uint32_t bankpins_mask; + + uint32_t out_val; + uint32_t dir_val; + uint32_t drv_val; +} pinbank_t; + +typedef struct pindesc { + pinbank_t *bank; + uint32_t pin_mask; +} pindesc_t; + +static struct pinbanks { + pinbank_t A; + pinbank_t B; +} g_PinBanks; + + +static void gpio_pin_set(const pindesc_t *desc, tristate_t state) { - if (state == FLOATING) { - *(pinctl.cfg_buf) &= ~pinctl.pin; - } else { - if (state == HIGH) - *(pinctl.port_buf) |= pinctl.pin; - else - *(pinctl.port_buf) &= ~pinctl.pin; - - *(pinctl.cfg_buf) |= pinctl.pin; + switch(state) { + case FLOATING: + desc->bank->dir_val &= ~desc->pin_mask; + break; + case HIGH: + desc->bank->dir_val |= desc->pin_mask; + desc->bank->out_val |= desc->pin_mask; + break; + default: + desc->bank->dir_val |= desc->pin_mask; + desc->bank->out_val &= ~desc->pin_mask; } } -void led_setDriveStrength(int is_20mA) +static void gpio_bank_tristate(pinbank_t *bank) { - drive_strength = is_20mA; + volatile uint32_t *out = (volatile uint32_t *)(bank->base + GPIO_OUT); + volatile uint32_t *drv = (volatile uint32_t *)(bank->base + GPIO_PD_DRV); + volatile uint32_t *dir = (volatile uint32_t *)(bank->base + GPIO_DIR); + uint32_t bankpins_mask; + + // Get state for pins we do NOT control + bankpins_mask = bank->bankpins_mask; + bank->out_val = + (bank->out_val & bankpins_mask) | (*out & ~bankpins_mask); + bank->dir_val = + (bank->dir_val & bankpins_mask) | (*dir & ~bankpins_mask); + bank->drv_val = + (g_pindrive_strong & bank->dir_val & bankpins_mask) | (*drv & ~bankpins_mask); + + // ... and tristate pins, we DO control + *dir = (bank->dir_val & ~bankpins_mask); } -static void gpio_buf_apply( - volatile uint8_t *gpio_base, - uint32_t *port, uint32_t *cfg, - uint32_t *mask) +static void gpio_bank_apply(pinbank_t *bank) { - if (drive_strength) { - uint32_t *drv = (uint32_t *)(gpio_base + GPIO_PD_DRV); - *drv = (*drv & ~*mask) | (*cfg & *mask); - } - uint32_t *dir = (uint32_t *)(gpio_base + GPIO_DIR); - *dir = (*dir & ~*mask) | (*cfg & *mask); + volatile uint32_t *out = (volatile uint32_t *)(bank->base + GPIO_OUT); + volatile uint32_t *drv = (volatile uint32_t *)(bank->base + GPIO_PD_DRV); + volatile uint32_t *dir = (volatile uint32_t *)(bank->base + GPIO_DIR); - uint32_t *out = (uint32_t *)(gpio_base + GPIO_OUT); - *out = (*out & ~*mask) | (*port & *mask); + *out = bank->out_val; + *drv = bank->drv_val; + *dir = bank->dir_val; } -static uint32_t PA_buf; -static uint32_t PB_buf; -static uint32_t PAcfg_buf; -static uint32_t PBcfg_buf; -static uint32_t PA_mask; -static uint32_t PB_mask; +#define GPIO_APPLY_ALL() do { \ + gpio_bank_tristate(&g_PinBanks.A); \ + gpio_bank_tristate(&g_PinBanks.B); \ + gpio_bank_apply(&g_PinBanks.A); \ + gpio_bank_apply(&g_PinBanks.B); \ +} while (0) -#define GPIO_APPLY_ALL() \ - gpio_buf_apply(BA_PA, &PA_buf, &PAcfg_buf, &PA_mask); \ - gpio_buf_apply(BA_PB, &PB_buf, &PBcfg_buf, &PB_mask) -#define PINCTRL(x, pin) { \ - &P##x##_buf, \ - &P##x##cfg_buf, \ - GPIO_Pin_##pin \ +#define PINDESC(bank_, pinnr_) { \ + &g_PinBanks.bank_ , \ + GPIO_Pin_##pinnr_ \ } -static const pinctrl_t led_pins[LED_PINCOUNT] = { - PINCTRL(A, 15), // A - PINCTRL(B, 18), // B - PINCTRL(B, 0), // C - PINCTRL(B, 7), // D - PINCTRL(A, 12), // E - PINCTRL(A, 10), // F - PINCTRL(A, 11), // G - PINCTRL(B, 9), // H - PINCTRL(B, 8), // I - PINCTRL(B, 15), // J - PINCTRL(B, 14), // K - PINCTRL(B, 13), // L - PINCTRL(B, 12), // M - PINCTRL(B, 5), // N - PINCTRL(A, 4), // O - PINCTRL(B, 3), // P - PINCTRL(B, 4), // Q - PINCTRL(B, 2), // R - PINCTRL(B, 1), // S +static const pindesc_t led_pins[LED_PINCOUNT] = { + PINDESC(A, 15), // A + PINDESC(B, 18), // B + PINDESC(B, 0), // C + PINDESC(B, 7), // D + PINDESC(A, 12), // E + PINDESC(A, 10), // F + PINDESC(A, 11), // G + PINDESC(B, 9), // H + PINDESC(B, 8), // I + PINDESC(B, 15), // J + PINDESC(B, 14), // K + PINDESC(B, 13), // L + PINDESC(B, 12), // M + PINDESC(B, 5), // N + PINDESC(A, 4), // O + PINDESC(B, 3), // P + PINDESC(B, 4), // Q + PINDESC(B, 2), // R + PINDESC(B, 1), // S #ifdef USBC_VERSION - PINCTRL(B, 6), // T + PINDESC(B, 6), // T #else - PINCTRL(B, 23), // T + PINDESC(B, 23), // T #endif - PINCTRL(B, 21), // U - PINCTRL(B, 20), // V - PINCTRL(B, 19), // W + PINDESC(B, 21), // U + PINDESC(B, 20), // V + PINDESC(B, 19), // W }; void led_init() { - for (int i=0; ibankpins_mask |= led_pins[i].pin_mask; } } void leds_releaseall() { for (int i=0; i LED on + } + gpio_pin_set(led_pins + i, pin_value); val >>= 1; } + g_pindrive_strong = 0x00000000; + if (on_count > 5) + g_pindrive_strong = 0xFFFFFFFF; GPIO_APPLY_ALL(); } -static uint32_t combine_cols(uint16_t col1_val, uint16_t col2_val) +static uint32_t combine_cols(uint16_t col1_val, uint16_t col2_val) { uint32_t dval = 0; dval |= ((col1_val & 0x01) << (LED_ROWS*2)); @@ -156,13 +192,24 @@ void led_write2dcol(int dcol, uint16_t col1_val, uint16_t col2_val) void led_write2row_raw(int row, int which_half, uint32_t val) { - row = row*2 + (which_half != 0); + int on_count; + int pin_value; - gpio_buf_set(led_pins[row], LOW); + row = row*2 + (which_half != 0); + gpio_pin_set(led_pins + row, LOW); + on_count = 0; for (int i=0; i>= 1; } + g_pindrive_strong = 0x00000000; + if (on_count > 5) + g_pindrive_strong = 0xFFFFFFFF; GPIO_APPLY_ALL(); } diff --git a/src/leddrv.h b/src/leddrv.h index 7474363..b9be6ba 100644 --- a/src/leddrv.h +++ b/src/leddrv.h @@ -10,6 +10,5 @@ void led_init(); void leds_releaseall(); void led_write2dcol(int dcol, uint16_t col1_val, uint16_t col2_val); void led_write2row_raw(int row, int which_half, uint32_t val); -void led_setDriveStrength(int is_20mA); #endif /* __LEDDRV_H__ */ diff --git a/src/main.c b/src/main.c index b531f7e..ab82307 100644 --- a/src/main.c +++ b/src/main.c @@ -60,7 +60,6 @@ __HIGH_CODE static void change_brightness() { NEXT_STATE(brightness, 0, BRIGHTNESS_LEVELS); - led_setDriveStrength(brightness / 2); } __HIGH_CODE @@ -272,7 +271,7 @@ void spawn_tasks() tmos_start_reload_task(common_taskid, ANI_MARQUE, ANI_MARQUE_SPEED_T / 625); tmos_start_reload_task(common_taskid, ANI_FLASH, ANI_FLASH_SPEED_T / 625); - tmos_start_reload_task(common_taskid, SCAN_BOOTLD_BTN, + tmos_start_reload_task(common_taskid, SCAN_BOOTLD_BTN, SCAN_BOOTLD_BTN_SPEED_T / 625); tmos_start_task(common_taskid, ANI_NEXT_STEP, 500000 / 625); } @@ -300,20 +299,20 @@ void handle_mode_transition() // Disable bitmap transition while in download mode btn_onOnePress(KEY2, NULL); - // Take control of the current bitmap to display + // Take control of the current bitmap to display // the Bluetooth animation ble_start(); while (mode == DOWNLOAD) { TMOS_SystemProcess(); } - // If not being flashed, pressing KEY1 again will + // If not being flashed, pressing KEY1 again will // make the badge goes off: - + // fallthrough case POWER_OFF: poweroff(); break; - + default: break; } @@ -402,7 +401,7 @@ static void fb_putchar(char c, int col, int row) { for (int i=0; i < 6; i++) { if (col + i >= LED_COLS) break; - fb[col + i] = (fb[col + i] & ~(0x7f << row)) + fb[col + i] = (fb[col + i] & ~(0x7f << row)) | (font5x7[c - ' '][i] << row); } } @@ -450,7 +449,7 @@ int main() usb_start(); led_init(); - TMR0_TimerInit(SCAN_T / 2); + TMR0_TimerInit(SCAN_T / 4); TMR0_ITCfg(ENABLE, TMR0_3_IT_CYC_END); PFIC_EnableIRQ(TMR0_IRQn); @@ -462,7 +461,7 @@ int main() btn_onLongPress(KEY1, change_brightness); disp_charging(); - + play_splash(&splash, 0, 0); load_bmlist(); @@ -483,19 +482,19 @@ __HIGH_CODE void TMR0_IRQHandler(void) { static int i; + int state; if (TMR0_GetITFlag(TMR0_3_IT_CYC_END)) { - i += 1; - if (i >= LED_COLS) { - i = 0; - } - - if (i % 2) { - if ((brightness + 1) % 2) - leds_releaseall(); - } else { - led_write2dcol(i/2, fb[i], fb[i + 1]); + i++; + state = i&3; + + if (state == 0) { + if ((i >> 1) >= LED_COLS) + i = 0; + led_write2dcol(i >> 2, fb[i >> 1], fb[(i >> 1) + 1]); } + else if (state > (brightness&3)) + leds_releaseall(); TMR0_ClearITFlag(TMR0_3_IT_CYC_END); }