lvgl / lvgl_esp32_drivers

Drivers for ESP32 to be used with LVGL
MIT License
339 stars 284 forks source link

Support for multiple displays on different controllers and more. Ideas and Reflections. #113

Closed SinglWolf closed 3 years ago

SinglWolf commented 3 years ago

The essence of the problem. It's global to me. On the verge of a complete rejection of LVGL completely. I will give an example (purely theoretically). The SPI bus frequency is 40 MHz and cannot be changed by the end user. The connected display can only operate at 35 MHz. The owner of the display should ask the firmware developer to compile a custom firmware for their display. Another example (already practical). There is a device with 100% software compatibility. But it is incompatible with the display pins assigned in the firmware. Is it necessary for the owner of the device to cut the tracks on the board and reconnect the display as indicated in the firmware? Or contact the firmware developer again? Or order a printed circuit board for this firmware? Most likely, the device user will look for another solution. There are many of them on the Internet. And the developer of this ill-fated firmware? I don’t know about others, but I’m trying to change something for the better. When discussing the functions of adjusting the brightness of the display backlight, a phrase flashed from the developers that there would be support for multiple displays. The idea is good. But how? Especially two displays on different controllers. Now even in components\lvgl_esp32_drivers\CMakeLists.txt it is written

# Include only the source file of the selected
# display controller.
if(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9341)
    list(APPEND SOURCES "lvgl_tft/ili9341.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9481)
    list(APPEND SOURCES "lvgl_tft/ili9481.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9486)
    list(APPEND SOURCES "lvgl_tft/ili9486.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9488)
    list(APPEND SOURCES "lvgl_tft/ili9488.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ST7789)
    list(APPEND SOURCES "lvgl_tft/st7789.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ST7735S)
    list(APPEND SOURCES "lvgl_tft/st7735s.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ST7796S)
    list(APPEND SOURCES "lvgl_tft/st7796s.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_HX8357)
    list(APPEND SOURCES "lvgl_tft/hx8357.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_SH1107)
    list(APPEND SOURCES "lvgl_tft/sh1107.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_SSD1306)
    list(APPEND SOURCES "lvgl_tft/ssd1306.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_FT81X)
    list(APPEND SOURCES "lvgl_tft/EVE_commands.c")
    list(APPEND SOURCES "lvgl_tft/FT81x.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_IL3820)
    list(APPEND SOURCES "lvgl_tft/il3820.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_JD79653A)
    list(APPEND SOURCES "lvgl_tft/jd79653a.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_UC8151D)
    list(APPEND SOURCES "lvgl_tft/uc8151d.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_RA8875)
    list(APPEND SOURCES "lvgl_tft/ra8875.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_GC9A01)
    list(APPEND SOURCES "lvgl_tft/GC9A01.c")
elseif(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9163C)
    list(APPEND SOURCES "lvgl_tft/ili9163c.c")
else()
    message(WARNING "LVGL ESP32 drivers: Display controller not defined.")
endif()

in components\lvgl_esp32_drivers\component.mk

$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9341),lvgl_tft/ili9341.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9481),lvgl_tft/ili9481.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9486),lvgl_tft/ili9486.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9488),lvgl_tft/ili9488.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ST7789),lvgl_tft/st7789.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ST7735S),lvgl_tft/st7735s.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ST7796S),lvgl_tft/st7796s.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_HX8357),lvgl_tft/hx8357.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_SH1107),lvgl_tft/sh1107.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_SSD1306),lvgl_tft/ssd1306.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_FT81X),lvgl_tft/EVE_commands.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_FT81X),lvgl_tft/FT81x.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_IL3820),lvgl_tft/il3820.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_JD79653A),lvgl_tft/jd79653a.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_UC8151D),lvgl_tft/uc8151d.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_RA8875),lvgl_tft/ra8875.o)
$(call compile_only_if,$(CONFIG_LV_TFT_DISPLAY_CONTROLLER_GC9A01),lvgl_tft/GC9A01.o)

Don't you trust the linker? Or do you care about reducing compile times? Now, with any change in Kconfig, the project is completely recompiled and the forced exclusion of some object modules (which do not exist, because most likely they are not compiled due to restrictions in lvgl_esp32_drivers\lvgl_tft\disp_driver.h) from the general compilation process, no matter what does not really affect. Or maybe this is a concern about the occupied space in the firmware? Even if, purely theoretically, of course, extra modules in some incomprehensible way fall into the firmware file, then a few tens of bytes will not harm the project in any way. On board any ESP32 module, the now is at least 4MB. By the way, about the occupied space. Especially about the occupied space in RAM. I have a feeling that economizing RAM is not for You. As an example, I will take the first function that caught my eye.

void disp_backlight_set(int brightness_percent)
{
    // Check input paramters
    if (bckl_dev == NULL)
        return;
    if (brightness_percent > 100)
        brightness_percent = 100;
    if (brightness_percent < 0)
        brightness_percent = 0;

    ESP_LOGI(TAG, "Setting LCD backlight: %d%%", brightness_percent);

    if (bckl_dev->pwm_control) {
        uint32_t duty_cycle = (1023 * brightness_percent) / 100; // LEDC resolution set to 10bits, thus: 100% = 1023
        ESP_ERROR_CHECK(ledc_set_duty(LEDC_LOW_SPEED_MODE, bckl_dev->index, duty_cycle));
        ESP_ERROR_CHECK(ledc_update_duty(LEDC_LOW_SPEED_MODE, bckl_dev->index));
    } else {
        ESP_ERROR_CHECK(gpio_set_level(bckl_dev->index, brightness_percent));
    }
}

And so, let's start parsing. An int is allocated under brightness_percent, when can limit to uint8_t, because the number is always positive and should never exceed 100. uint32_t duty_cycle - sane? No. Need uint8_t duty_cycle, because the result of the calculation will never exceed 100. And there are a lot of such examples of waste. And really, why bother with it? The demos work and okay. But in my project, I was able to allocate only 1024 bytes for the display buffer. Amazing, right? And this is in the presence of additional extended memory 8MB.

The idea is this. Give the developer users of your driver library the ability to change the settings of certain parameters at runtime, that is, on the fly. How? Need to invent something? Change the concept completely? Yes and no. Look at how the settings system is arranged in the LVGL itself. There it is possible to disable the Kconfig settings and use your own configuration file. Why don't You go the same way? If you like my idea, I can provide more specific examples of how to create such a configuration file in terms of pin reassignment and some other points. Thank you for reading such a long opus. Sorry for the harsh statements, it just boiled over.

tore-espressif commented 3 years ago

But it is incompatible with the display pins assigned in the firmware.

You can set all pins in menuconfig, if you don't use any predefined display. Select predefined display configuration set to None.

Is it necessary for the owner of the device to cut the tracks on the board and reconnect the display as indicated in the firmware? Or contact the firmware developer again? Or order a printed circuit board for this firmware?

I believe you are not serious with these questions.

I don’t know about others, but I’m trying to change something for the better.

You can help by submitting meaningful Pull Requests. Issues like this doesn't help at all.

Now even in components\lvgl_esp32_drivers\CMakeLists.txt it is written...

We have recently pointed you to develop branch. Check it out here: https://github.com/lvgl/lvgl_esp32_drivers/blob/develop/CMakeLists.txt

likely they are not compiled

Everything that is added to CMake configuration is compiled. Linker takes care of elimination of unused symbols.

extra modules in some incomprehensible way fall into the firmware file

Linker behavior is deterministic.

An int is allocated under brightness_percent

Nothing is allocated here. This value is passed as a parameter to the function via CPU register. CPU registers are 32bit wide.

uint32_t duty_cycle - sane? No. Need uint8_t duty_cycle, because the result of the calculation will never exceed 100.

Answer to your question is in a comment, directly after the statement. If you read it, you wouldn't be so confused. uint32_t duty_cycle = (1023 * brightness_percent) / 100; // LEDC resolution set to 10bits, thus: 100% = 1023 uint16_t would suffice here, but ledc_set_duty accepts uint32_t so we are using this type.

But in my project, I was able to allocate only 1024 bytes for the display buffer

you can set display buffer size in menuconfig.

You have made lot of false assumption about C language, compiling, linking and running software on embedded target. As you can see, it is not trivial. The multidisplay support, in particular, is difficult to add with current state of the code. We made few steps to get there, but there is also on option to start from scratch.

SinglWolf commented 3 years ago

You have made lot of false assumption about C language, compiling, linking and running software on embedded target. You didn't even pay attention to my idea of using an independent config. Of course, I'm a dumb C user who just mastered "Hello World!" trying to teach a C programming guru. Yes, God be with You. Write whatever You want. Good luck.

embeddedt commented 3 years ago

I would like to remind everyone to please be constructive when providing suggestions/ideas for improvement. Statements like "you didn't even pay attention", "do you care about reducing compile times?", and "write whatever you want" are not actionable and do not really follow our code of conduct either.

I am not that familiar with the ESP32 project in its current state, as it's not a platform I use, but I think that @tore-espressif and @C47D are genuinely trying to redesign the architecture to be as useful as possible, and the bikeshedding in this post is not particularly constructive. There may be some good suggestions here, but they are difficult to pick out.

Please do not take this statement personally. Feel free to open more specific issue(s) on aspects you feel need improvement. But, at the moment, I am locking this topic.