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

[QP] Support for LS0XX displays #21844

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft

Conversation

elpekenin
Copy link
Contributor

Description

Initial support for this family of MIP(memory in display) devices.

I dont really like how the code is integrated right now:

  • These displays use inverted CS (active high)
  • I've tested using #define SPI_SELECT_MODE SPI_SELECT_MODE_NONE and setting the pin from the display driver
  • This is very prone to errors as soon as you add other devices (their CS pins will be ignored by ChibiOS and you'd need to manually control them)
  • Sadly ChibiOS doesnt seem to have an API or anything to make CS work inverted (as far as i can tell), so i have no idea how to make this any better...

Depends on:

PS: According to drFaustroll's research (reported on Discord server), some of this code may also work for displays manufactured by JDI (also MIP but 3bpp -aka 8 colors- instead of monochrome), maybe it could be refactored (or simply renamed) to reuse it across both families.

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).

@drashna drashna added the awaiting_pr Relies on another PR to be merged first label Sep 3, 2023
@infinityis
Copy link
Contributor

infinityis commented Sep 22, 2023

Regarding the inverted CS pin, after looking through the ChibiOS code I agree that it does not support inverted CS pins in any way at all. Thus, I'm seeing three possible ways to reliably overcome this issue:

  • modify Quantum Painter to allow for an inverted CS pin that is actuated manually. For this approach the QP init function would need to include a bool argument to indicate whether CS is inverted, and then the start and stop comms functions would step in and conditionally override the CS pin state (writing to it manually) after every call to spi_start and spi_stop (since those are the driver functions that call spiSelect and spiUnselect)
  • modify the QMK SPI driver to allow for an inverted CS pin. Argument-wise this would look similar to the boolean for lsbFirst. Implementation-wise, it would just swap all the calls to spiSelect and spiUnselect (for ARM that is...AVR is similar except the SPI driver would swap calls to writePinLow and writePinHigh for the CS pin). This would still require to a modification of the QP init function arguments to add invertedCS (or something similar), but it offloads the implementation to the QMK SPI driver.
  • require that any hardware using this display type implement hardware logic inversion of the CS signal using either a logic level transistor (assuming it is fast enough...would need a reference implementation/chip that is known to work) or more likely a simple inverting buffer. There is precedent for this with things like ARM chips needing a 5V logic buffer to correctly talk to WS2812 RGB LEDs (or a different hacky hardware solution like a 5V pullup resistor). Ultimately, because it's the LS0XX display that violates the SPI standard by having an active high CS pin, it feels like this hardware-based approach may actually be the most reasonable.

@elpekenin
Copy link
Contributor Author

Yeah, i agree that the hardware solution is probably the best (and also easiest to handle) one.

Will try and handwire a SMD (the only type i have) transistor + use "regular" SPI code to confirm, but im pretty confident it will work just fine. Will report results once (if) i run a test 😃

@drashna drashna requested a review from a team October 22, 2023 09:49
@elpekenin
Copy link
Contributor Author

Now depends on #23699

Note: Added the vtable customization at QP's (not the device's) level because i remember having to do the same with eInk displays, so it will be ready for reusability.

However i dont quite like the nothing vs "inverted" naming scheme, ideas are welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting_pr Relies on another PR to be merged first core documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants