search

lvgl / lv_port_linux

LVGL configured to work with a standard Linux framebuffer
MIT License
208 stars 140 forks source link

Improve video device selection flexibility and add DRM/KMS and SDL2 backend support #47

Closed marex closed 2 months ago

marex commented 3 months ago

Introduce LV_VIDEO_CARD variable to let users specify different video device via environment variable, instead of recompiling the demo. Add DRM/KMS and SDL2 backend support, so users can test different backends.

In order to build the SDL2 backend, the lvgl submodule has to be rolled forward to newer commit ID, add commit which does that.

kisvegabor commented 3 months ago

Can we make it more dynamic by parsing the content of LV_VIDEO_CARD? If it contains fb, it's a frame buffer device, if it contains dri it's DRM, else it's SDL. For more explicit selection we can use an other environment variable, like LV_VIDEO_CARD_TYPE.

I'm not sure the "video card" is the best name here. What about LV_DISPLAY_DEVICE_NAME and LV_DISPLAY_DEVICE_TYPE?

marex commented 3 months ago

Can we make it more dynamic by parsing the content of LV_VIDEO_CARD? If it contains fb, it's a frame buffer device, if it contains dri it's DRM, else it's SDL.

I think there are two questions here (and pardon my ignorance, I am not that deep in LVGL):

For more explicit selection we can use an other environment variable, like LV_VIDEO_CARD_TYPE.

I updated the current patchset to use separate environment variables for each backend, that is probably better.

I'm not sure the "video card" is the best name here. What about LV_DISPLAY_DEVICE_NAME and LV_DISPLAY_DEVICE_TYPE?

For DRM it is card (see https://www.kernel.org/doc/html/latest/gpu/drm-uapi.html#render-nodes ), for fbdev I am not aware of specific term, so I would say, device node (see mknod(1)). SDL environment variables are out of scope of this discussion.

kisvegabor commented 3 months ago

Is it allowed to build LVGL with multiple backends at the same time (multiple of { fbdev, drm, sdl } )?

Absolutely yes. However the required libs needs to be installed to compile succesfully.

marex commented 3 months ago

Is it allowed to build LVGL with multiple backends at the same time (multiple of { fbdev, drm, sdl } )?

Absolutely yes. However the required libs needs to be installed to compile succesfully.

should this be supported by this example code ?

In fact, what is your take on the current state of this patchset ?

kisvegabor commented 3 months ago

In fact, what is your take on the current state of this patchset ?

I can't clearly see the use case that we are about to solve. When does it happen that you havea single compiled image that you use in so different environments?

marex commented 3 months ago

In fact, what is your take on the current state of this patchset ?

I can't clearly see the use case that we are about to solve. When does it happen that you havea single compiled image that you use in so different environments?

I don't have such a use case. For me it is sufficient to define which backend to use at build time.

kisvegabor commented 3 months ago

Is selecting the display device (e.g. /dev/fb0) still important?

marex commented 3 months ago

Is selecting the display device (e.g. /dev/fb0) still important?

There are systems with multiple outputs, e.g. MX8M Plus has DSI/LVDS/HDMI outputs, each on separate CRTC, which would also each get allocated a separate /dev/fbN device node.

kisvegabor commented 3 months ago

I see. What if we do something like this:

const char * dev = "/dev/fb0"
if(argc > 0) dev = argv[1];
lv_linux_fbdev_set_file(disp, dev);

I think we can leave this project for fb dev only as it's trivial to change it support DRM or SDL. Mentioning it in the comments or in the README could be useful though.

marex commented 2 months ago

I see. What if we do something like this:

const char * dev = "/dev/fb0"
if(argc > 0) dev = argv[1];
lv_linux_fbdev_set_file(disp, dev);

I think we can leave this project for fb dev only as it's trivial to change it support DRM or SDL. Mentioning it in the comments or in the README could be useful though.

Sorry for the late reply.

fbdev and its emulation is long deprecated in favor of DRM/KMS, so the DRM/KMS backend is far more interesting on Linux. The SDL backend is nice to have for GPU access, since SDL can set up the GL context. I think DRM/KMS and SDL are the more important backends on Linux.

What about renaming the repository to plain lv_port_linux and then cover all the output options ? I think github does create backward compatibility repo url alias, so the rename should be harmless.

kisvegabor commented 2 months ago

What about renaming the repository to plain lv_port_linux and then cover all the output options ?

Good idea! On embedded /dev/fb is still widely used, but as long we support all, I'm happy.

I think github does create backward compatibility repo url alias, so the rename should be harmless.

Yes, it does. No problem with renaming.

However I'm afraid we are over-complicating the problem. In the end it's an example project. If we add too many logic we beat the purpose of having a simple code which is easy to understand and modify.

So what I would do is:

marex commented 2 months ago

About this part:

* explain in the README how to use other drivers

Wouldn't that be effectively turning README into main.c , except without the compile coverage, so the examples there would bitrot over time ?

My hope is that with the ifdefs , the different driver use examples are clearly marked in the code , so that hopefully shouldn't confuse people. This is how it looks in the end -- https://github.com/marex/lv_port_linux_frame_buffer/blob/example_drm_kms_sdl2/main.c#L12 .

kisvegabor commented 2 months ago

I think it's a good compromise! Let's go for it!

marex commented 2 months ago

I think it's a good compromise! Let's go for it!

So uh ... what changes should I do here ?

kisvegabor commented 2 months ago

Sorry for the late reply. So I suggest this:

Do you agree?

marex commented 2 months ago

Sorry for the late reply. So I suggest this:

* Add [this](https://github.com/marex/lv_port_linux_frame_buffer/blob/example_drm_kms_sdl2/main.c#L12) in `main.c`

This part is already in place, you can see the result here https://github.com/marex/lv_port_linux_frame_buffer/tree/example_drm_kms_sdl2 and specifically https://github.com/marex/lv_port_linux_frame_buffer/blob/example_drm_kms_sdl2/main.c ). Is that what you want implemented or did I misunderstand ?

* Update the README with telling we support multiple Linux drivers

Done, you can see that in the first push from today, the second push is just tab indent fixup. See https://github.com/lvgl/lv_port_linux_frame_buffer/compare/57d8cff35d8f21fc760f00744fd5720d6b1d9087..47ba151551172c2a1816be32cbf8ff10ee4bbfc1 .

* I'll rename the repo once the PR is merged.

Please give it a once over and see if it still works for you too on your hardware. I can test it locally on MX8M and STM32MP, but another independent test would be nice.

Do you agree?

I do, thank you.

kisvegabor commented 2 months ago

Thank you, really everything were here. I don't know what I was watching :sweat_smile:

I've tested it on my Linux notebook with SDL and needed to add 2 includes. Without these it was crashing. I guess getenv's return value was truncated to 32 bit.

Now all good, merging! I'm doing the rename too.

marex commented 2 months ago

Thank you, really everything were here. I don't know what I was watching 😅

No worries.

I've tested it on my Linux notebook with SDL and needed to add 2 includes. Without these it was crashing. I guess getenv's return value was truncated to 32 bit.

The getenv(3) returns char * , atoi(3) takes char * and returns integer . That shouldn't overflow on x86 or amd64, unless your laptop resolution is enormous.

But including stdlib.h is definitely valid for both getenv() and atoi() according to manpage.

Did the crash go away when you included the missing headers ?

Now all good, merging! I'm doing the rename too.

Thank you

kisvegabor commented 2 months ago

The getenv(3) returns char , atoi(3) takes char and returns integer . That shouldn't overflow on x86 or amd64, unless your laptop resolution is enormous.

The problem is that when a function doesn't have a prototype even on 64 bit systems 32 bit return value is assumed. So let's say getenv returned 0x112233445667788 (an address), but as it was truncated to 32 bit only 0x1122334400000000 arrived. I don't know what the exact standard is behind it but I've already experienced this issue many times.

kisvegabor commented 2 months ago

FYI, I opened a PR to mention this project as a simulator project https://github.com/lvgl/lvgl/pull/6205

marex commented 1 month ago

FYI, I opened a PR to mention this project as a simulator project lvgl/lvgl#6205

Recently, I was wondering, wouldn't it make even more sense to put the main.c from this repository into main lvgl repository demos/ subdirectory , and deprecate this repository altogether ?

kisvegabor commented 1 month ago

I was thinking about something similar too. We have/can have support for these hardware/platform related things:

  1. Display drivers: display controllers (e.g. ILI9341), DRM, Native windows, etc
  2. LCD controller support (drive a display with HSYNC/VSYNC + RGB data): e.g. Renesas's GLCDC periphery
  3. GPU support (speed up rendering): VG-Lite, Dave2D, SDL, OpenGL
  4. Ready to use projects: like this one, but the same for development boards.

1, 2, and 3 are already supported, and now we are talking about 4. So the question is how to support full projects. In a project typically we have:

If we added e.g. a Linux project configured for Eclipse, VSCode, and CMake the user could download LVGL and open a project from e.g. lvgl/projects/linux. It could work, but it's a little bit strange to me. A library like LVGL should be part of a project and not the root folder of a project. I'm also afraid that adding e.g. 5 full NXP MCUXpreso projects (~200 MB each) will bloat LVGL (already 1GB).