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

@@ -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 {
+typedef struct pinbank {
-	uint32_t *port_buf;
+	volatile uint8_t *base;
-	uint32_t *cfg_buf;
-	uint32_t pin;
-} pinctrl_t;
 
-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) {
+	switch(state) {
-		*(pinctl.cfg_buf) &= ~pinctl.pin;
+	case FLOATING:
-	} else {
+		desc->bank->dir_val &= ~desc->pin_mask;
-		if (state == HIGH) 
+		break;
-			*(pinctl.port_buf) |= pinctl.pin;
+	case HIGH:
-		else 
+		desc->bank->dir_val |= desc->pin_mask;
-			*(pinctl.port_buf) &= ~pinctl.pin;
+		desc->bank->out_val |= desc->pin_mask;
-
+		break;
-		*(pinctl.cfg_buf) |= pinctl.pin;
+	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(
+static void gpio_bank_apply(pinbank_t *bank)
-				volatile uint8_t *gpio_base, 
-				uint32_t *port, uint32_t *cfg, 
-				uint32_t *mask)
 {
-	if (drive_strength) {
+	volatile uint32_t *out = (volatile uint32_t *)(bank->base + GPIO_OUT);
-		uint32_t *drv = (uint32_t *)(gpio_base + GPIO_PD_DRV);
+	volatile uint32_t *drv = (volatile uint32_t *)(bank->base + GPIO_PD_DRV);
-		*drv = (*drv & ~*mask) | (*cfg & *mask);
+	volatile uint32_t *dir = (volatile uint32_t *)(bank->base + GPIO_DIR);
-	}
-	uint32_t *dir = (uint32_t *)(gpio_base + GPIO_DIR);
-	*dir = (*dir & ~*mask) | (*cfg & *mask);
 
-	uint32_t *out = (uint32_t *)(gpio_base + GPIO_OUT);
+	*out = bank->out_val;
-	*out = (*out & ~*mask) | (*port & *mask);
+	*drv = bank->drv_val;
+	*dir = bank->dir_val;
 }
 
-static uint32_t PA_buf;
+#define GPIO_APPLY_ALL() do { \
-static uint32_t PB_buf;
+	gpio_bank_tristate(&g_PinBanks.A); \
-static uint32_t PAcfg_buf;
+	gpio_bank_tristate(&g_PinBanks.B); \
-static uint32_t PBcfg_buf;
+	gpio_bank_apply(&g_PinBanks.A); \
-static uint32_t PA_mask;
+	gpio_bank_apply(&g_PinBanks.B); \
-static uint32_t PB_mask;
+} 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) {	\
+#define PINDESC(bank_, pinnr_) { \
-	&P##x##_buf, 		\
+	&g_PinBanks.bank_ , \
-	&P##x##cfg_buf, 	\
+	GPIO_Pin_##pinnr_ \
-	GPIO_Pin_##pin 			\
 }
 
-static const pinctrl_t led_pins[LED_PINCOUNT] = {
+static const pindesc_t led_pins[LED_PINCOUNT] = {
-	PINCTRL(A, 15), // A 
+	PINDESC(A, 15), // A
-	PINCTRL(B, 18), // B
+	PINDESC(B, 18), // B
-	PINCTRL(B, 0),  // C
+	PINDESC(B, 0),  // C
-	PINCTRL(B, 7),  // D
+	PINDESC(B, 7),  // D
-	PINCTRL(A, 12), // E
+	PINDESC(A, 12), // E
-	PINCTRL(A, 10), // F
+	PINDESC(A, 10), // F
-	PINCTRL(A, 11), // G
+	PINDESC(A, 11), // G
-	PINCTRL(B, 9),  // H
+	PINDESC(B, 9),  // H
-	PINCTRL(B, 8),  // I
+	PINDESC(B, 8),  // I
-	PINCTRL(B, 15), // J
+	PINDESC(B, 15), // J
-	PINCTRL(B, 14), // K
+	PINDESC(B, 14), // K
-	PINCTRL(B, 13), // L
+	PINDESC(B, 13), // L
-	PINCTRL(B, 12), // M
+	PINDESC(B, 12), // M
-	PINCTRL(B, 5),  // N
+	PINDESC(B, 5),  // N
-	PINCTRL(A, 4),  // O
+	PINDESC(A, 4),  // O
-	PINCTRL(B, 3),  // P
+	PINDESC(B, 3),  // P
-	PINCTRL(B, 4),  // Q
+	PINDESC(B, 4),  // Q
-	PINCTRL(B, 2),  // R
+	PINDESC(B, 2),  // R
-	PINCTRL(B, 1),  // S
+	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
+	PINDESC(B, 21), // U
-	PINCTRL(B, 20), // V
+	PINDESC(B, 20), // V
-	PINCTRL(B, 19), // W
+	PINDESC(B, 19), // W
 };
 
 void led_init()
 {
-	for (int i=0; i<LED_PINCOUNT; i++) {
+	int i;
-		if (led_pins[i].port_buf == &PA_buf)
+
-			PA_mask |= led_pins[i].pin;
+	g_PinBanks.A.base = BA_PA;
-		else
+	g_PinBanks.B.base = BA_PB;
-			PB_mask |= led_pins[i].pin;
+	for (i = 0; i < LED_PINCOUNT; i++) {
+		led_pins[i].bank->bankpins_mask |= led_pins[i].pin_mask;
 	}
 }
 
 void leds_releaseall() {
 	for (int i=0; i<LED_PINCOUNT; i++)
-		gpio_buf_set(led_pins[i], FLOATING);
+		gpio_pin_set(led_pins + i, FLOATING);
+	g_pindrive_strong = 0x00000000;
 	GPIO_APPLY_ALL();
 }
 
 static void led_write2dcol_raw(int dcol, uint32_t val)
 {
-	// TODO: assert params
+	int on_count;
-	gpio_buf_set(led_pins[dcol], HIGH);
+	int pin_value;
+
+	gpio_pin_set(led_pins + dcol, HIGH);
+	on_count = 0;
 	for (int i=0; i<LED_PINCOUNT; i++) {
 		if (i == dcol) continue;
-		gpio_buf_set(led_pins[i], (val & 0x01) ? LOW : FLOATING); // danger: floating=0 (led off) or low=1 (led on)
+		pin_value = FLOATING;
+		if (val & 0x01) {
+			on_count++;
+			pin_value = LOW; // pin LOW => 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<LED_PINCOUNT; i++) {
 		if (i == row) continue;
-		gpio_buf_set(led_pins[i], (val & 0x01) ? HIGH : FLOATING);
+		pin_value = FLOATING;
+		if (val & 0x01) {
+			on_count++;
+			pin_value = HIGH;
+		}
+		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();
 }

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__ */

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;
+		i++;
-		if (i >= LED_COLS) {
+		state = i&3;
-			i = 0;
+
-		}
+		if (state == 0) {
-		
+			if ((i >> 1) >= LED_COLS)
-		if (i % 2) {
+				i = 0;
-			if ((brightness + 1) % 2) 
+			led_write2dcol(i >> 2, fb[i >> 1], fb[(i >> 1) + 1]);
-				leds_releaseall();
-		} else {
-			led_write2dcol(i/2, fb[i], fb[i + 1]);
 		}
+		else if (state > (brightness&3))
+			leds_releaseall();
 
 		TMR0_ClearITFlag(TMR0_3_IT_CYC_END);
 	}