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

Add ST7565 LCD driver #13089

Merged
merged 9 commits into from
Jun 10, 2021
Merged

Add ST7565 LCD driver #13089

merged 9 commits into from
Jun 10, 2021

Conversation

fauxpark
Copy link
Member

@fauxpark fauxpark commented Jun 3, 2021

Description

One more step towards eliminating uGFX. Adapted from the SSD1306/OLED driver. Tested on Proton-C but should work on AVR as well.

@firetech

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@firetech
Copy link
Contributor

firetech commented Jun 3, 2021

Nice! I'll try playing around with it. :)

@firetech
Copy link
Contributor

firetech commented Jun 3, 2021

There's a kind of big hurdle here... drivers/chibios/spi_master.c doesn't compile for K20x... Their SPIConfig struct is quite different from STM32 (see ergodox_infinity/board_st7565.h for some working K20x code). Not sure how to proceed...

USART has basically the same issue, but I had to write a custom serial driver when porting split_common anyway, due to the Infinity using one USART port on the master and another on the slave, so I got around that there.

Edit: The changes I made to test this are here: https://github.com/firetech/qmk_firmware/tree/st7565_test

@KarlK90
Copy link
Member

KarlK90 commented Jun 3, 2021

@firetech I'm in the process of overhauling the serial driver in #13081 and making the driver less stm32 specific would be great. From what I see in your code the serial_config, assigned serial driver and some hooks into the side specific init code would have to be configurable right? Feel free to chime in the serial PR :-)

@fauxpark
Copy link
Member Author

fauxpark commented Jun 3, 2021

The SPIConfig (hal_spi_config) struct is actually mostly the same:
https://chibiforge.org/doc/19.1/full_rm/structhal__spi__config.html

Where they differ is in the spi_lld_config_fields, which is implemented by each SPI driver. For example STM32's SPIv3 appends the cfg1 and cfg2 fields corresponding to the two SPI configuration registers, whereas Kinetis just has one tar0 field for the SPI0_CTAR0 register. The MK20DX256 appears to have two CTAR registers, but only one is needed as they are identical and seem to be useful for quickly switching between configurations.
https://github.com/qmk/ChibiOS-Contrib/blob/HEAD/os/hal/ports/KINETIS/LLD/SPIv1/hal_spi_lld.h#L121

So, the config from the uGFX template can basically be transplanted in with an ifdef, with some other tweaks. I'll give it a try tomorrow to see if it compiles and draft a PR, but I don't have any Kinetis devices on hand to test.

@firetech
Copy link
Contributor

firetech commented Jun 3, 2021

So, the config from the uGFX template can basically be transplanted in with an ifdef, with some other tweaks. I'll give it a try tomorrow to see if it compiles and draft a PR, but I don't have any Kinetis devices on hand to test.

Ping me in a draft PR or just on Discord and I'll be happy to test :)

@firetech
Copy link
Contributor

firetech commented Jun 3, 2021

Together with #13098, seems to work :)
image

Edit: Code here.

Copy link
Contributor

@firetech firetech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor mistakes in the documentation.

docs/feature_st7565.md Outdated Show resolved Hide resolved
docs/feature_st7565.md Outdated Show resolved Hide resolved
Co-authored-by: Joakim Tufvegren <[email protected]>
@firetech
Copy link
Contributor

firetech commented Jun 3, 2021

One thing I would like to request (but not sure how/if it should be implemented) is some form of user/kb callback in st7565_on/off(), since the NHD-C12832A1Z has RGB backlight that is controlled completely independently from the LCD contents (just four pins for R, G, B and common anode). It would probably be nice to have that turn on/off with the rest of the display.

@fauxpark
Copy link
Member Author

fauxpark commented Jun 4, 2021

How's this?

@firetech
Copy link
Contributor

firetech commented Jun 4, 2021

Great! ❤️

I'm going to spend some of my weekend porting over my keymap's display code (based on quantum/visualizer) to this to see if I can get myself a uGFX-less daily driver firmware. 🙂

@firetech
Copy link
Contributor

firetech commented Jun 5, 2021

A slightly more philosophical question... Currently, a default build of Ergodox Infinity has a visualizer implemented (even for other keymaps than the default). Should, for this reason, a default (weak) st7565_task_user be implemented for a future switchover of Ergodox Infinity, or is that just a waste of firmware space?

For comparison, how are keyboards with OLEDs handled in the configurator?

@fauxpark
Copy link
Member Author

fauxpark commented Jun 5, 2021

how are keyboards with OLEDs handled in the configurator?

They're not. The Configurator generates its own keymap rather than using the default keymap, so there is no OLED code present and it will simply be blank. Personally I'm fine with having this behaviour for the Ergodox Infinity as well - currently you can't build it in Configurator anyway due to uGFX.

drivers/lcd/st7565.h Outdated Show resolved Hide resolved
drivers/lcd/st7565.c Outdated Show resolved Hide resolved
@firetech
Copy link
Contributor

firetech commented Jun 5, 2021

I've now finalized my keymap implementation of this, here. Changing from Visualizer to this more or less quadrupled my matrix scan rate. 😄

I have a separate branch of just the keyboard-level changes, and intend to make a PR out of that. The dependency tree has become quite horrible, though. It more or less depends on both this, your SPI for Kinetis PR and my not yet submitted split_common PR, which in turn depends on my LED Matrix PR and two other PRs related to split_common and serial... Should I wait for all the required PRs to be merged before submitting each PR? 🤔

@fauxpark
Copy link
Member Author

fauxpark commented Jun 6, 2021

If it doesn't compile without these PRs, then yeah, it may have to wait.

@firetech
Copy link
Contributor

firetech commented Jun 8, 2021

If it doesn't compile without these PRs, then yeah, it may have to wait.

The ST7565 migration of Ergodox Infinity basically only needs this PR now to compile, but it doesn't make much sense to merge it without merging the LED Matrix migration first (mostly because those PR's would conflict otherwise).

When this and #9657 have been merged, I'll submit a PR for migrating Ergodox Infinity over to this driver. The split_common migration can wait, since it's already waiting for two other PRs. Doing it in that order also removes the need for some code I added to sync the visualizer over split_common, which might be a plus. 😛

@fauxpark fauxpark merged commit b2fdd48 into qmk:develop Jun 10, 2021
@fauxpark fauxpark deleted the st7565-driver branch June 10, 2021 07:16
mechlovin pushed a commit to mechlovin/qmk_firmware that referenced this pull request Jul 30, 2021
mechlovin pushed a commit to mechlovin/qmk_firmware that referenced this pull request Jul 30, 2021
nhongooi pushed a commit to nhongooi/qmk_firmware that referenced this pull request Dec 5, 2021
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
Co-authored-by: Joakim Tufvegren <[email protected]>
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.

4 participants