SDL display driver doesn't support API calls from a thread other than main #71410

kartben · 2024-04-11T21:48:13Z

Describe the bug

Trying to call display functions (ex display_write) from a thread other than main does not seem to work.

To Reproduce
Steps to reproduce the behavior:

change samples/drivers/display/src/main.c so that the contents of main() is effectively executed in a different thread
build and run on native_sim --> screen is black

Expected behavior

SDL driver should work in a multithreaded environment, just like other display drivers.

Impact

can't run custom GUI code that requires a dedicated thread using native sim

Environment (please complete the following information):

Linux x86_64, 3.6.99 f419ea7

The text was updated successfully, but these errors were encountered:

aescolar · 2024-04-12T07:13:02Z

@kartben can you provide a minimal repro case?

kartben · 2024-04-12T10:23:25Z

@aescolar Tweaking the display driver sample like the below is the minimal repro. After applying the change, sample still works fine on e.g. M5Stack Core 2 (ILI9342C display controller), but SDL/native_sim_64 gives a black screen.

diff --git a/samples/drivers/display/src/main.c b/samples/drivers/display/src/main.c
index a9267b51c7..af14abc3b0 100644
--- a/samples/drivers/display/src/main.c
+++ b/samples/drivers/display/src/main.c
@@ -170,8 +170,24 @@ static inline void fill_buffer_mono10(enum corner corner, uint8_t grey,
 	fill_buffer_mono(corner, grey, 0xFFu, 0x00u, buf, buf_size);
 }
 
-int main(void)
+int gui_entry_point(void *arg1, void *arg2, void *arg3);
+
+K_THREAD_STACK_DEFINE(thread_stack, 4096);
+static struct k_thread thread;
+
+int main(void) {
+	k_thread_create(&thread, thread_stack, K_THREAD_STACK_SIZEOF(thread_stack),
+			(k_thread_entry_t)gui_entry_point, NULL, NULL, NULL,
+			K_PRIO_PREEMPT(5), 0, K_NO_WAIT);
+	return 0;
+}
+
+int gui_entry_point(void *arg1, void *arg2, void *arg3)
 {
+	ARG_UNUSED(arg1);
+	ARG_UNUSED(arg2);
+	ARG_UNUSED(arg3);
+	
 	size_t x;
 	size_t y;
 	size_t rect_w;

SDL documentation is pretty clear on the fact that e.g. SDL_RenderPresent() or SDL_PollEvent() should only be called from the thread that initialized the video system.

aescolar · 2024-04-12T11:50:31Z

CC @jfischer-no @danieldegrasse @dcpleung

aescolar · 2024-04-12T12:17:03Z

@kartben I can reproduce it with that example.

SDL documentation is pretty clear on the fact that e.g. SDL_RenderPresent() or SDL_PollEvent() should only be called from the thread that initialized the video system.

sdl_display_init() is run in the same "zephyr boot" thread that will eventually run the app main()
So it may be that you are right and that is the cause.
Also, as with a quick hack like this below, on top of your patch it renders properly (so sdl_display_init_bottom is called from the same thread that does the display updates):

diff --git a/drivers/display/display_sdl.c b/drivers/display/display_sdl.c
index f21288d1b0f..44013c5adfe 100644
--- a/drivers/display/display_sdl.c
+++ b/drivers/display/display_sdl.c
@@ -48,7 +48,10 @@ static inline uint32_t mono_pixel_order(uint32_t order)
        }
 }
 
-static int sdl_display_init(const struct device *dev)
+static int sdl_display_init(const struct device *dev) {
+       return 0;
+}
+int _sdl_display_init(const struct device *dev)
 {
        const struct sdl_display_config *config = dev->config;
        struct sdl_display_data *disp_data = dev->data;

diff --git a/samples/drivers/display/src/main.c b/samples/drivers/display/src/main.c
index a9267b51c74..1142d36ed30 100644
--- a/samples/drivers/display/src/main.c
+++ b/samples/drivers/display/src/main.c
@@ -189,6 +204,9 @@ int main(void)
        fill_buffer fill_buffer_fnc = NULL;
 
        display_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_display));
+       _sdl_display_init(display_dev);
        if (!device_is_ready(display_dev)) {
                LOG_ERR("Device %s not found. Aborting sample.",
                        display_dev->name);

Anyhow, SDL is a bit far from my expertise.
But I guess that if this is indeed the reason, a fix would be to move all the sdl display handling into the same thread.
I won't have time to do this now though.
So I better remove myself as assignee.

Maybe you or one of the display maintainers has the bandwith?

See issue for more details zephyrproject-rtos/zephyr#71410 Workaround is to run lv_task_handler from main thread and disable CONFIG_LV_Z_FLUSH_THREAD.

github-actions · 2024-06-12T00:29:38Z

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

See issue for more details zephyrproject-rtos/zephyr#71410 Workaround is to run lv_task_handler from main thread and disable CONFIG_LV_Z_FLUSH_THREAD.

KimBP · 2024-07-16T19:27:55Z

@kartben I can reproduce it with that example.

SDL documentation is pretty clear on the fact that e.g. SDL_RenderPresent() or SDL_PollEvent() should only be called from the thread that initialized the video system.

sdl_display_init() is run in the same "zephyr boot" thread that will eventually run the app main() So it may be that you are right and that is the cause. Also, as with a quick hack like this below, on top of your patch it renders properly (so sdl_display_init_bottom is called from the same thread that does the display updates):
diff --git a/drivers/display/display_sdl.c b/drivers/display/display_sdl.c
index f21288d1b0f..44013c5adfe 100644
--- a/drivers/display/display_sdl.c
+++ b/drivers/display/display_sdl.c
@@ -48,7 +48,10 @@ static inline uint32_t mono_pixel_order(uint32_t order)
        }
 }
 
-static int sdl_display_init(const struct device *dev)
+static int sdl_display_init(const struct device *dev) {
+       return 0;
+}
+int _sdl_display_init(const struct device *dev)
 {
        const struct sdl_display_config *config = dev->config;
        struct sdl_display_data *disp_data = dev->data;

diff --git a/samples/drivers/display/src/main.c b/samples/drivers/display/src/main.c
index a9267b51c74..1142d36ed30 100644
--- a/samples/drivers/display/src/main.c
+++ b/samples/drivers/display/src/main.c
@@ -189,6 +204,9 @@ int main(void)
        fill_buffer fill_buffer_fnc = NULL;
 
        display_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_display));
+       _sdl_display_init(display_dev);
        if (!device_is_ready(display_dev)) {
                LOG_ERR("Device %s not found. Aborting sample.",
                        display_dev->name);
Anyhow, SDL is a bit far from my expertise. But I guess that if this is indeed the reason, a fix would be to move all the sdl display handling into the same thread. I won't have time to do this now though. So I better remove myself as assignee.

Maybe you or one of the display maintainers has the bandwith?

This "workaround" wont work if you also add lvgl into your application. lvgl_init() is called too early and will miss a display to work at.
Forcing lv_task_handler() to be called in context of main-thread like suggested by @jakkra I also find very limiting.
Wouldn't a solution be to (optionally) make the sdl-display-driver spawn a thread being responsible for all the SDL critical stuff like initializing 'video system' and calling RenderPresent etc?
I doubt the overhead of a thread in a native_sim application would cause any resource issues

aescolar · 2024-07-17T05:58:55Z

@KimBP just to be clear: That (#71410 (comment)) was not a workaround, and I would very much discourage anybody from doing that. That comment was only meant to provide information about the possible cause of the issue.

github-actions · 2024-09-16T00:33:45Z

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

kartben · 2024-11-10T09:24:05Z

@Finomnis just in case you feel like spending more time on the SDL driver :-)

Finomnis · 2024-11-10T09:34:54Z

@kartben Might take a look at it. Although the only solution that comes to my mind is spawning a dedicated SDL thread.

github-actions · 2025-01-10T00:34:33Z

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

github-actions · 2025-03-16T00:38:47Z

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

PetervdPerk-NXP · 2025-03-24T21:43:12Z

Related to #87519, setting SDL_DISPLAY_USE_HARDWARE_ACCELERATOR=n gives a screen again calling from other threads. But then sdl input breaks, most likely due to resource starvation.

Finomnis · 2025-03-24T21:46:04Z

Related to #87519, setting SDL_DISPLAY_USE_HARDWARE_ACCELERATOR=n fixes screen calling from other threads. But then sdl input breaks.

I would intercept this and weaken it a little; we did not confirm that it actually fully breaks SDL input. It could be just in your project, where the priority of the render thread starves the input thread.

Finomnis · 2025-03-24T21:47:35Z

Also, be careful with the word "fixes", both are undefined behavior in the current implementation, it just happens to work better.

PetervdPerk-NXP · 2025-03-24T21:55:22Z

Related to #87519, setting SDL_DISPLAY_USE_HARDWARE_ACCELERATOR=n fixes screen calling from other threads. But then sdl input breaks.

I would intercept this and weaken it a little; we did not confirm that it actually fully breaks SDL input. It could be just in your project, where the priority of the render thread starves the input thread.

Good point could it be that this is a problem in general?
Render thread being more important then input thread?
Your typical mouse cursor on desktop also has a high priority.

EDIT: my apologies for closing and re-opening. Pressed the wrong button.

Finomnis · 2025-03-24T22:21:40Z

Good point could it be that this is a problem in general? Render thread being more important then input thread? Your typical mouse cursor on desktop also has a high priority.

Those priorities should be configurable. Maybe they should be reversed in your project?

faxe1008 · 2025-03-27T09:57:31Z

I took some time yesterday and did some testing. An easy way to break rendering is to have an additional flush thread:

west build -b native_sim/native/64 -d build samples/modules/lvgl/demos -- -DCONFIG_LV_Z_FLUSH_THREAD=y

What I found is that rendering breaks the second writes are done from a different thread:

diff --git a/drivers/display/display_sdl.c b/drivers/display/display_sdl.c
index 7396b105052..9f65c4e44c9 100644
--- a/drivers/display/display_sdl.c
+++ b/drivers/display/display_sdl.c
@@ -8,6 +8,7 @@
 #define DT_DRV_COMPAT zephyr_sdl_dc

 #include <zephyr/drivers/display.h>
+#include <zephyr/kernel.h>

 #include <string.h>
 #include <stdlib.h>
@@ -38,6 +39,7 @@ struct sdl_display_data {
        enum display_pixel_format current_pixel_format;
        uint8_t *buf;
        uint8_t *read_buf;
+       k_tid_t sdl_thread_id;
 };

 static inline uint32_t mono_pixel_order(uint32_t order)
@@ -55,6 +57,7 @@ static int sdl_display_init(const struct device *dev)
        struct sdl_display_data *disp_data = dev->data;
        bool use_accelerator = true;
        LOG_DBG("Initializing display driver");
+    disp_data->sdl_thread_id = k_current_get();

        IF_DISABLED(CONFIG_SDL_DISPLAY_USE_HARDWARE_ACCELERATOR, (use_accelerator = false));

@@ -254,6 +257,10 @@ static int sdl_display_write(const struct device *dev, const uint16_t x,
        const struct sdl_display_config *config = dev->config;
        struct sdl_display_data *disp_data = dev->data;

+       if (disp_data->sdl_thread_id != k_current_get()) {
+               LOG_WRN_ONCE("Display write called from wrong thread");
+       }
+
        LOG_DBG("Writing %dx%d (w,h) bitmap @ %dx%d (x,y)", desc->width,
                        desc->height, x, y);

What I find strange is that currently even without the -DCONFIG_LV_Z_FLUSH_THREAD handling of input events also happens from another thread https://github.com/zephyrproject-rtos/zephyr/blob/main/boards/native/common/sdl/sdl_events.c#L13
But this seems to work?

Anyways I feel like all operations need to be happening in one thread:

Init of display (partially done in display_sdl_bottom.c and sdl_events.c, but the part within sdl_bottom seems to be the important one)
Handling of events (currently in sdl_events.c)
Display write operations

The input_sdl_touch binding should be ok since it only adds a callback within the init everything else pretty much happens when the callback is triggered.

Don't know what is the best solution for this - let the display driver have an own thread where init happens and a message queue for pending writes? Any ideas @Finomnis @aescolar

aescolar · 2025-03-27T10:03:32Z

Don't know what is the best solution for this - let the display driver have an own thread where init happens and a message queue for pending writes?

Something like that makes sense to me.

Finomnis · 2025-03-27T10:14:46Z

I would probably implement it very similar to how the LVGL flush thread is implemented.

About priorities: does this thread need a high priority or will it automatically inherit the priority of the calling thread if they synchronize via semaphore?

faxe1008 · 2025-03-27T10:30:03Z

Did a very dirty quick sketch for this:
faxe1008@f1f31bc

I find that this indeed fixes the rendering, would be intersting if this also fixes @PetervdPerk-NXP and @JarmouniA issues.

Keeping the message buffer in memory really will not be needed if we make sure that there is only write message at a time, so thats something I will cleanup before submitting.

About priorities: does this thread need a high priority or will it automatically inherit the priority of the calling thread if they synchronize via semaphore?

I think this will be a matter of priorities.

Finomnis · 2025-03-27T10:36:53Z

@faxe1008 gotta be careful about recursion, the case where the input handlers get called from this thread which in turn causes a new write message. Might need to check if the calling thread is the SDL thread and then bypass the synchronization primitives.

Add a thread to the SDL display driver to ensure that init and renderer calls are performed from the same thread. Fixes zephyrproject-rtos#71410. Signed-off-by: Fabian Blatz <fabianblatz@gmail.com>

kartben added bug The issue is a bug, or the PR is fixing a bug area: native port Host native arch port (native_sim) area: Display labels Apr 11, 2024

zephyrbot assigned aescolar Apr 11, 2024

aescolar assigned danieldegrasse and unassigned aescolar Apr 12, 2024

aescolar added the priority: low Low impact/importance bug label Apr 12, 2024

faxe1008 mentioned this issue Apr 30, 2024

Update LVGL module to 9.2.0 #68168

Merged

8 tasks

jakkra mentioned this issue May 31, 2024

Workaround for Linux build getting black screen on some computers. ZSWatch/ZSWatch#288

Merged

github-actions bot added the Stale label Jun 12, 2024

kartben removed the Stale label Jun 12, 2024

github-actions bot added the Stale label Sep 16, 2024

mmahadevan108 removed the Stale label Sep 17, 2024

github-actions bot added the Stale label Jan 10, 2025

danieldegrasse removed the Stale label Jan 14, 2025

github-actions bot added the Stale label Mar 16, 2025

Finomnis mentioned this issue Mar 24, 2025

[native_sim] Zephyr 4.1 breaks GPIO_EMUL_SDL and INPUT_SDL_TOUCH #87519

Open

PetervdPerk-NXP closed this as completed Mar 24, 2025

PetervdPerk-NXP reopened this Mar 24, 2025

github-actions bot removed the Stale label Mar 25, 2025

JarmouniA mentioned this issue Mar 25, 2025

[WIP] LVGL: multi-display support #86815

Draft

faxe1008 linked a pull request Mar 31, 2025 that will close this issue

drivers: display: sdl: Ensure draw is performed from init thread #87883

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SDL display driver doesn't support API calls from a thread other than main #71410

SDL display driver doesn't support API calls from a thread other than main #71410

kartben commented Apr 11, 2024 •

edited

Loading

aescolar commented Apr 12, 2024

kartben commented Apr 12, 2024

aescolar commented Apr 12, 2024

aescolar commented Apr 12, 2024 •

edited

Loading

github-actions bot commented Jun 12, 2024

KimBP commented Jul 16, 2024

aescolar commented Jul 17, 2024

github-actions bot commented Sep 16, 2024

kartben commented Nov 10, 2024

Finomnis commented Nov 10, 2024 •

edited

Loading

github-actions bot commented Jan 10, 2025

github-actions bot commented Mar 16, 2025

PetervdPerk-NXP commented Mar 24, 2025 •

edited

Loading

Finomnis commented Mar 24, 2025 •

edited

Loading

Finomnis commented Mar 24, 2025 •

edited

Loading

PetervdPerk-NXP commented Mar 24, 2025 •

edited

Loading

Finomnis commented Mar 24, 2025

faxe1008 commented Mar 27, 2025

aescolar commented Mar 27, 2025

Finomnis commented Mar 27, 2025 •

edited

Loading

faxe1008 commented Mar 27, 2025

Finomnis commented Mar 27, 2025

SDL display driver doesn't support API calls from a thread other than main #71410

SDL display driver doesn't support API calls from a thread other than main #71410

Comments

kartben commented Apr 11, 2024 • edited Loading

aescolar commented Apr 12, 2024

kartben commented Apr 12, 2024

aescolar commented Apr 12, 2024

aescolar commented Apr 12, 2024 • edited Loading

github-actions bot commented Jun 12, 2024

KimBP commented Jul 16, 2024

aescolar commented Jul 17, 2024

github-actions bot commented Sep 16, 2024

kartben commented Nov 10, 2024

Finomnis commented Nov 10, 2024 • edited Loading

github-actions bot commented Jan 10, 2025

github-actions bot commented Mar 16, 2025

PetervdPerk-NXP commented Mar 24, 2025 • edited Loading

Finomnis commented Mar 24, 2025 • edited Loading

Finomnis commented Mar 24, 2025 • edited Loading

PetervdPerk-NXP commented Mar 24, 2025 • edited Loading

Finomnis commented Mar 24, 2025

faxe1008 commented Mar 27, 2025

aescolar commented Mar 27, 2025

Finomnis commented Mar 27, 2025 • edited Loading

faxe1008 commented Mar 27, 2025

Finomnis commented Mar 27, 2025

kartben commented Apr 11, 2024 •

edited

Loading

aescolar commented Apr 12, 2024 •

edited

Loading

Finomnis commented Nov 10, 2024 •

edited

Loading

PetervdPerk-NXP commented Mar 24, 2025 •

edited

Loading

Finomnis commented Mar 24, 2025 •

edited

Loading

Finomnis commented Mar 24, 2025 •

edited

Loading

PetervdPerk-NXP commented Mar 24, 2025 •

edited

Loading

Finomnis commented Mar 27, 2025 •

edited

Loading