Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hardware PWM backlight support for all MCU #4324

Conversation

masterzen
Copy link
Contributor

The hardware based PWM backlight was only supported on the ATmega32U4 PWM pins.
This meant that any board using a different MCU couldn’t use the hardware backlight system unless the backlight pin was among B5, B6, B7, C6.

This patch adds the correct PWM pins for the current MCU for backlight allowing boards with an ATmega32A, ATmega32/16U2 or AT90USB1286/646 MCU to use the hardware backlight system, instead of rolling their own.

Note that this patch doesn’t remove the various custom backlight implementations that are defined in some keyboards. This could be done once those keyboards maintainer have tested that the common implementation works for them.

This PR doesn't directly depend on #4015, but requires it for the pins addresses to be correct on ATmega32A. So to work correctly on this MCU, #4015 should be applied first.

quantum/quantum.c Outdated Show resolved Hide resolved
@masterzen masterzen force-pushed the feature/support-hardware-pwm-on-more-mcu branch from 018893e to e0e2bbc Compare December 2, 2018 10:54
@drashna drashna requested review from mechmerlin and yanfali March 23, 2019 00:26
@zvecr
Copy link
Member

zvecr commented Apr 6, 2019

With this change, on a ATmega32A with #define BACKLIGHT_PIN D4 and #define BACKLIGHT_BREATHING i get the following build errors:

Compiling: quantum/quantum.c                                                                       quantum/quantum.c: In function ‘set_pwm’:
quantum/quantum.c:1286:2: error: ‘OCRxx’ undeclared (first use in this function)
  OCRxx = val;
  ^
quantum/quantum.c:1286:2: note: each undeclared identifier is reported only once for each function it appears in
quantum/quantum.c: In function ‘backlight_set’:
quantum/quantum.c:1297:5: error: ‘TCCRxA’ undeclared (first use in this function)
     TCCRxA &= ~(_BV(COMxx1));
     ^
In file included from /usr/lib/avr/include/avr/io.h:99:0,
                 from quantum/config_common.h:29,
                 from ./keyboards/jj4x4/config.h:20,
                 from <command-line>:0:
quantum/quantum.c:1297:21: error: ‘COMxx1’ undeclared (first use in this function)
     TCCRxA &= ~(_BV(COMxx1));
                     ^
quantum/quantum.c: In function ‘is_breathing’:
quantum/quantum.c:1321:15: error: ‘TIMSK1’ undeclared (first use in this function)
     return !!(TIMSK1 & _BV(TOIE1));
               ^
quantum/quantum.c: In function ‘breathing_enable’:
quantum/quantum.c:1324:42: error: ‘TIMSK1’ undeclared (first use in this function)
 #define breathing_interrupt_enable() do {TIMSK1 |= _BV(TOIE1);} while (0)
                                          ^
quantum/quantum.c:1333:3: note: in expansion of macro ‘breathing_interrupt_enable’
   breathing_interrupt_enable();
   ^
quantum/quantum.c: In function ‘breathing_pulse’:
quantum/quantum.c:1324:42: error: ‘TIMSK1’ undeclared (first use in this function)
 #define breathing_interrupt_enable() do {TIMSK1 |= _BV(TOIE1);} while (0)
                                          ^
quantum/quantum.c:1343:5: note: in expansion of macro ‘breathing_interrupt_enable’
     breathing_interrupt_enable();
     ^
quantum/quantum.c: In function ‘breathing_disable’:
quantum/quantum.c:1325:43: error: ‘TIMSK1’ undeclared (first use in this function)
 #define breathing_interrupt_disable() do {TIMSK1 &= ~_BV(TOIE1);} while (0)
                                           ^
quantum/quantum.c:1348:5: note: in expansion of macro ‘breathing_interrupt_disable’
     breathing_interrupt_disable();
     ^
quantum/quantum.c: In function ‘__vector_9’:
quantum/quantum.c:1325:43: error: ‘TIMSK1’ undeclared (first use in this function)
 #define breathing_interrupt_disable() do {TIMSK1 &= ~_BV(TOIE1);} while (0)
                                           ^
quantum/quantum.c:1412:7: note: in expansion of macro ‘breathing_interrupt_disable’
       breathing_interrupt_disable();
       ^
quantum/quantum.c: In function ‘backlight_init_ports’:
quantum/quantum.c:1448:3: error: ‘TCCRxA’ undeclared (first use in this function)
   TCCRxA = _BV(COMxx1) | _BV(WGM11); // = 0b00001010;
   ^
In file included from /usr/lib/avr/include/avr/io.h:99:0,
                 from quantum/config_common.h:29,
                 from ./keyboards/jj4x4/config.h:20,
                 from <command-line>:0:
quantum/quantum.c:1448:16: error: ‘COMxx1’ undeclared (first use in this function)
   TCCRxA = _BV(COMxx1) | _BV(WGM11); // = 0b00001010;
                ^
quantum/quantum.c:1449:3: error: ‘TCCRxB’ undeclared (first use in this function)
   TCCRxB = _BV(WGM13) | _BV(WGM12) | _BV(CS10); // = 0b00011001;
   ^
quantum/quantum.c:1451:3: error: ‘ICRx’ undeclared (first use in this function)
   ICRx = TIMER_TOP;
   ^
quantum/quantum.c: In function ‘is_breathing’:
quantum/quantum.c:1322:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^
cc1: all warnings being treated as errors

I can also report that #define BACKLIGHT_BREATHING seems very unhappy happy enough using the bodge # define TIMSK1 TIMSK

@zvecr zvecr mentioned this pull request Apr 6, 2019
13 tasks
zvecr added a commit to zvecr/qmk_firmware that referenced this pull request Apr 7, 2019
zvecr added a commit to zvecr/qmk_firmware that referenced this pull request Apr 7, 2019
@zvecr zvecr mentioned this pull request Apr 7, 2019
13 tasks
drashna pushed a commit that referenced this pull request Apr 8, 2019
* Refactor jj40 in line with current ps2avrgb template

* Disable bootmagic lite as it seems to not work on atmega32a/bootloadHID

* Add backlight pwm bodge till #4324 lands

* Increase planck keymap compatibility
drashna pushed a commit that referenced this pull request Apr 8, 2019
* Refactor 4x4 in line with current ps2avrgb template

* Add backlight pwm bodge till #4324 lands

* Disable bootmagic lite as it seems to not work on atmega32a/bootloadHID
@masterzen
Copy link
Contributor Author

@zvecr I'm able to reproduce the TIMSK1 issue, but not the compilation error of OCRxx and TCRxa.
Since the branch is now outdated, I'm going to push a rebased one soon that at least fix the TIMSK1 issue you reported.

@masterzen masterzen force-pushed the feature/support-hardware-pwm-on-more-mcu branch 3 times, most recently from e3197f1 to c1f16df Compare April 8, 2019 22:49
@zvecr
Copy link
Member

zvecr commented Apr 10, 2019

Now #5567 has been merged, and included in this branch, I can confirm that i no longer get the compile error and backlight breathing works as expected on the jj4x4(ATmega32A with #define BACKLIGHT_PIN D4).

@masterzen
Copy link
Contributor Author

@zvecr this is a good news!
Thanks for confirming.

I'm waiting for the koyu error to be fixed upstream and then I'll rebase this PR.

@zvecr zvecr mentioned this pull request Apr 12, 2019
13 tasks
The hardware based PWM backlight was only supported on the ATmega32U4
PWM pins. This meant that any board using a different MCU couldn’t
use the hardware backlight system unless the backlight pin was among
B5, B6, B7, C6.

This patch adds the correct PWM pins for the supported MCU for
backlight allowing boards with an ATmega32A, ATmega32/16U2
or AT90USB1286/646 MCU to use the hardware backlight system,
instead of rolling their own.

Note that this patch doesn’t remove the various custom backlight
implementations that are defined in some keyboards.
@masterzen masterzen force-pushed the feature/support-hardware-pwm-on-more-mcu branch from c1f16df to 4a00375 Compare April 23, 2019 18:19
@masterzen
Copy link
Contributor Author

The build fail, but I don't think this PR is in cause.

@masterzen
Copy link
Contributor Author

In other news, I just rebased the PR on top of HEAD (which now contains #3615)

danielo515 pushed a commit to danielo515/qmk_firmware that referenced this pull request May 15, 2019
* Refactor jj40 in line with current ps2avrgb template

* Disable bootmagic lite as it seems to not work on atmega32a/bootloadHID

* Add backlight pwm bodge till qmk#4324 lands

* Increase planck keymap compatibility
danielo515 pushed a commit to danielo515/qmk_firmware that referenced this pull request May 15, 2019
* Refactor 4x4 in line with current ps2avrgb template

* Add backlight pwm bodge till qmk#4324 lands

* Disable bootmagic lite as it seems to not work on atmega32a/bootloadHID
shimesaba-type0 pushed a commit to shimesaba-type0/qmk_firmware that referenced this pull request Jun 22, 2019
* Refactor jj40 in line with current ps2avrgb template

* Disable bootmagic lite as it seems to not work on atmega32a/bootloadHID

* Add backlight pwm bodge till qmk#4324 lands

* Increase planck keymap compatibility
shimesaba-type0 pushed a commit to shimesaba-type0/qmk_firmware that referenced this pull request Jun 22, 2019
* Refactor 4x4 in line with current ps2avrgb template

* Add backlight pwm bodge till qmk#4324 lands

* Disable bootmagic lite as it seems to not work on atmega32a/bootloadHID
Timbus pushed a commit to Timbus/qmk_firmware that referenced this pull request Jun 23, 2019
* Refactor jj40 in line with current ps2avrgb template

* Disable bootmagic lite as it seems to not work on atmega32a/bootloadHID

* Add backlight pwm bodge till qmk#4324 lands

* Increase planck keymap compatibility
Timbus pushed a commit to Timbus/qmk_firmware that referenced this pull request Jun 23, 2019
* Refactor 4x4 in line with current ps2avrgb template

* Add backlight pwm bodge till qmk#4324 lands

* Disable bootmagic lite as it seems to not work on atmega32a/bootloadHID
@drashna
Copy link
Member

drashna commented Sep 21, 2019

There are a number of merge conflicts that have popped up.

Would you mind updating the PR and fixing these?

@fauxpark
Copy link
Member

I think this may actually be superseded by the improvements already made towards backlight support 😕

@drashna
Copy link
Member

drashna commented Sep 21, 2019

I suspected as much, actually.

@drashna
Copy link
Member

drashna commented Nov 16, 2019

Since, these may have been superseded, and a lack of activity, I'm going to close this PR.

If you want, you can reopen the PR, and fix the merge conflicts.

@drashna drashna closed this Nov 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants