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

Support for ESP32-S3 peripherals #605

Closed
shlomozippel opened this issue Jan 3, 2023 · 14 comments
Closed

Support for ESP32-S3 peripherals #605

shlomozippel opened this issue Jan 3, 2023 · 14 comments
Labels
enhancement pending Pending new release; already merged but not released in a version

Comments

@shlomozippel
Copy link
Contributor

There is already support for RMT and SPI on ESP32-S3 and it would be great to take it a few steps further:

  1. There are 2 GP-SPI peripherals, SPI2 has support for 8 bit transfer and SPI3 has support for 4 bit. Currently DotStarEsp32DmaSpiMethod ESP32-S3 support includes SPI2 with up to 4 bit transfers, and doesn't support SPI3
  2. The new LCD peripheral supports 8-16 bit transfers. It seems like the I2S peripheral isn't used for this scenario anymore like it was in previous ESP32 chips and now only supports 1 data line. Example of using this new peripheral: https://blog.adafruit.com/2022/06/14/esp32uesday-hacking-the-esp32-s3-lcd-peripheral/

Related to #598 and #559

@Makuna Makuna added enhancement future consideration This issue is tracking a feature for consideration in any future major work. labels Jan 3, 2023
@Makuna
Copy link
Owner

Makuna commented Jan 3, 2023

Thanks for suggestion and pointers.

@softhack007
Copy link
Contributor

softhack007 commented Jun 29, 2023

If I understand the adafruit infos correctly, the LCD driver unit is basically an evolution of the esp32 "secret I2S feature" to drive 8x/16x parallel outputs? So it might be possible to get up to 16 parallel led outputs on -S3 from the lcd unit?

@Makuna
Copy link
Owner

Makuna commented Jun 29, 2023

@softhack007 Yes, this is my understanding. BUT it seems they wrapped the details into a driver model that only exposes LCD API rather than a data API. When I get bits of time I am still investigating, and it is helpful if others come upon details to link them here for me to review. Searching seems to be like finding a needle in a haystack.

@coliniuliano
Copy link
Contributor

@Makuna I'd like to dig into this, based on the parallel LCD support for FastLED here https://github.com/hpwit/I2SClockLessLedDriveresp32s3

Do you have any advice on how you would approach this?

@Makuna Makuna added investigating Currently under investigation for more understanding of the problem. and removed future consideration This issue is tracking a feature for consideration in any future major work. labels May 8, 2024
@shlomozippel
Copy link
Contributor Author

shlomozippel commented May 8, 2024

@Makuna I'd like to dig into this, based on the parallel LCD support for FastLED here https://github.com/hpwit/I2SClockLessLedDriveresp32s3

Do you have any advice on how you would approach this?

That example implementation is a good reference, and I found the adafruit one to be a bit more readable: https://github.com/adafruit/Adafruit_NeoPXL8/blob/master/Adafruit_NeoPXL8.cpp (Note that there are multiple implementations there for multiple microcontrollers).

I wrote a driver using the adafruit code as reference for the LCD peripheral, FastLED code for the transpose logic, and custom color order and gamma tables: https://github.com/chroma-tech/micropython/blob/fern/ports/esp32/usermodules/modcanopy/driver.h

(You wouldn't need to deal with the color order and gamma stuff if porting to NPB)

@Makuna
Copy link
Owner

Makuna commented May 8, 2024

@Makuna I'd like to dig into this, based on the parallel LCD support for FastLED here https://github.com/hpwit/I2SClockLessLedDriveresp32s3

Do you have any advice on how you would approach this?

Review the CoreShaderBeta or work within that branch, as that is going to be next major release and some of this has changed.
Copy this file, https://github.com/Makuna/NeoPixelBus/blob/master/src/internal/methods/NeoEsp32I2sXMethod.h
into a NeoEsp32LcdXMethod.h.
Remove I2S low level code calls and Replace with LCD calls.
Change any !defined(CONFIG_IDF_TARGET_ESP32S3) or C3 and remove the not (!) so it's only for those two.
Rename all i2s to lcd keeping the capitalization present.
Not sure there is more than one LCD peripheral, so may need to remove the i2s0 and i2s1 support if there is only one.
Make sure to support x8 and x16 if possible if it uses less DMA memory. Expand to x24 if needed as the memory consumption is ridiculous and it rarely is needed, while x8 is the greatest priority, then x16.

NPB does color order, luminance, and gamma at a higher level, so the methods are just about getting the bits out.

@coliniuliano
Copy link
Contributor

I've managed to implement code for my specific use-case: S3 using the LCD peripheral to drive 8 parallel WS2812x strips with WLED.
PR is here in case it's useful to anyone: #808

@Makuna
Copy link
Owner

Makuna commented Jun 4, 2024

@coliniuliano Thanks! I will take a look at the PR as soon as I can, but don't expect it this week.

@Makuna
Copy link
Owner

Makuna commented Jun 22, 2024

@coliniuliano Good work on the PR. Let me know if you can (willing) to apply the requested changes. Alternately I can provide a branch (for the changes to master) that you can provide the pull to, and I will merge and fixup before merging into main.
I will already have to do some work as you stated you were only doing the WS2812x and the others will be needed.

@Makuna Makuna added active development This issue is a primary item I am actively working on. Interested testers add a comment. and removed investigating Currently under investigation for more understanding of the problem. labels Jun 22, 2024
@Makuna
Copy link
Owner

Makuna commented Aug 4, 2024

Just an update. There is a branch that takes Colin changes but there are some things that still need to be cleaned up.

  • There are some optimizations I already have made but not checked in yet. After a few tests this will get merged into that branch (Esp32S3Led)
  • It needs to support more than just WS2812x chip speeds and reset. This will require some investigation into the LCD speed properties, but they seem similar to the i2s.
  • Testing, lots of testing.
    Once these are done, the branch will get merged into master.

@Makuna
Copy link
Owner

Makuna commented Aug 18, 2024

Optimizations checked into the Esp32S3Led branch.
Next is other chip speeds.

This current work is based on a 3 step cadence (1/3, 2/3 pulse ratio timing). While this uses less memory, it also slows down processing time since it requires byte level access to data. The I2s Parallel uses a 4 step cadence (1/4, 3/4 pulse ratio timing) so everything is word aligned, and native element size is used to copy/clear (faster). I am leaving this as a 3 step as memory seems to be a bigger problem with parallel than speed. It does raise another memory optimization option that could be done to the i2s parallel.

@Makuna Makuna added pending Pending new release; already merged but not released in a version and removed active development This issue is a primary item I am actively working on. Interested testers add a comment. labels Aug 18, 2024
@Makuna
Copy link
Owner

Makuna commented Aug 18, 2024

#824

Merged in, with example.

@Makuna Makuna pinned this issue Aug 26, 2024
@Makuna
Copy link
Owner

Makuna commented Sep 2, 2024

https://github.com/Makuna/NeoPixelBus/releases/tag/2.8.1

@Makuna Makuna closed this as completed Sep 2, 2024
@Makuna Makuna unpinned this issue Sep 2, 2024
@coliniuliano
Copy link
Contributor

@Makuna Sorry for leaving this half finished and thank you so much for driving it home!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pending Pending new release; already merged but not released in a version
Projects
None yet
Development

No branches or pull requests

4 participants