Made AVR backlight pwm resolution configurable (#7521)

* Made static backlight pwm resolution configurable

* Made breathing backlighting configurable too

* Finished my ifdef

* Ran clang-format

* Added missing semi-colon

* Solved weird behaviour by right-shifting the right amount

* Made breathing period scaled on actual pwm frequency

* Made the low end deadzone scaled on the top value

* Moved 'pwm_frequency' declaration outside ifdef

* Fixed 'never used' error

* Fixed 'never used' error

* Fixed breathing ISR to 120Hz

* Removed pwm_frequency constant
Constant is no longer needed since running the breathing ISR at a fixed 120Hz

* Re-add brightness limiting

* re-introduce scaling

1 files changed, 68 insertions, 41 deletions

diff --git a/quantum/backlight/backlight_avr.c b/quantum/backlight/backlight_avr.c
index e47192de34..59050e2505 100644
--- a/quantum/backlight/backlight_avr.c
+++ b/quantum/backlight/backlight_avr.c
@@ -199,13 +199,14 @@ static inline void disable_pwm(void) {
 // reaches the backlight level, where we turn off the LEDs,
 // but also an overflow interrupt when the counter rolls back to 0,
 // in which we're going to turn on the LEDs.
-// The LED will then be on for OCRxx/0xFFFF time, adjusted every 244Hz.
+// The LED will then be on for OCRxx/0xFFFF time, adjusted every 244Hz,
+// or F_CPU/BACKLIGHT_CUSTOM_RESOLUTION if used.
 
 // Triggered when the counter reaches the OCRx value
 ISR(TIMERx_COMPA_vect) { backlight_pins_off(); }
 
 // Triggered when the counter reaches the TOP value
-// this one triggers at F_CPU/65536 =~ 244 Hz
+// this one triggers at F_CPU/ICRx = 16MHz/65536 =~ 244 Hz
 ISR(TIMERx_OVF_vect) {
 #    ifdef BACKLIGHT_BREATHING
     if (is_breathing()) {
@@ -220,8 +221,8 @@ ISR(TIMERx_OVF_vect) {
     // artifacts (especially while breathing, because breathing_task
     // takes many computation cycles).
     // so better not turn them on while the counter TOP is very low.
-    if (OCRxx > 256) {
-        backlight_pins_on();
+    if (OCRxx > ICRx / 250 + 5) {
+        FOR_EACH_LED(backlight_on(backlight_pin);)
     }
 }
 
@@ -231,24 +232,26 @@ ISR(TIMERx_OVF_vect) {
 
 // See http://jared.geek.nz/2013/feb/linear-led-pwm
 static uint16_t cie_lightness(uint16_t v) {
-    if (v <= 5243)     // if below 8% of max
-        return v / 9;  // same as dividing by 900%
-    else {
-        uint32_t y = (((uint32_t)v + 10486) << 8) / (10486 + 0xFFFFUL);  // add 16% of max and compare
-        // to get a useful result with integer division, we shift left in the expression above
-        // and revert what we've done again after squaring.
-        y = y * y * y >> 8;
-        if (y > 0xFFFFUL)  // prevent overflow
-            return 0xFFFFU;
-        else
-            return (uint16_t)y;
+    if (v <= ICRx / 12)  // If the value is less than or equal to ~8% of max
+    {
+        return v / 9;  // Same as dividing by 900%
+    } else {
+        // In the next two lines values are bit-shifted. This is to avoid loosing decimals in integer math.
+        uint32_t y = (((uint32_t)v + ICRx / 6) << 5) / (ICRx / 6 + ICRx);  // If above 8%, add ~16% of max, and normalize with (max + ~16% max)
+        uint32_t out = (y * y * y * ICRx) >> 15;                           // Cube it and undo the bit-shifting. (which is now three times as much due to the cubing)
+
+        if (out > ICRx)  // Avoid overflows
+        {
+            out = ICRx;
+        }
+        return out;
     }
 }
 
-// rescale the supplied backlight value to be in terms of the value limit
+// rescale the supplied backlight value to be in terms of the value limit	// range for val is [0..ICRx]. PWM pin is high while the timer count is below val.
 static uint32_t rescale_limit_val(uint32_t val) { return (val * (BACKLIGHT_LIMIT_VAL + 1)) / 256; }
 
-// range for val is [0..TIMER_TOP]. PWM pin is high while the timer count is below val.
+// range for val is [0..ICRx]. PWM pin is high while the timer count is below val.
 static inline void set_pwm(uint16_t val) { OCRxx = val; }
 
 void backlight_set(uint8_t level) {
@@ -277,7 +280,7 @@ void backlight_set(uint8_t level) {
 #endif
     }
     // Set the brightness
-    set_pwm(cie_lightness(rescale_limit_val(TIMER_TOP * (uint32_t)level / BACKLIGHT_LEVELS)));
+    set_pwm(cie_lightness(rescale_limit_val(ICRx * (uint32_t)level / BACKLIGHT_LEVELS)));
 }
 
 void backlight_task(void) {}
@@ -292,6 +295,11 @@ void backlight_task(void) {}
 static uint8_t  breathing_halt    = BREATHING_NO_HALT;
 static uint16_t breathing_counter = 0;
 
+static uint8_t breath_scale_counter = 1;
+/* Run the breathing loop at ~120Hz*/
+const uint8_t breathing_ISR_frequency = 120;
+static uint16_t breathing_freq_scale_factor = 2;
+
 #    ifdef BACKLIGHT_PWM_TIMER
 static bool breathing = false;
 
@@ -319,14 +327,14 @@ bool is_breathing(void) { return !!(TIMSKx & _BV(TOIEx)); }
             } while (0)
 #    endif
 
-#    define breathing_min()        \
-        do {                       \
-            breathing_counter = 0; \
-        } while (0)
-#    define breathing_max()                                       \
-        do {                                                      \
-            breathing_counter = get_breathing_period() * 244 / 2; \
-        } while (0)
+#            define breathing_min()        \
+                do {                       \
+                    breathing_counter = 0; \
+                } while (0)
+#            define breathing_max()                                           \
+                do {                                                          \
+                    breathing_counter = breathing_period * breathing_ISR_frequency / 2; \
+                } while (0)
 
 void breathing_enable(void) {
     breathing_counter = 0;
@@ -369,21 +377,33 @@ void breathing_task(void)
 #    else
 /* Assuming a 16MHz CPU clock and a timer that resets at 64k (ICR1), the following interrupt handler will run
  * about 244 times per second.
+ *
+ * The following ISR runs at F_CPU/ISRx. With a 16MHz clock and default pwm resolution, that means 244Hz
  */
 ISR(TIMERx_OVF_vect)
 #    endif
 {
-    uint8_t  breathing_period = get_breathing_period();
-    uint16_t interval         = (uint16_t)breathing_period * 244 / BREATHING_STEPS;
+
+    // Only run this ISR at ~120 Hz
+    if(breath_scale_counter++ == breathing_freq_scale_factor)
+    {
+        breath_scale_counter = 1;
+    }
+    else
+    {
+        return;
+    }
+    uint16_t interval = (uint16_t)breathing_period * breathing_ISR_frequency / BREATHING_STEPS;
     // resetting after one period to prevent ugly reset at overflow.
-    breathing_counter = (breathing_counter + 1) % (breathing_period * 244);
-    uint8_t index     = breathing_counter / interval % BREATHING_STEPS;
+    breathing_counter = (breathing_counter + 1) % (breathing_period * breathing_ISR_frequency);
+    uint8_t index = breathing_counter / interval % BREATHING_STEPS;
 
     if (((breathing_halt == BREATHING_HALT_ON) && (index == BREATHING_STEPS / 2)) || ((breathing_halt == BREATHING_HALT_OFF) && (index == BREATHING_STEPS - 1))) {
         breathing_interrupt_disable();
     }
 
-    set_pwm(cie_lightness(rescale_limit_val(scale_backlight((uint16_t)pgm_read_byte(&breathing_table[index]) * 0x0101U))));
+    // Set PWM to a brightnessvalue scaled to the configured resolution
+    set_pwm(cie_lightness(rescale_limit_val(scale_backlight((uint16_t)pgm_read_byte(&breathing_table[index]) * ICRx / 255))));
 }
 
 #endif  // BACKLIGHT_BREATHING
@@ -413,16 +433,23 @@ void backlight_init_ports(void) {
     "In fast PWM mode, the compare units allow generation of PWM waveforms on the OCnx pins. Setting the COMnx1:0 bits to two will produce a non-inverted PWM [..]."
     "In fast PWM mode the counter is incremented until the counter value matches either one of the fixed values 0x00FF, 0x01FF, or 0x03FF (WGMn3:0 = 5, 6, or 7), the value in ICRn (WGMn3:0 = 14), or the value in OCRnA (WGMn3:0 = 15)."
     */
-#    if BACKLIGHT_ON_STATE == 1
-    TCCRxA = _BV(COMxx1) | _BV(WGM11);
-#    else
-    TCCRxA = _BV(COMxx1) | _BV(COMxx0) | _BV(WGM11);
-#    endif
+    TCCRxA = _BV(COMxx1) | _BV(WGM11);             // = 0b00001010;
+    TCCRxB = _BV(WGM13) | _BV(WGM12) | _BV(CS10);  // = 0b00011001;
+#        endif
 
-    TCCRxB = _BV(WGM13) | _BV(WGM12) | _BV(CS10);
-#endif
-    // Use full 16-bit resolution. Counter counts to ICR1 before reset to 0.
-    ICRx = TIMER_TOP;
+#        ifdef BACKLIGHT_CUSTOM_RESOLUTION
+#            if (BACKLIGHT_CUSTOM_RESOLUTION > 0xFFFF || BACKLIGHT_CUSTOM_RESOLUTION < 1)
+#                error "This out of range of the timer capabilities"
+#            elif (BACKLIGHT_CUSTOM_RESOLUTION < 0xFF)
+#                warning "Resolution lower than 0xFF isn't recommended"
+#            endif
+#            ifdef BACKLIGHT_BREATHING
+    breathing_freq_scale_factor = F_CPU / BACKLIGHT_CUSTOM_RESOLUTION / 120;
+#            endif
+    ICRx = BACKLIGHT_CUSTOM_RESOLUTION;
+#        else
+    ICRx   = TIMER_TOP;
+#        endif
 
     backlight_init();
 #ifdef BACKLIGHT_BREATHING
@@ -430,4 +457,4 @@ void backlight_init_ports(void) {
         breathing_enable();
     }
 #endif
-}
+}
\ No newline at end of file