Reducing ISSI I2C LED flickering

- During HID-IO testing there was a lot of LED flickering with ATSAM4S
  I2C + ISSI I2C devices running faster than 400 kHz (mainly 31FL3733)
- With a bunch of testing the baud rate was brought down to 460 kHz from
  800 kHz and the clock divider was increased to 1.
- This means it's very difficult to maintain 100 FPS (83.33 FPS, 2 levels
  down has a solid lock).
- This issue does not affect newer keyboards that used ISSI SPI
  controllers (currently the preferred controller)
- Reorganized LED_reset code for I2C ISSI Chips
  * Removed last is first synchronization hack/fix (causing
    initialization errors)
  * Increase I2C bus speed to reduce the chance of noise errors during
    initialization, then reduce to normal speed

Scan/Devices/ISSILed/i2c.c

@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2014 Jan Rychter
- * Modifications (C) 2015-2018 Jacob Alexander
+ * Modifications (C) 2015-2020 Jacob Alexander
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files ( the "Software" ), to deal
@@ -65,7 +65,7 @@ void i2c_initial()
 	}
 }
 
-void i2c_setup()
+void i2c_setup(uint8_t fast)
 {
 	for ( uint8_t ch = ISSI_I2C_FirstBus_define; ch < ISSI_I2C_Buses_define + ISSI_I2C_FirstBus_define; ch++ )
 	{
@@ -165,11 +165,26 @@ void i2c_setup()
 #elif defined(_sam_)
 
 #if ISSI_Chip_31FL3731_define == 1
+#define BAUD_FAST 400000
+#define CK_FAST 1
 #define BAUD 400000
 #define CK 1
 #elif ISSI_Chip_31FL3732_define == 1 || ISSI_Chip_31FL3733_define == 1 || ISSI_Chip_31FL3736_define == 1
-#define BAUD 800000
-#define CK 0
+// XXX (HaaTa): ATSAM4S seems to have a limitation of about 400 kHz
+//              This is the maximum the datasheet supports.
+//              ASF errors out when specifying over 400 kHz.
+//              Setting the Clock Divider at 1, allows for a safe 460 kHz on ATSAM4S.
+//              If you're reading this, please use an SPI ISSI chip, SPI works great on ATSAM4S.
+//              Going much above 460 kHz is going to require serious tuning and pcb adjustments.
+//              Just make sure you run a lot of USB traffic to test for flickering.
+//
+// XXX (HaaTa): The purpose of BAUD_FAST is for initialization of the ISSI controller
+//              as some ISSI chips are buggy when intialized too slowly.
+//              Speeding up this section (which will not have any flickering) seems to reduce errors.
+#define BAUD_FAST 900000
+#define CK_FAST 1
+#define BAUD 460000
+#define CK 1
 #endif
 
 		switch ( ch )
@@ -188,10 +203,20 @@ void i2c_setup()
 		}
 
 		Twi *twi_dev = twi_devs[ch];
-		uint16_t div = (F_CPU/BAUD - 4) / (2<<CK);
+		if (fast)
+		{
+			uint16_t div = (F_CPU/BAUD_FAST - 4) / (2<<CK_FAST);
 
-		// Set clock
-		twi_dev->TWI_CWGR = TWI_CWGR_CLDIV(div) + TWI_CWGR_CHDIV(div) + TWI_CWGR_CKDIV(CK);
+			// Set clock
+			twi_dev->TWI_CWGR = TWI_CWGR_CLDIV(div) + TWI_CWGR_CHDIV(div) + TWI_CWGR_CKDIV(CK_FAST);
+		}
+		else
+		{
+			uint16_t div = (F_CPU/BAUD - 4) / (2<<CK);
+
+			// Set clock
+			twi_dev->TWI_CWGR = TWI_CWGR_CLDIV(div) + TWI_CWGR_CHDIV(div) + TWI_CWGR_CKDIV(CK);
+		}
 
 		// Enable master mode
 		twi_dev->TWI_CR = TWI_CR_MSDIS | TWI_CR_SVDIS;
@@ -237,7 +262,7 @@ void i2c_reset()
 		channel->status = I2C_AVAILABLE;
 	}
 
-	i2c_setup();
+	i2c_setup(0);
 }
 
 uint8_t i2c_busy( uint8_t ch )

Scan/Devices/ISSILed/i2c.h

@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2014 Jan Rychter
- * Modifications (C) 2015-2018 Jacob Alexander
+ * Modifications (C) 2015-2020 Jacob Alexander
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -71,7 +71,7 @@ void i2c_initial();
  * I2C Module Setup
  */
 
-void i2c_setup();
+void i2c_setup(uint8_t fast);
 
 
 /*

Scan/Devices/ISSILed/led_scan.c

@@ -1,4 +1,4 @@
-/* Copyright (C) 2014-2019 by Jacob Alexander
+/* Copyright (C) 2014-2020 by Jacob Alexander
  *
  * This file is free software: you can redistribute it and/or modify
  * it under the terms of the GNU Lesser General Public License as published by
@@ -397,7 +397,7 @@ void LED_sendPage( uint8_t bus, uint8_t addr, uint16_t *buffer, uint32_t len, ui
 	print(")page(");
 	printHex( page );
 	print(")data[](");
-	for ( uint8_t c = 0; c < 9; c++ )
+	for ( uint8_t c = 0; c < len; c++ )
 	{
 		printHex( buffer[c] );
 		print(" ");
@@ -519,30 +519,47 @@ uint8_t LED_readReg( uint8_t bus, uint8_t addr, uint8_t reg, uint8_t page )
 
 void LED_reset()
 {
+	// Initialize I2C in fast mode
+	i2c_setup(1);
+
 	// Force PixelMap to stop during reset
 	Pixel_FrameState = FrameState_Sending;
 
 	// Disable FPS by default
 	LED_displayFPS = 0;
 
-	// Enable Hardware shutdown (pull low)
+	// Hardware shutdown (pull low)
 	GPIO_Ctrl( hardware_shutdown_pin, GPIO_Type_DriveSetup, GPIO_Config_Pullup );
 	GPIO_Ctrl( hardware_shutdown_pin, GPIO_Type_DriveLow, GPIO_Config_Pullup );
-	delay_us(50);
+	delay_us(200);
 
 #if ISSI_Chip_31FL3733_define == 1 || ISSI_Chip_31FL3736_define == 1
 	// Reset I2C bus
-	GPIO_Ctrl( iirst_pin, GPIO_Type_DriveSetup, GPIO_Config_Pullup );
-	GPIO_Ctrl( iirst_pin, GPIO_Type_DriveHigh, GPIO_Config_Pullup );
-	delay_us(50);
-	GPIO_Ctrl( iirst_pin, GPIO_Type_DriveLow, GPIO_Config_Pullup );
+	GPIO_Ctrl( iirst_pin, GPIO_Type_DriveSetup, GPIO_Config_Pulldown );
+	GPIO_Ctrl( iirst_pin, GPIO_Type_DriveLow, GPIO_Config_Pulldown );
+	delay_us(200);
+	GPIO_Ctrl( iirst_pin, GPIO_Type_DriveHigh, GPIO_Config_Pulldown );
+	delay_us(200);
+	GPIO_Ctrl( iirst_pin, GPIO_Type_DriveLow, GPIO_Config_Pulldown );
+	delay_us(500);
 #endif
 
-	// Disable Hardware shutdown of ISSI chips (pull high)
-	if ( LED_enable && LED_enable_current )
+#if ISSI_Chip_31FL3733_define == 1 || ISSI_Chip_31FL3736_define == 1
+	// Clear LED Pages
+	// Software shutdown
+	for ( uint8_t ch = 0; ch < ISSI_Chips_define; ch++ )
 	{
-		GPIO_Ctrl( hardware_shutdown_pin, GPIO_Type_DriveHigh, GPIO_Config_Pullup );
+		uint8_t addr = LED_ChannelMapping[ ch ].addr;
+		uint8_t bus = LED_ChannelMapping[ ch ].bus;
+
+		// POR (Power-on-Reset)
+		LED_readReg( bus, addr, 0x11, ISSI_ConfigPage );
+		delay_us(200);
+
+		// Software shutdown
+		LED_writeReg( bus, addr, 0x00, 0x00, ISSI_ConfigPage );
 	}
+#endif
 
 	// Clear LED Pages
 	// Enable LEDs based upon mask
@@ -552,11 +569,6 @@ void LED_reset()
 		uint8_t bus = LED_ChannelMapping[ ch ].bus;
 
 #if ISSI_Chip_31FL3733_define == 1 || ISSI_Chip_31FL3736_define == 1
-		// POR (Power-on-Reset)
-		// Clears all registers to default value (i.e. zeros)
-		LED_readReg( bus, addr, 0x11, ISSI_ConfigPage );
-		delay_us(50); // Give some time for the ISSI chip to reset
-
 		// Set the enable mask
 		LED_sendPage(
 			bus,
@@ -610,11 +622,12 @@ void LED_reset()
 		uint8_t bus = LED_ChannelMapping[ ch ].bus;
 
 #if ISSI_Chip_31FL3733_define == 1 || ISSI_Chip_31FL3736_define == 1
+		// Disable software shutdown
+		LED_writeReg( bus, addr, 0x00, 0x01, ISSI_ConfigPage );
+		delay_us(200);
+
 		// Enable master sync for the last chip and disable software shutdown
-		// XXX (HaaTa); The last chip is used as it is the last chip all of the frame data is sent to
-		// This is imporant as it may take more time to send the packet than the ISSI chip can handle
-		// between frames.
-		if ( ch == ISSI_Chips_define - 1 )
+		if ( ch == 0 )
 		{
 			LED_writeReg( bus, addr, 0x00, 0x41, ISSI_ConfigPage );
 		}
@@ -647,6 +660,15 @@ void LED_reset()
 #endif
 	}
 
+	// Disable Hardware shutdown of ISSI chips (pull high)
+	if ( LED_enable && LED_enable_current )
+	{
+		GPIO_Ctrl( hardware_shutdown_pin, GPIO_Type_DriveHigh, GPIO_Config_Pullup );
+	}
+
+	// Initialize I2C in slow mode
+	i2c_setup(0);
+
 	// Force PixelMap to be ready for the next frame
 	Pixel_FrameState = FrameState_Update;
 
@@ -743,9 +765,6 @@ inline void LED_setup()
 	// Initialize I2C error counters
 	i2c_initial();
 
-	// Initialize I2C
-	i2c_setup();
-
 	// Setup LED_pageBuffer addresses and brightness section
 	LED_pageBuffer[0].i2c_addr = LED_MapCh1_Addr_define;
 	LED_pageBuffer[0].reg_addr = ISSI_LEDPwmRegStart;

Scan/Gemini_Dusk_Dawn/scancode_map.kll

@@ -1,11 +1,11 @@
 # Gemini Dusk/Dawn Base Configuration
 
 Name = GeminiDuskDawn;
-Version = 0.6;
-Author = "HaaTa (Jacob Alexander) 2015-2019";
+Version = 0.7;
+Author = "HaaTa (Jacob Alexander) 2015-2020";
 
 # Modified Date
-Date = 2019-12-06;
+Date = 2020-03-17;
 
 
 # ScanCode Strobe List
@@ -235,7 +235,7 @@ Pixel_HardCode_Channels = 3;
 # So the framerate will not go above this amount.
 # If the framerate goes below, ledFPS cli will issue warnings when enabled.
 ISSI_FrameRate_ms => ISSI_FrameRate_ms_define;
-ISSI_FrameRate_ms = 10; # 1000 / <ISSI_FrameRate_ms> = 100 fps
+ISSI_FrameRate_ms = 12; # 1000 / <ISSI_FrameRate_ms> = 83.33 fps
 
 
 

Scan/Kira/scancode_map.kll

@@ -1,11 +1,11 @@
 # Kira Base Configuration
 
 Name = Kira;
-Version = 0.11;
-Author = "HaaTa (Jacob Alexander) 2017-2019";
+Version = 0.12;
+Author = "HaaTa (Jacob Alexander) 2017-2020";
 
 # Modified Date
-Date = 2019-12-06;
+Date = 2020-03-17;
 
 
 # Kira
@@ -233,7 +233,7 @@ Pixel_HardCode_Channels = 3;
 # So the framerate will not go above this amount.
 # If the framerate goes below, ledFPS cli will issue warnings when enabled.
 ISSI_FrameRate_ms => ISSI_FrameRate_ms_define;
-ISSI_FrameRate_ms = 10; # 1000 / <ISSI_FrameRate_ms> = 100 fps
+ISSI_FrameRate_ms = 12; # 1000 / <ISSI_FrameRate_ms> = 83.33 fps