Implemented more functions for better compatibility with LVGL #61

expaso · 2024-03-28T16:16:00Z

No description provided.

landonr · 2024-04-06T02:59:13Z

Are these all LVGL related, or is some touch screen? do you have an example config?

expaso · 2024-04-07T10:51:32Z

Hi @***@***.***>, Sorry, I just saw I PRd the wrong way around. I was in the process of pulling in your changes to my fork of your repo. Basically I adapted your drivers for the display and the touchscreen of the Lilygo T-Display S3 to provide better compatibility with LGVL (I used it in this project VibroTactile Gloves against Parkinson's disease | Elektor Magazine<https://www.elektormagazine.com/labs/vibrotactile-gloves-against-parkinsons-disease>). The issue is more of an ESPHome issue actually. I basically needed 2 things: Drawing into own framebuffer, and flip it: LVGL is a library that is drawing directly in it's own allocated framebuffers. The ESPHome display model is more of a model that gives you methods to draw into a buffer allocated by the display driver. For performance reasons, I needed a way to let LVGL handle it's own framebuffer, and when it's time to show that framebuffer on the screen, hand that buffer over to your displaydriver to copy it to the LCD, while LVGL can do drawing of the next frame on a secondary framebuffer. This way I could use the multiple cores on the ESP32: The main thread lets LVGL do it's drawing, and a secondary thread (that runs on the other core) that copies over the framebuffer to the display. To make that work, I added a few settings to disable the internal framebuffer (because LVGL allocates it's own buffers), and to use ASYNC_IO (i.e. use an internal thread on a separate core to do the actual copying of the data so the main core is free for drawing again). Because only LVGL knows which portion of the screen actually needs updating, the updateArea function was added, so the driver does not have to copy over the entire frame each time. This is tremendous performance saver, leading to more then 110 FPS on this device. Note: You can even mix and match both. So, you could let LVGL do it's stuff, and write with the normal ESPHome drawLine and drawPixel functions on the screen at the same time (don't know why you would need this, but I left it intact). ESPHome does not have an API or interface for this kind of buffer-flipping of area-management Touchscreen: Because my device was battery powered, I added some extra means of telling the touchscreen driver IC to go to sleep, so it consumes less energy. The change to add IRAM_ATTR to the interrupt routine is needed to 'wake' the ESP on touchcreen events. Basically, the touchscreen chip is capable of going to a lower power state ('sleep') where it polls the display less frequently, but it still can recognize some built-in gestures in this mode, waking the ESP from sleep to handle this. And now the big but: I saw there were quite some changes going on around ESPHome display and displaybuffer handling lately. If you like, I could try to adapt all these changes and update this PR. Have you opted for integrating your code into regular ESPhome? This way, we could perhaps push dis ESPHome display model to be more inline with the scneario's above. I think it would be a huge benefit for multiple display's being able to do ASYNC drawing and/or delegate drawing to a specific library like LVGL. Let me know your thoughts! BTW, nice drivers! I enjoyed working on them! Kind regards, Hans

…

________________________________ Van: Landon Rohatensky ***@***.***> Verzonden: zaterdag 6 april 2024 04:59 Aan: landonr/lilygo-tdisplays3-esphome ***@***.***> CC: Hans van Essen ***@***.***>; Author ***@***.***> Onderwerp: Re: [landonr/lilygo-tdisplays3-esphome] Implemented more functions for better compatibility with LVGL (PR #61) Are these all LVGL related, or is some touch screen? do you have an example config? — Reply to this email directly, view it on GitHub<#61 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AGIIE7EPSO47JI2MQFB5YCTY35QJNAVCNFSM6AAAAABFNCPTW2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBQHEYDQMJXGI>. You are receiving this because you authored the thread.

landonr · 2024-04-07T12:17:05Z

Regular support for these displays should be in ESPHome, once I get a working config, I'll deprecate this repo #60

I still don't get how you're building LVGL, do you have a config I can try running?

expaso · 2024-04-07T15:52:35Z

Yeah sure!

I will create a small demo with a config for you this week, without too much clutter, it that allright?

clowrey · 2024-04-07T16:52:52Z

LVGL will be in mainline ESPhome soon 😁 I've tried it and works great! Just really need some PSRAM..
https://deploy-preview-3510--esphome.netlify.app/components/lvgl.html#:~:text=to%2016.-,buffer_size,-(Optional%2C%20percentage
https://gist.github.com/clydebarrow/ef89e9a93bd44771483b9144ae9042a1
https://gist.github.com/clydebarrow/9d2cffd739cb844e9d6d5005fd29518d

expaso · 2024-04-07T18:32:03Z

Yeah I checked out the lvgl component, and I use it in my project.

It's a nice start, but it performance was not good enough, due to the fact that the communication between the display-driver and the LVGL component was not optimal. This leads to sub-par FPS. Thats why I created the multi-threaded buffer handling, and richer interface between the driver and lvgl to optimize what exactly has to be drawn.

Edit: I am giving waaaay to less credit here, because the LVGL component as it has currently landedin ESPHome is a HUUGE step for UI. It wasn't there when I needed it back in november so I had to do all this stuff by myself.

And there were other factors too, like I created my whole interface in Squareline studio, and I wanted to export all those C-files to a folder, where ESPHome would pick them up, without modification.

This is what I did with it: https://youtu.be/RJwAdNKXpX8?si=0WGuvMw0ej2FJtB9&t=57

But I am more then happy to work with you guys to see if we squeeze the maximum awesomeness out of this combination.
Because LVGL with ESPHome and these touch displays works really really great!

Check: https://youtube.com/shorts/McAPOdtXWlw?feature=share

clowrey · 2024-04-07T22:59:10Z

@expaso Awesome!! I was just excited to share that we will have something better than the default older "lambda" graphics methods in ESPhome. What you have done looks amazing too!! And you're right the current implementation is not the fastest - it also basically NEEDS PSRAM - not sure if yours somehow can use less ram for buffers - I am basically just a user of these components and ESPhome not a developer myself, any and all improvements and options welcomed by me :)

landonr · 2024-04-08T12:07:54Z

I'm excited to try this out! This is a huge improvement to esphome!

Can you run clang format to get the lint to pass?

@guillempages do you want to take a quick look here?

guillempages · 2024-04-08T12:24:01Z

components/tdisplays3/display.py

@@ -78,6 +84,7 @@ async def to_code(config):
    cg.add_build_flag("-DTFT_D6=47")
    cg.add_build_flag("-DTFT_D7=48")

+    cg.add_build_flag("-DSPI_FREQUENCY=40000000")


Is this mandatory? Can't it be done in the standard esphome way (i.e. configuring SPI)?

Yeah perhaps it could. I just added it here because this whole component had all this settings defined here.

For me it worked without specifying the frequency; that's why I'm asking whether it is mandatory.

guillempages · 2024-04-08T12:27:06Z

components/tdisplays3/display.py

+    if CONF_USE_ASYNC_IO in config:
+        if config[CONF_USE_ASYNC_IO]:


This could be shortened:

Suggested change

if CONF_USE_ASYNC_IO in config:

if config[CONF_USE_ASYNC_IO]:

if config.get(CONF_USE_ASYNC_IO, False):

And actually, since a default is set, it shouldn't even be needed; you could just check if config[CONF_USE_ASYNC_IO]: (but it's safer to check both, of course)

Totally agree

guillempages · 2024-04-08T12:30:14Z

components/tdisplays3/t_display_s3.cpp

@@ -8,26 +8,132 @@ namespace tdisplays3 {

 static const char *const TAG = "TDisplayS3";

+#ifdef TDISPLAYS3_USE_ASYNC_IO
+namespace writer_shim {


Not wrong, but is the extra namespace really needed? And if it is, could you add the " // end namespace xyz comment at its end? ;-)

After looking at the whole implementation in the namespace, wouldn't this be better as a class instead of a namespace?
The whole variables are (namespace-)global variables, which feels kind of anti-pattern-ish

Yeah I though about that, but the writer-shim provides an interface between the class (the actual component), and the extra thread that it is running. The namespace is purely there to isolate the members of this namespace (clearification).

Another way would be to put this in a class, and pass the class reference as a parameter to the thread-start function.
I could rework towards this if you like.
On a second thought, it would give the added possibility of having multiple of these displays running on the same ESP.

I think you just answered yourself, that the class approach does have its advantages ;-)

components/tdisplays3/t_display_s3.cpp

guillempages · 2024-04-08T12:33:56Z

components/tdisplays3/t_display_s3.cpp

+    while (true) {
+      ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
+
+      if (x1 == 0 && y1 == 0 && x2 == 0 && y2 == 0) {


Wouldn't a better condition be
if ((x1 == 0 && x2 == 0) || (y1==0 && y2 == 0))?
i.e. what happens if one of the dimensions is empty, but the other is not? (Corner case, but is it fully impossible?)

Yes sir, your suggestion is the better implementation :)

guillempages · 2024-04-08T12:48:25Z

components/tdisplays3/touchscreen/t_display_s3_touchscreen.cpp

+  } else {
+    for (auto *listener : this->touch_listeners_)
+      listener->touch(tp);


Same here; is it worth it to update the code for old ESPHome versions?

guillempages · 2024-04-08T12:50:53Z

components/tdisplays3/touchscreen/__init__.py

@@ -34,6 +34,9 @@
            cv.Required(CONF_INTERRUPT_PIN): cv.All(
                pins.internal_gpio_input_pin_schema
            ),
+            cv.Required(CONF_RESET_PIN): cv.All(


Why does it need to be mandatory? If I understand it right, this is only needed for the sleep implementation, so why force people to add that if they don't need sleep?

Since the pin is hard-coded (hard-soldered actually) on the board; could this be set as "optional" with the actual PIN as a default?
(And now that I think about it, the same could be done for the interrupt one above)

components/tdisplays3/t_display_s3.cpp

guillempages · 2024-04-08T12:55:31Z

components/tdisplays3/t_display_s3.cpp

+  switch (this->get_rotation())
+  {
+    case esphome::display::DisplayRotation::DISPLAY_ROTATION_90_DEGREES:
+      this->tft_->setRotation(1);
+      break;
+    case esphome::display::DisplayRotation::DISPLAY_ROTATION_180_DEGREES:
+      this->tft_->setRotation(2);
+      break;
+    case esphome::display::DisplayRotation::DISPLAY_ROTATION_270_DEGREES:
+      this->tft_->setRotation(3);
+      break;    
+    default:
+      break;
+    }


IIRC, on a recent ESPHome version a "transform:" configuration option was added for some displays, where x and y can be swapped and/or mirrored, instead of rotated.
The general convention for displays is then, that "rotate" does SW rotation (as it was doing before), and "transform" does HW rotation.
Maybe worth looking into.

guillempages · 2024-04-08T12:56:09Z

components/tdisplays3/t_display_s3.cpp

+#ifdef TDISPLAYS3_USE_ASYNC_IO
+  ESP_LOGCONFIG(TAG, "Using Async IO");
+#else
+  ESP_LOGCONFIG(TAG, "NOT Using Async IO");


Suggested change

ESP_LOGCONFIG(TAG, "NOT Using Async IO");

ESP_LOGCONFIG(TAG, "Not using Async IO");

expaso · 2024-04-08T20:17:59Z

@guillempages , Thanks for the review! I really appreciate!

I have a question tough.. @landonr mentioned that this display chip is landing in mainstream ESPHome soon, and this repo would be deprecated.

Is this PR worth it's value then?

Also in my opinion, from an architectural perspective maybe we should seek to implement the ASYNC_IO functionality on a lower level, like the ESPHome's Display class, because a lot of display drivers inheriting from that could benefit from this functionality.

Another reason is that we need class-interface changes to play nicely with LVGL.
What LVGL wants is:

a) Allocate it's own buffers. LVGL has very good reasons for this. The buffers could be single, or double, don't need to be the entire framesize perse, and prefer special capabilities.
This means that the display driver does NOT need to allocate it's own buffers.
Here you see this for double buffers on my ESP32:

       auto *buf1 = (lv_color_t *)heap_caps_malloc(sizeof(lv_color_t) * len, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
       auto *buf2 = (lv_color_t *)heap_caps_malloc(sizeof(lv_color_t) * len, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);

As you see, these are no ordinary mallocs. The MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT capabilities prefer FAST Internal RAM, instead of PSRAM. (they will fallback to PSRAM tough if not enough internal RAM), because of FPS performance.
Let the slow PSRam be used for other non-critical stuff.

b) Being able to hand over a buffer, and indicate what exact portion of that buffer needs to be updated to the screen (updateArea). Perhaps only a small portion of the framebuffer was actually updated by LVGL, so only those pixels need to be copied over to the actual screen.

c) A callback to know when the display-driver is done flushing the freshly handed-over framebuffer to the screen, so the next frame can be presented.

Perhaps we should seek to consult JesseRocks and find out what his roadmap for disdplay-drivers is like? Maybe we can work towards more standardization.

What is your view on this?

guillempages · 2024-04-08T22:05:26Z

@guillempages , Thanks for the review! I really appreciate!

I have a question tough.. @landonr mentioned that this display chip is landing in mainstream ESPHome soon, and this repo would be deprecated.

Is this PR worth it's value then?

Well, I didn't comment it before, but this PR is actually doing two (or actually three) things: It's improving the performance for the display rendering, it's allowing the sleep mode for the display and it's allowing the sleep mode for the touchscreen.

I'd say the touchscreen changes are not worth it, since there is already an official implementation in upstream of the CST chip used. (Unless you are adding some functionality not present in ESPHome, and don't want to add it there ;-) )

If I am not mistaken, the implementation for the display driver that is done upstream is for the OLED version of the t-display-S3; which is pretty different than this one. That said, I haven't really checked in detail; it might be, that this one also already works there.

Also in my opinion, from an architectural perspective maybe we should seek to implement the ASYNC_IO functionality on a lower level, like the ESPHome's Display class, because a lot of display drivers inheriting from that could benefit from this functionality.

This would benefit other displays as well, but thenm, it would need to be done on the ESPHome repo ;-)

Another reason is that we need class-interface changes to play nicely with LVGL. What LVGL wants is:

a) Allocate it's own buffers. LVGL has very good reasons for this. The buffers could be single, or double, don't need to be the entire framesize perse, and prefer special capabilities. This means that the display driver does NOT need to allocate it's own buffers. Here you see this for double buffers on my ESP32:
       auto *buf1 = (lv_color_t *)heap_caps_malloc(sizeof(lv_color_t) * len, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
       auto *buf2 = (lv_color_t *)heap_caps_malloc(sizeof(lv_color_t) * len, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
As you see, these are no ordinary mallocs. The MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT capabilities prefer FAST Internal RAM, instead of PSRAM. (they will fallback to PSRAM tough if not enough internal RAM), because of FPS performance. Let the slow PSRam be used for other non-critical stuff.

b) Being able to hand over a buffer, and indicate what exact portion of that buffer needs to be updated to the screen (updateArea). Perhaps only a small portion of the framebuffer was actually updated by LVGL, so only those pixels need to be copied over to the actual screen.

c) A callback to know when the display-driver is done flushing the freshly handed-over framebuffer to the screen, so the next frame can be presented.

Perhaps we should seek to consult JesseRocks and find out what his roadmap for disdplay-drivers is like? Maybe we can work towards more standardization.

What is your view on this?

There are currently some efforts done by Clydebarrow to improve the performance of some display drivers, and to add LVGL support. Jesse would welcome any performance improvements on ESPHome, AFAIK, I'm not sure that there is an explicit roadmap (I don't think so). Clyde and NielsNL have some ideas on how to proceed on a display front; you might want to join the discussions on discord: https://discord.com/channels/429907082951524364/1119046373543780363

This repository initially had just a description on how to use TFT_eSPI for this display in combination with ESPHome, but without the whole display implementation; i.e. just allowing C++ drawing. At some point, I reworked the code a bit to be able to use it as a standard ESPHome display, but did not focus (much) on optimization; just on having it work without much hassle.
The repo as-is (i.e. using TFT_eSPI) will most probably never get merged into ESPHome; since the project frowns upon adding (big) external libraries unless really needed.
If you have ideas on improving the performance here (as in this PR), you are welcome to submit them, and we will review them, as long as they do not break standard ESPHome functionality.

landonr · 2024-04-09T02:38:56Z

@clydebarrow can you chime in? Are you working on mainlien support for TDisplay S3 Amoled? It'd like to make devices pages for TDisplay S3 (tft and amoled)

clydebarrow · 2024-04-09T03:31:56Z

@clydebarrow can you chime in? Are you working on mainlien support for TDisplay S3 Amoled? It'd like to make devices pages for TDisplay S3 (tft and amoled)

T-Display S3 AMOLED (quad-SPI) is already supported in ESPHome along with the T4-S3. TFT T-Display S3 (which I think is an i8080 interface) is not yet supported, but it's a WIP.

h3ndrik · 2024-09-28T17:46:21Z

This doesn't seem to compile (anymore):

[...]
Compiling .pioenvs/boombox/src/esphome/components/tdisplays3/t_display_s3.cpp.o
In file included from src/esphome/components/tdisplays3/t_display_s3.cpp:1:
src/esphome/components/tdisplays3/t_display_s3.h:31:8: error: 'void esphome::tdisplays3::TDisplayS3::updateArea(int16_t, int16_t, int16_t, int16_t, uint16_t*, void (*)())' marked 'override', but does not override
   void updateArea(int16_t x1, int16_t y1, int16_t x2, int16_t y2, uint16_t *buffer, void (*ready_callback)(void)) override;
        ^~~~~~~~~~
*** [.pioenvs/boombox/src/esphome/components/tdisplays3/t_display_s3.cpp.o] Error 1
========================= [FAILED] Took 164.49 seconds =========================

(And unfortunately, seems support for the displays with a parallel bus hasn't made it into ESPHome. I don't see any current work on that.)

clydebarrow · 2024-09-28T21:12:16Z

support for the displays with a parallel bus hasn't made it into ESPHome. I don't see any current work on that.)

It's available as an external component. I closed the PR for reasons I won't discuss here.

expaso · 2024-09-30T12:42:41Z

Yeah, I saw that display-code was really in flux last versions of ESPHome, breaking this PR, and was not going into the direction I need to keep the performance optimal when used with LVGL.

@clydebarrow You can indeed abandon this PR. I have pulled everything I need to external components myself until ESPHome has a more mature display driver architecture.

h3ndrik · 2024-09-30T14:57:11Z

Hmm. Bummer. esphome/esphome#6537 seems to require ESP_IDF, and I need the Arduino platform for a media_player, which has already been discussed in some of the issues/PRs. And I mean it's bound to be out of date at some point, too...

@clydebarrow You mentioned seperating out the PR in the esphome repo, put in some effort and then closed it on Aug 7. I suppose you're not working on that anymore? Are there things to know about when someone else attempts to get that running or incorporated into esphome? Any technical barriers / personal matters? Or were you just unable to put in the effort to clean it up and get it accepted in esphome after all of the lvgl stuff? I mean you already said you won't discuss it here. So feel free not to answer. I'd just like not to waste too much time myself and run into the same obstacles.

Hans van Essen and others added 11 commits November 12, 2023 21:38

Worked on display drivers.

4ed1544

Worked on Display Drivers with Touchscreen Support

d5e05f0

Worked on performance

f69199d

Fixed multithreading

417549e

Implemented Async IO

f79f1f5

Finished driver

2c02e54

Worked on display

aad5418

Implemented Sleep

916cda1

Implemented poweron/poweroff

6332039

Added reset pin / shutdown

6279ac1

Merge branch 'main' into IntegratorAfterMerge

b4566f1

expaso changed the title ~~Integrated changes from origin~~ Implemented more functions for better compatibility with LVGL Mar 28, 2024

guillempages reviewed Apr 8, 2024

View reviewed changes

expaso closed this Sep 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented more functions for better compatibility with LVGL #61

Implemented more functions for better compatibility with LVGL #61

expaso commented Mar 28, 2024

landonr commented Apr 6, 2024

expaso commented Apr 7, 2024 via email

landonr commented Apr 7, 2024

expaso commented Apr 7, 2024

clowrey commented Apr 7, 2024 •

edited

Loading

expaso commented Apr 7, 2024 •

edited

Loading

clowrey commented Apr 7, 2024

landonr commented Apr 8, 2024

guillempages Apr 8, 2024

expaso Apr 8, 2024

guillempages Apr 8, 2024

guillempages Apr 8, 2024

guillempages Apr 8, 2024

expaso Apr 8, 2024

guillempages Apr 8, 2024

guillempages Apr 8, 2024

expaso Apr 8, 2024

guillempages Apr 8, 2024

guillempages Apr 8, 2024

expaso Apr 8, 2024

guillempages Apr 8, 2024

guillempages Apr 8, 2024

guillempages Apr 8, 2024

guillempages Apr 8, 2024

expaso commented Apr 8, 2024

guillempages commented Apr 8, 2024

landonr commented Apr 9, 2024

clydebarrow commented Apr 9, 2024

h3ndrik commented Sep 28, 2024 •

edited

Loading

clydebarrow commented Sep 28, 2024

expaso commented Sep 30, 2024

h3ndrik commented Sep 30, 2024 •

edited

Loading

		if CONF_USE_ASYNC_IO in config:
		if config[CONF_USE_ASYNC_IO]:

	if CONF_USE_ASYNC_IO in config:
	if config[CONF_USE_ASYNC_IO]:
	if config.get(CONF_USE_ASYNC_IO, False):

	ESP_LOGCONFIG(TAG, "NOT Using Async IO");
	ESP_LOGCONFIG(TAG, "Not using Async IO");

Implemented more functions for better compatibility with LVGL #61

Implemented more functions for better compatibility with LVGL #61

Conversation

expaso commented Mar 28, 2024

landonr commented Apr 6, 2024

expaso commented Apr 7, 2024 via email

landonr commented Apr 7, 2024

expaso commented Apr 7, 2024

clowrey commented Apr 7, 2024 • edited Loading

expaso commented Apr 7, 2024 • edited Loading

clowrey commented Apr 7, 2024

landonr commented Apr 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

expaso commented Apr 8, 2024

guillempages commented Apr 8, 2024

landonr commented Apr 9, 2024

clydebarrow commented Apr 9, 2024

h3ndrik commented Sep 28, 2024 • edited Loading

clydebarrow commented Sep 28, 2024

expaso commented Sep 30, 2024

h3ndrik commented Sep 30, 2024 • edited Loading

clowrey commented Apr 7, 2024 •

edited

Loading

expaso commented Apr 7, 2024 •

edited

Loading

h3ndrik commented Sep 28, 2024 •

edited

Loading

h3ndrik commented Sep 30, 2024 •

edited

Loading