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

Dotclockframebuffer #8351

Merged
merged 20 commits into from
Sep 7, 2023
Merged

Dotclockframebuffer #8351

merged 20 commits into from
Sep 7, 2023

Conversation

jepler
Copy link
Member

@jepler jepler commented Aug 30, 2023

This requires a change in our fork of esp-idf:

My test platform is https://www.tindie.com/products/makerfabs/matouch-esp32-s3-parallel-tft-with-touch-7/ [800x480 model]

PXL_20230830_153557908 MP

The display is unstable (horizontal tearing) at higher dot clocks, and on my test screen the low refresh rate of ~12Hz is a bit flickery. We think that this is due to sub-optimal configuration of PSRAM which may be fixed/improved by #6913
(see Lzw655/ESP32_Display_Panel#4 (comment)) -- this kind of problem is apparently prevalent in esp-idf software and some other suggestions are here: https://docs.espressif.com/projects/esp-faq/en/latest/software-framework/peripherals/lcd.html#why-do-i-get-drift-overall-drift-of-the-display-when-driving-an-rgb-lcd-screen

The display will glitch during CIRCUITPY writes or other things that hold the internal flash/spiram bus in use.

May not all be a part of this PR but here are some TODOs:

  • pixel start may be nonzero & framebuffer width may not match visible width (e.g., one display is 300 wide but must be initialized with a visible width of 400), so add a rowstride property & make it work
  • initialization of some displays uses SPI; sometimes SPI bus is on an I2C I/O expander; can bitbang this in python code to begin with.
  • check if psram is really being set up in octal mode [sdkconfig has CONFIG_SPIRAM_MODE_OCT=y]
  • see if psram performance can be boosted without IDF change [sdkconfig has CONFIG_SPIRAM_SPEED_80M=y and _120M for octal RAM doesn't seem to be in idf4 and may require core speed to be 240MHz]

@kmatch98 you may be interested
@ladyada it's on

jepler added 7 commits August 30, 2023 10:09
this is working, though it has to be down-clocked to 6.5MHz to prevent
display glitching
This is not working/tested. The display requires an initialization sequence,
and its SPI bus is on the other side of an I2C GPIO expander making things
more difficult.
@ladyada
Copy link
Member

ladyada commented Aug 30, 2023

its on like donkey kong!

jepler added 2 commits August 30, 2023 11:34
There will be a revision but get the basics in for now.

This successfully displays on a TL040HDS20-B1502A screen with:
```
import board
from framebufferio import FramebufferDisplay
from dotclockframebuffer import DotClockFramebuffer
from displayio import release_displays

tft_pins = board.TFT
tft_timings = {
    "frequency": 6_500_000,
    "width": 720,
    "height": 720,
    "hsync_pulse_width": 20,
    "hsync_front_porch": 40,
    "hsync_back_porch": 40,
    "vsync_pulse_width": 10,
    "vsync_front_porch": 40,
    "vsync_back_porch": 40,
    "hsync_idle_low": False,
    "vsync_idle_low": False,
    "de_idle_high": False,
    "pclk_active_high": False,
    "pclk_idle_high": False,
}

release_displays()
fb = DotClockFramebuffer(**tft_pins, **tft_timings)
disp = FramebufferDisplay(fb)
```
@tannewt
Copy link
Member

tannewt commented Aug 30, 2023

This requires a change in our fork of esp-idf:

Instead of changing the IDF could we enable CONFIG_SPIRAM_USE_CAPS_ALLOC? I'm not sure how many other modules will start trying to use PSRAM then but maybe that's ok. We don't really need it all for CP.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Just a few minor things. Thanks for getting this going!

@tannewt
Copy link
Member

tannewt commented Aug 30, 2023

Another thing to check after IDF 5 merge is SPI flash settings since they are on a shared set of pins. The longer the flash reads take, the longer the PSRAM transactions will have to wait. I'm reworking how these settings are done in IDF 5.

@jepler
Copy link
Member Author

jepler commented Aug 30, 2023

Instead of changing the IDF could we enable CONFIG_SPIRAM_USE_CAPS_ALLOC? I'm not sure how many other modules will start trying to use PSRAM then but maybe that's ok. We don't really need it all for CP.

I was reluctant to do this because it looked like a big change (we like to just manage the PSRAM ourselves so we always have a nice big contiguous heap). As far as I understand it, we have to change that if we change this setting. If I'm wrong about that then I'd be happy to change it.

@jepler
Copy link
Member Author

jepler commented Aug 31, 2023

Code for TL021WVC02 480px round display & Adafruit TFT Experiment Rev A https://gist.github.com/jepler/71e9ae8b1e45616fa75988d1139140c1

@jepler jepler requested a review from tannewt August 31, 2023 19:54
@RetiredWizard
Copy link

RetiredWizard commented Sep 2, 2023

I've got @kmatch98's Hack Tablet working with this PR. The bouncing ball block demo seems to work fine but the only other thing I've tested so far is the animated Homer Simpson gif which works but is very small on this display 😁. I'll keep playing and see what I can do, but let me know if there's anything in particular you want tested.

@kmatch98
Copy link
Collaborator

kmatch98 commented Sep 2, 2023

I haven't build CircuitPython in a while. Is there a way to collect the .bin artifacts for the Makerfabs 7" build or the HACKtablet?

I actually had to go digging to find mine.

IMG_8696

@RetiredWizard
Copy link

Is there a way to collect the .bin artifacts for the Makerfabs 7" build or the HACKtablet?

The HACKtablet artifact isn't being built by Jeff's PR yet but I've attached the one I've built. I don't see the Makerfabs artifact, I think you normally find the artifacts by clicking on the "checks" tab on this page and then select "Build CI", the artifacts would be at the very bottom but I only see adafruit_esp32s3_rgb_tft_experiment for some reason.

HACK Tablet firmware.zip

@RetiredWizard
Copy link

RetiredWizard commented Sep 2, 2023

Keep in mind that the latest build from main isn't compatible with .mpy files so you need to grab .py versions of any libraries you want to use.

Here's the code I'm using to initialize the display.

import displayio
import framebufferio
import dotclockframebuffer
import board

displayio.release_displays()

fb=dotclockframebuffer.DotClockFramebuffer(**board.TFT,**board.TIMINGS800)
display=framebufferio.FramebufferDisplay(fb)

I initially had the CIRCUITPY_RESERVED_PSRAM variable in settings.toml set to 1024000 but the board would crash when trying to re-initalize the display. I increased it to 4098000 which solved the issue but I'm pretty sure 4 Meg is overkill. But with 8 Meg of RAM I figured I could worry about that later 😁

@RetiredWizard
Copy link

Oops, sorry, you said .bin artifact...
HACK Tablet firmware.bin.zip

@jepler
Copy link
Member Author

jepler commented Sep 3, 2023

There's no board definition for the hack tablet in this branch and because of the way the CI works now (tries to build as few boards as possible) you have to go through a few hoops to find the makerfabs one.

I think it'll be: makerfabs_tft7

but basically you go back through the green check marks until you find a build that has your board in it, then go to that link, then click summary, and finally you can scroll down and find the artifact. It's .. pretty bad, but that's github's UI in a nutshell.

@kmatch98
Copy link
Collaborator

kmatch98 commented Sep 3, 2023

Thanks jepler and RetiredWizard. I got the makerfabs display working with the binary and the awesome ESP webserial uploader.

Agreed on the difficulty to find GitHub artifacts, I clicked and clicked but didn't know the secret pathway to find what I was looking for.

Special thanks RetiredWizard for the setup instructions for the display, these worked perfectly on the makerfabs binary too.

IMG_8699

With DEBUG build and a consle UART, this would occur early during startup on makerfabs tft7:
```
I (0) cpu_start: Starting scheduler on APP CPU.
I (10) uart: queue free spaces: 20

***ERROR*** A stack overflow in task uart_event_task has been detected.


Backtrace: 0x403786f2:0x3fce9f40 0x403820a9:0x3fce9f60 0x403850da:0x3fce9f80 0x40383a7d:0x3fcea000 0x40382158:0x3fcea030 0x4038214e:0xa5a5a5a5 |<-CORRUPTED
```

Decoded backtrace was not enlightening:
```
0x403786f2: panic_abort at /home/jepler/src/circuitpython/ports/espressif/build-makerfabs_tft7/esp-idf/../../esp-idf/components/esp_system/panic.c:408
0x403820a9: esp_system_abort at /home/jepler/src/circuitpython/ports/espressif/build-makerfabs_tft7/esp-idf/../../esp-idf/components/esp_system/esp_system.c:137
0x403850da: vApplicationStackOverflowHook at /home/jepler/src/circuitpython/ports/espressif/build-makerfabs_tft7/esp-idf/../../esp-idf/components/freertos/port/xtensa/port.c:407
0x40383a7d: vTaskSwitchContext at /home/jepler/src/circuitpython/ports/espressif/build-makerfabs_tft7/esp-idf/../../esp-idf/components/freertos/tasks.c:3505
0x40382158: _frxt_dispatch at /home/jepler/src/circuitpython/ports/espressif/build-makerfabs_tft7/esp-idf/../../esp-idf/components/freertos/port/xtensa/portasm.S:436
0x4038214e: _frxt_int_exit at /home/jepler/src/circuitpython/ports/espressif/build-makerfabs_tft7/esp-idf/../../esp-idf/components/freertos/port/xtensa/portasm.S:231
0x00000000: ?? ??:0
```

Adding an additional 512 bytes of stack allowed CircuitPython to start
successfully.
.. and switch makerfabs tft7 over to it as a test.

We have our existing way of "reserving" PSRAM for esp-idf (we actually
control it all but add back the "reserved" part). However, this does
not work with off the shelf esp_lcd, which only will allocate a
framebuffer in PSRAM if CONFIG_SPIRAM_USE_CAPS_ALLOC (or CONFIG_SPIRAM_USE_ALLOC)
is defined, not if CONFIG_SPIRAM_USE_MEMMAP is.

This new way is possibly compatible with more esp-idf code, but it complicates
CircuitPython's initial startup since nothing until port_heap_init is
permitted to use the CP heap or supervisor allocator. In practice this
seems to be OK today.

Right now this doesn't change the setting across all boards with PSRAM and so
it does not revert esp-idf to its prior state. Instead, what I'm thinking is
that we can do it during or just after the IDF5 update when sdkconfig files
will be getting an overhaul anyway.
@jepler
Copy link
Member Author

jepler commented Sep 5, 2023

@RetiredWizard I'd be happy to add the hack tablet as a board as part of this PR!

@tannewt I think I made SPIRAM_USE_CAPS_ALLOC work, please review again! However, I didn't change all sdkconfig files to use it, I figured we could do that when we update idf. Let me know if you concur.

@RetiredWizard
Copy link

I'd like to make a few more changes to the pins file (for consideration anyway) and then I'll attach the board files to this PR 😁

Thanks!

@RetiredWizard
Copy link

Here are the board files I'm using for the Hacktablet...
espressif_esp32s3_devkitc_1_n8r8_hacktablet.zip

@tannewt
Copy link
Member

tannewt commented Sep 5, 2023

Looks like there are compilation issues still.

@kmatch98
Copy link
Collaborator

kmatch98 commented Sep 5, 2023

Jepler, just a few things I remember were issues specific to the S3. Perhaps you have resolved them.

  • I got tearing because the ESP-IDF is set to automatically trigger a refresh from some internal timer. I had to turn off the IDF’s auto refresh and created a function where I could allow CP to call for refreshes. This may have been include in v5.0 but I have not verified. That added another issue where typically CP (expecting the display to have its own memory) doesn’t call for a refresh unless something changes. I know foamyguy covered a resolution of this in a stream.

  • there are PSRAM access collisions that degrade the refresh rate or downright cause glitches when CP and the LCD DMA both want to access PSRAM bandwidth and the LCD gets starved. The ESP-IDF has a “Bounce buffer” to essentially tie up the processor to avoid this. Maybe there is a better way in CP to avoid and give the LCD preference but I assume there are trade offs.

I don’t think either of these are killers but if not resolved will likely limit the refresh rates. In the end performance will likely be limited to some combination of PSRAM writes by CP and the LCD PSRAM bandwidth required to meet a certain refresh rate.

My apologies if you already addressed these but wanted to leave a paper trail in case someone bumps into a limit while trying to push this driver to its limit. Also I haven’t followed the latest IdF updates so maybe the Espressif folks have found some new tricks.

I’m excited to see what folks will do with all these cool displays!

these are conditionally-defined identifiers so have to use if defined.
@jepler
Copy link
Member Author

jepler commented Sep 6, 2023

Yes, almost anytime there's contention for the access to the SPI bus the display will shift and tear, but, it self heals. we're hoping IDF 5 will improve things, and will fine tune then.

I didn't encounter the problem you're mentioning with the lack of automatic refresh. the "relax_on_idle" flag does need to be set to false to get automatic refresh, but that seemed to be all that was necessary.

@jepler
Copy link
Member Author

jepler commented Sep 6, 2023

@tannewt CI is green now. I'd like to leave the refresh argument, because it matches rgbmatrix, will be helpful if/when we add double buffering, and doesn't affect displayio use (displayio calls the equivalent internally).

tannewt
tannewt previously requested changes Sep 6, 2023
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thanks for getting the CI green. Just a couple other minor things.

main.c Outdated Show resolved Hide resolved
ports/espressif/esp-idf Outdated Show resolved Hide resolved
shared-bindings/dotclockframebuffer/DotClockFramebuffer.c Outdated Show resolved Hide resolved
@RetiredWizard
Copy link

When you take a look at the HackTablet board files......

I've reduced and tested the DEFAULT_RESERVED_PSRAM setting as follows:

// a 800x480 16BPP framebuffer + some breathing room
#define DEFAULT_RESERVED_PSRAM      (800 * 800 * 2)

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

This looks OK to me, though of course I don't know some details. But it's consistent and I understand the related changes. Thanks!

@jepler jepler requested a review from tannewt September 7, 2023 19:02
@jepler jepler dismissed tannewt’s stale review September 7, 2023 19:02

done, thanks!

@ladyada
Copy link
Member

ladyada commented Sep 7, 2023

🥳

@tannewt tannewt merged commit 0928a95 into adafruit:main Sep 7, 2023
447 checks passed
@bwhitman
Copy link

I saw the note about this work in the circuitpython newsletter. Just wanted to offer any help on performance of this, we've spent a LOT of time / code getting the same thing going in Micropython for TulipCC.

We're using ESP5.1-rc2 and running the SPIRAM at 120MHz. We're getting pretty stable 30FPS on full screen refreshes on a 1024x600 RGB dot clock display. We use the bounce buffers reading from a SPIRAM framebuffer that's bigger than the screen size (for scrolling effects.) Most of the ESP32S3 code is here https://github.com/bwhitman/tulipcc/blob/main/tulip/esp32s3/esp32s3_display.c and the platform-independent display code (writing text, sprites, scrolling etc) is here https://github.com/bwhitman/tulipcc/blob/main/tulip/shared/display.c

I'd be delighted to merge whatever we've done into Circuitpython or whatever makes sense for everyone, I'm not fully up to speed on the differences between Micropython and Circuitpython.

@ladyada
Copy link
Member

ladyada commented Sep 12, 2023

@bwhitman hihi thanks - we've got the proof-of-concept going, and will probably focus more on automation over the next bit. the text/sprites stuff is already managed by 'displayio' and the REPL-on-display is built in as well as the mass storage filesystem. it may be a fairly easy 'port' if you want to try adapting it to circuitpy

@tannewt
Copy link
Member

tannewt commented Sep 12, 2023

@bwhitman Thanks! Do you have any mapping between espressif modules and flash and ram settings? We haven't optimized it in CP and (I suspect) have a lot to gain from it. We have 131 boards so it's tricky to know the best settings for each.

@bwhitman
Copy link

@tannewt Two places to look: (1) for DDR 120MHz PSRAM you need "octal flash", which are the modules that say OT in this chart
https://docs.espressif.com/projects/esp-idf/en/latest/esp32s3/hw-reference/esp32s3/user-guide-devkitc-1.html#ordering-information

(2) You want to see this list of "groups" of settings: https://docs.espressif.com/projects/esp-idf/en/latest/esp32s3/api-guides/flash_psram_config.html#f8r8-hardware

Note that the N8R8 in (1) is OT PSRAM but "only" quad flash -- while this other doc indicates it should support 120MHz DDR RAM, I have not been able to get it going as you need to also select octal flash to indicate 120MHz ram. I think that there is a forthcoming or otherwise new rev of the N8R8 that supports OT PSRAM and flash. The N8R8s I have were purchased from Adafruit earlier this year and do no support OT flash.

So as far as I know, the only modules that will support the fastest RAM settings are ESP32-S3-WROOM-2-N32R8V and ESP32-S3-WROOM-2-N16R8V, the forthcoming or otherwise rev2 of the N8R8, and then all other R8s "only" support 80.

For actual sdkconfig settings, see https://github.com/bwhitman/tulipcc/blob/main/tulip/esp32s3/boards/TULIP4_N32R8/sdkconfig.board for the fast PSRAM speed and https://github.com/bwhitman/tulipcc/blob/main/tulip/esp32s3/boards/TULIP4_N8R8/sdkconfig.board for the "slow" one. We see a real improvement of 20% or so of "GPU" type access times using the faster speed. But it requires ESP-IDF 5.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants