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

Port to LVGL v7.0 #101

Closed
3 tasks done
C47D opened this issue Apr 21, 2020 · 130 comments
Closed
3 tasks done

Port to LVGL v7.0 #101

C47D opened this issue Apr 21, 2020 · 130 comments

Comments

@C47D
Copy link
Collaborator

C47D commented Apr 21, 2020

List of necessary changes:

  • Update lvgl submodule.
  • Update lvgl example submodule.
  • Update lvgl configuration file.
@kisvegabor
Copy link
Member

Update lvgl submodule.

It's in dev-7.0 branch

Update lvgl example submodule.

It's in rework-7 branch

Update lvgl configuration file.

It'd be best to create a new one based on lv_conf_templ.h in dev-7.0.

Could you make a build test to see if there are any serious issues?

@C47D
Copy link
Collaborator Author

C47D commented Apr 22, 2020

I will do the tests here: https://github.com/C47D/lv_port_esp32_v7
Once we get it working I will update this repo

@C47D
Copy link
Collaborator Author

C47D commented Apr 23, 2020

First teaser, still some work to do i think

lvgl_v7

@kisvegabor
Copy link
Member

What is the size and resolution of the display and LV_DPI?

@C47D
Copy link
Collaborator Author

C47D commented Apr 23, 2020

Size of the screen is 320*240, LV_DPI is 100, now it have the proper configuration (of display size and orientation).

lvgl_v7_2

@kisvegabor
Copy link
Member

The demo recognizes if it's a small, medium, large or extra large display and sets the layouts accordingly. This display should have been recognized as small.
What is the size of the display (in inches). We can calculate the real DPI from it.

Besides, please modify LV_DISP_SMALL_LIMIT to 25 in lv_conf.h.

@C47D
Copy link
Collaborator Author

C47D commented Apr 23, 2020

The LCD size is 3.2" (inches), ok I will modify LV_DISP_SMALL_LIMIT to 25 and upload a pic.

@kisvegabor
Copy link
Member

So the actual DPI is: `sqrt(320^2 + 240^2) / 3.2 = 125.
And width of the display is 320/125 = 2.56".

If you set to LV_DISP_SMALL_LIMIT to 25 lvgl will consider > 2.5" display as medium-sized and create this ugly layout. So please set it to 30 (3.0") instead of 25.

@C47D
Copy link
Collaborator Author

C47D commented Apr 23, 2020

Much better

lvgl_v7_3_landscape

@kisvegabor
Copy link
Member

Awesome, thank you!
I'll recalculate the default size limits.

You can enable LV_USE_PERF_MONITOR in lv_conf.h to see the current FPS and CPU usage.

@C47D
Copy link
Collaborator Author

C47D commented Apr 23, 2020

I'm getting 33FPS, with CPU from 18% to 28% and peaks of 34%.

@embeddedt
Copy link
Member

@C47D I'm curious; what FPS do you get at the moment when you scroll past the gauges in the Values tab? I'd like to compare with what I see on STM32F7 (I get 14-18 FPS while scrolling; it increases after that).

@C47D
Copy link
Collaborator Author

C47D commented Apr 23, 2020

Hi @embeddedt, this particular display doesn't have a touch controller, I will try to setup another display to measure the FPS values you've requested.

@kisvegabor
Copy link
Member

@C47D I've modified the demo in test/no-tp branch. It changes between the tabs automatically and shows the gauges on the second tab.

@C47D
Copy link
Collaborator Author

C47D commented Apr 24, 2020

Hi, I took a video of the demo using the test/no-tp branch.

https://youtu.be/BJv-vr03RsM

@kisvegabor
Copy link
Member

kisvegabor commented Apr 24, 2020

Thank you!

There is still huge FPS drop when the gauge is fully redrawn. At least refreshing the needle only is not that bad.

I have a few questions:

  • What is the size of the display buffer?
  • Do you use 2 buffers with DMA in flush_cb?
  • Have you enabled -Os or -O3 optimization?
  • What is the speed of the SPI?

@C47D
Copy link
Collaborator Author

C47D commented Apr 24, 2020

Hi, I'm not at home right now I will update my reply with the data you requested.

I had an issue with the demo, the demo stopped working after some minutes and the screen is stuck, I have tried to replicate the issue but it stopped at different times. I will try to add the log functions to lvgl so I can know when it stops.

@embeddedt
Copy link
Member

the demo stopped working after some minutes and the screen is stuck

I also saw this happen once on my STM32F7, but I didn't have time to investigate further. It was a few days ago.

@C47D
Copy link
Collaborator Author

C47D commented Apr 24, 2020

Added some printf debugging points to see where the display is getting stuck and I can't replicate the issue, the demo has been running for almost an hour, but noticed that the lv_tick_task is still running.

There's also a new issue on the lvgl repo which uses the same dev board as me and doesn't report the issue Dev-7.0 performance experiments

@kisvegabor
Copy link
Member

I also saw the freeze but I thought it's a bug in my hacky display driver. I've already debuged that it stops on a while(vdb->flushing); I still don't know if lv_disp_flush_ready is not called or it's called but as it is called from an interrupt (for all of us) it might mess up things somehow. I mean some assembly level you-never-find-it bug.

I suspect the latest because while(vdb->flushing); reads a bit field which can be quite complicated on assembly level.

@kisvegabor
Copy link
Member

I suggest running the lv_demo_stress() which makes much more drawing.

@embeddedt
Copy link
Member

I call lv_disp_flush_ready directly from the flush_cb function, so it can't be entirely an interrupt-related issue.

@kisvegabor
Copy link
Member

I call lv_disp_flush_ready directly from the flush_cb function, so it can't be entirely an interrupt-related issue.

Ah, good to know. I thought you are using the DMA based flush_cb.

@C47D
Copy link
Collaborator Author

C47D commented Apr 24, 2020

I call lv_disp_flush_ready from a callback when the SPI finished transferring the data via DMA.

@C47D
Copy link
Collaborator Author

C47D commented Apr 24, 2020

@kisvegabor Do you know if it's possible to get a backtrace when the application is stuck? I'm searching some tutorials to debug the esp32 chip with gdb.

@embeddedt
Copy link
Member

If you have debug symbols available and can attach to the device with GDB after it's already running, you should be able to interrupt execution, type bt, and get a decently usable backtrace.

@kisvegabor
Copy link
Member

It seems @embeddedt knows it much better than me 🙁


I added some debug code and will run the stress demo all night, and hopefully it'll freeze.

@C47D
Copy link
Collaborator Author

C47D commented Apr 24, 2020

@embeddedt Thanks, I do have debug symbols available, but it's a pain to setup the ESP32 tools on Windows, I was using the WSL but can't run openocd there, so i have to setup the toolchain in Windows directly :/.

@kisvegabor
Copy link
Member

kisvegabor commented Apr 24, 2020

I think I found the issue but the picture is not perfect yet.

These are the interesting parts of lv_disp_buf_t. So there are 4 bits next to each other. When going to the next part to refresh lvgl might write the last 3 fields. So what I suspect is:

  1. When e.g. last_area is set, first the whole bitfield is read to a register
  2. An interrupt comes and set flushing = 0
  3. In the register read in 1) the last_area is set but flushing is still 1 too because it was cleared in the "real" variable.
  4. The register is written back and it overwrites the flushing bit.

So it's a typical Read-Modify-Write issue.
It all makes sense but @embeddedt said he doesn't use interrupt in the flush_cb.
So @embeddedt, are you sure you weren't using an interrupt based driver when it froze for you?

I've pushed a trivial fix. Let's see if it helps.

@persello
Copy link
Contributor

persello commented May 8, 2020

Hello, I just tried the dev-7.0 branch with my ILI9488. I had some problems with the default frequency set to 64MHz by the f6113af commit. I have to say, I'm on a breadboard, so this could be the issue here, and I won't get any PCBs for my project until late this summer. From what I have tested, 64MHz seems just above the "sweet spot" for this display driver (data sheet says max. 40MHz, but I reached 60MHz with no artifacts), since the screen contents are intelligible but heavily damaged. Could it be possible to have a custom default frequency for this driver set to 40~60MHz and maybe, in the future, having the ability to set a custom one from menuconfig? I have never used Kconfig but looking at it, it doesn't seem that difficult, so I could at least try to add this small feature tomorrow or in the next few days.

EDIT: Created a draft pull request #126 (this is my first PR and Kconfig edit, so changes/edits/suggestions are welcome and no problem if rejected).

@C47D
Copy link
Collaborator Author

C47D commented May 9, 2020

It looks really helpful, thanks for the PR, hopefully more to come :)

@barbiani
Copy link

barbiani commented May 9, 2020 via email

@barbiani
Copy link

@C47D
Have you ever tried running guiTask in the PRO_CPU? I can not understand why it is much faster than APP_CPU. This is true if wifi/bt are disabled.

@C47D
Copy link
Collaborator Author

C47D commented May 24, 2020

No, I haven't. Did you tried to?

@barbiani
Copy link

barbiani commented May 24, 2020 via email

@C47D
Copy link
Collaborator Author

C47D commented May 24, 2020

Do you think we should add a comment about that on the source code? I'm about to merge the dev-7.0 PR into master, I'm finishing some arrangements in the README.

If you could give it a try it would be great, #110

@barbiani
Copy link

I would not add the note until someone figures it out. The cores should be the same.

@embeddedt
Copy link
Member

I would guess that there's another task running on the APP_CPU that we're unaware of.

@kisvegabor
Copy link
Member

@barbiani
Copy link

barbiani commented May 25, 2020

It has something to do with running lvgl in a task. This approach gives the same result as running lvgl in core 0 but without the drawbacks of sharing the core with the comms stack.

Do expect 2x the performance over lv_port_esp32.

I just changed it to:

void lv_tick_hook (void) {
    lv_tick_inc(1);
}

esp_register_freertos_tick_hook(lv_tick_hook);

@C47D
Copy link
Collaborator Author

C47D commented May 25, 2020

So instead of running a periodic timer and increase the LVGL tick on the callback of it you are increasing it at the FreeRTOS tick rate?

@barbiani
Copy link

barbiani commented May 25, 2020 via email

@embeddedt
Copy link
Member

Maybe #136 is related?

@C47D
Copy link
Collaborator Author

C47D commented May 25, 2020

@barbiani Could you review it? I don't have access to my pc to test it right now

@vheathen
Copy link
Contributor

@C47D
Have you ever tried running guiTask in the PRO_CPU? I can not understand why it is much faster than APP_CPU. This is true if wifi/bt are disabled.

Could it be connected to this?

Timer callbacks are called from a task running on the PRO CPU.

@C47D
Copy link
Collaborator Author

C47D commented May 25, 2020

I think @barbiani meant the task that runs LVGL, which is attached to APP CPU, not the CPU of the tick timer.

@vheathen
Copy link
Contributor

I think @barbiani meant the task that runs LVGL, which is attached to APP CPU, not the CPU of the tick timer.

Yes, I understand it. If a timer callback runs on the different (in regard to the LVGL task) core it might have speed penalty (I didn't dig into the lv_tick_inc() func so don't know how expensive it is). If the LVGL task started on the same core as a timer callback then performance is higher than if it started on an another core, so it is just a guess.

@barbiani
Copy link

What I am experiencing is that I get better performance when lv_task_handler() is scheduled on core 0.

@C47D
Copy link
Collaborator Author

C47D commented May 26, 2020

For this demo is OK to pin the lvgl task to CPU 0, or don't even create a new task for it. Once the users try to enable WiFi or BLE into their applications they would need to pin the lvgl to CPU 1.
Is this right?

@C47D
Copy link
Collaborator Author

C47D commented Jun 4, 2020

Closed with #110 being merged into master. Further performance improvements can be discussed in new issues.

@C47D C47D closed this as completed Jun 4, 2020
@barbiani
Copy link

barbiani commented Jun 9, 2020

A small optimization to prevent uneeded context switches.

uint32_t lv_delay;	
while (1) {
	if (xSemaphoreTake(xGuiSemaphore, (TickType_t)10) == pdTRUE) {
		lv_delay = lv_task_handler();
		xSemaphoreGive(xGuiSemaphore);
	}
	vTaskDelay(lv_delay);
}

@C47D
Copy link
Collaborator Author

C47D commented Jun 10, 2020

Nice to see that LVGL feature being used, thanks for the tip @barbiani :), mind to send a pull request for it?

@embeddedt
Copy link
Member

embeddedt commented Jun 10, 2020

I'd add lv_delay = 10; at the beginning of the loop body. That way if the semaphore doesn't get taken the thread will always sleep for 10 ms and try again. The current code could sleep for an undefined amount of time as lv_delay has a potentially undefined value.

@barbiani
Copy link

barbiani commented Jun 10, 2020

Good point. The idea was posted. I have it as static.

Could increase the sem take timeout... but anyway, it sleeps exactly the amount needed by lv_task_handler();

The performance monitor cpu usage drops a lot.

@barbiani
Copy link

Nice to see that LVGL feature being used, thanks for the tip @barbiani :), mind to send a pull request for it?

I am using CONFIG_FREERTOS_HZ == 1000. If it is set to 100hz (10ms) most delays will be broken in 2 or more.

Does it make sense to delay a task by 1ms if the system tick is 10ms?

@embeddedt
Copy link
Member

embeddedt commented Jun 10, 2020

Does it make sense to delay a task by 1ms if the system tick is 10ms?

In general, I believe delays get rounded up to the system timer frequency, therefore the task will effectively be delayed for up to 10ms.

I don't know if FreeRTOS follows that rule or not but most RTOSes do from what I know.

@barbiani
Copy link

barbiani commented Jun 10, 2020 via email

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

No branches or pull requests

6 participants