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

feature(lvgl_port): Initial support for SIMD rendering in LVGL (BSP-525) #357

Conversation

peter-marcisovsky
Copy link
Collaborator

@peter-marcisovsky peter-marcisovsky commented Jul 31, 2024

ESP-BSP Pull Request checklist

Note: For new BSPs create a PR with this link.

  • Version of modified component bumped
  • CI passing

Change description

This MR adds an initial support for SIMD rendering in LVGL using LVGL's internal Blend API

Related

This MR Implements:

  • Assembly source files integrated into the lvgl_port component
  • Integration into the LVGL blend API via LVGL's kconfig
    • LV_DRAW_SW_ASM_CUSTOM must be enabled
    • LV_DRAW_SW_ASM_CUSTOM_INCLUDE must be provided with a header file with function header for the assembly implemented functions, in our case esp_lvgl_port_lv_blend.h
    • This way, the esp_lvgl_port_lv_blend.h is under our control during the development phase, and will be included into the official LVGL repo once finalized, just like the existing SIMD header files from ARM.
  • Assembly implementation of following primitive rendering functions:
    • Simple fill for ARGB8888 color format, supported targets are esp32s3 and esp32
    • Simple fill for RGB565 color format, supported target is esp32
  • Test app with functionality and benchmark tests

@CLAassistant
Copy link

CLAassistant commented Jul 31, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title feature(lvgl_port): Initial support for SIMD rendering in LVGL feature(lvgl_port): Initial support for SIMD rendering in LVGL (BSP-525) Jul 31, 2024
Copy link
Collaborator

@tore-espressif tore-espressif left a comment

Choose a reason for hiding this comment

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

@peter-marcisovsky thank you for the PR! I can see that there is a lot of work done on the assembly part that I am unable to review.

So I'm only leaving few comments about integration with LVGL. We can talk offline toghther with @espzav

@peter-marcisovsky
Copy link
Collaborator Author

Reworked test apps:

  • Functionality test:

    • uses hard copy of necessary lvgl source files, thus creating a local ANSI C version of the LVGL Blend API for functionality test comparison with the Assembly version of the API
    • Does not use neither the LVGL port, nor the LVGL managed component
  • Benchmark test:

    • 2 separate tests apps with different sdkconfig settings (Assembly render Enabled/Disabled) must be build for benchmark comparison
    • Uses LVGL port, as it gives a performance comparison of an overall Assembly render integration into the LVGL

@peter-marcisovsky peter-marcisovsky force-pushed the feature/lvgl_port_enable_simd_render branch 3 times, most recently from afabff1 to 85906eb Compare September 5, 2024 15:44
Copy link
Collaborator

@espzav espzav left a comment

Choose a reason for hiding this comment

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

@peter-marcisovsky thank you for this PR. Good job! LGTM

Copy link
Collaborator

@tore-espressif tore-espressif left a comment

Choose a reason for hiding this comment

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

@peter-marcisovsky I can't compile the test_app locally. Could you please provide som instruction how to do it?

@peter-marcisovsky
Copy link
Collaborator Author

Reworked test apps (again):

Due to a recent LVGL refactoring in the LVGL 9.2 release, where it is not possible to access a private LVGL members by default, also couple of other (breaking in my case) changes, it was decided to get rid of the lvgl component for test apps completely, not to risk another possible refactoring of the lvgl port and test apps.

  • Build:

    • Only for LVGL 9.1 (below)
  • Test apps:

    • Both (functionality and benchmark) merged into a single project, with a single CMake
    • Both are using hard copy of VLGL's blend API directly, thus we do not depend on the higher VLGL API, which is, either-way, not important for us in our tests and not very readable when creating test cases
    • No need to rebuild a project to switch between the ASM and ANSI rendering
  • LVGL version:

    • For now, LVGL 9.1 version is used
    • There is a possibility of opening an new issue to the LVGL, to fix some include errors on their side, which we are facing in this MR with the LVGL 9.2
    • After that we will be able to use changes in this MR with LVGL 9.2

@peter-marcisovsky peter-marcisovsky force-pushed the feature/lvgl_port_enable_simd_render branch from d520d8b to 19db7d6 Compare September 10, 2024 14:11
@tore-espressif
Copy link
Collaborator

Very nice work @peter-marcisovsky !

The only remaining thing is compatibility with LVGL 9.2, we can discuss next week

Copy link
Collaborator

@tore-espressif tore-espressif left a comment

Choose a reason for hiding this comment

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

@peter-marcisovsky Please check if these comments correctly enable SIMD acceleration only for ESP32 and ESP32s3 and for LVGL versions 9.0 and 9.1

Then we can go ahead with merge :)

components/esp_lvgl_port/CMakeLists.txt Outdated Show resolved Hide resolved
components/esp_lvgl_port/idf_component.yml Outdated Show resolved Hide resolved
@peter-marcisovsky peter-marcisovsky force-pushed the feature/lvgl_port_enable_simd_render branch from 19db7d6 to e868e95 Compare September 17, 2024 13:56
Copy link
Collaborator

@tore-espressif tore-espressif left a comment

Choose a reason for hiding this comment

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

LGTM

@espzav @igrr This is ready for merging. Please let us know if you want to have another look before we merge

    - Assembly source files for LVGL blend API integrated into lvgl_port
    - Initial assembly assembly implementation of:
        - ARGB8888 simple fill for esp32s3 and esp32
        - RGB565 simple fill for esp32
    - Functionality and benchmark test app
@peter-marcisovsky peter-marcisovsky force-pushed the feature/lvgl_port_enable_simd_render branch from e868e95 to be9da14 Compare September 17, 2024 17:31
@peter-marcisovsky
Copy link
Collaborator Author

@tore-espressif
Resolved all conversations, LVGL version for this feature set in CMake to 9.1.0 <= LVG_version < 9.2.0
Ready to merge

@espzav
Copy link
Collaborator

espzav commented Sep 18, 2024

LGTM

@espzav @igrr This is ready for merging. Please let us know if you want to have another look before we merge

I fast reviewed it again, LGTM

@igrr
Copy link
Member

igrr commented Sep 20, 2024

Latest version LGTM!

Have you done any composite benchmarks (e.g. frames per second on some LVGL demo)? If yes, it would be interesting to see how much we gain from these assembly routines. (Not a blocker for pressing the merge button.)

@tore-espressif tore-espressif merged commit 064e0e4 into espressif:master Sep 20, 2024
16 checks passed
@peter-marcisovsky peter-marcisovsky self-assigned this Oct 17, 2024
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.

5 participants