dp: Switch DP thread trigger from semaphore to event #10419

softwarecki · 2025-12-08T16:46:43Z

Replace the DP thread trigger mechanism from semaphore to event to prepare for handling multiple types of signals within the DP thread. The previous semaphore-based approach could only wake the thread without providing context about what needs to be processed. Using an event allows signaling different conditions so the thread can identify which tasks to handle. Currently, no new event types are introduced, but this change lays the groundwork for future extensions without altering existing functionality.

Copilot

Pull request overview

This PR refactors the DP thread synchronization mechanism by replacing semaphores with events to enable more flexible task signaling. The change allows the DP thread to distinguish between different types of signals (process vs. cancel) rather than just waking up without context.

Key changes:

Replaced semaphore-based synchronization with event-based synchronization throughout the DP scheduler
Introduced event type enums (DP_EVENT_PROCESS and DP_EVENT_CANCEL) to signal different conditions
Updated thread waiting logic to handle multiple event types

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/schedule/zephyr_dp_schedule.h	Defines event type enums and replaces semaphore fields with event fields in task data structure
src/schedule/zephyr_dp_schedule.c	Updates task initialization, cancellation, and cleanup to use events instead of semaphores
src/schedule/zephyr_dp_schedule_native.c	Modifies thread signaling and waiting logic to use events with multiple event type support
src/schedule/CMakeLists.txt	Changes build configuration to include native scheduler implementation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-08T16:47:11Z

src/schedule/zephyr_dp_schedule_native.c

 	struct scheduler_dp_data *dp_sch = NULL;
 	unsigned int lock_key;
 	enum task_state state;
+	uint32_t events;


The variable 'events' is declared but never used after the k_event_wait_safe() call. Consider checking the returned event flags to validate which event was received, or handling the DP_EVENT_CANCEL case appropriately if the thread needs to behave differently based on the event type.

Copilot · 2025-12-08T16:47:12Z

src/schedule/zephyr_dp_schedule.c


-	/* start the thread, it should immediately stop at a semaphore, so clean it */
-	k_sem_init(pdata->sem, 0, 1);
+	/* start the thread, it should immediately stop at a event */


Corrected grammar from 'at a event' to 'at an event'.

Suggested change

/* start the thread, it should immediately stop at a event */

/* start the thread, it should immediately stop at an event */

lgirdwood · 2025-12-08T17:02:07Z

src/schedule/zephyr_dp_schedule_native.c

+		events = k_event_wait_safe(task_pdata->event,
+					   DP_EVENT_PROCESS | DP_EVENT_CANCEL,
+					   false, K_FOREVER);


This will likely throw a warning now if events set but not used ? I think we have -Wall set for most files ?

lyakh · 2025-12-09T09:03:22Z

src/schedule/CMakeLists.txt

 	zephyr_library_sources_ifdef(CONFIG_ZEPHYR_DP_SCHEDULER
 		zephyr_dp_schedule.c
-		zephyr_dp_schedule_proxy.c
+		zephyr_dp_schedule_native.c


sorry, don't understand the renaming.

The previous name implied a proxy layer, which is misleading because the file only contains the native DP thread logic and is used even when userspace is inactive. It doesn't implement any proxy layer. Renaming improves clarity and prevents incorrect assumptions about its role. This is a cosmetic change and does not affect functionality, so it shouldn't block the PR.

@softwarecki calling one of the 2 alternatives "native" can be interpreted as the other one being "less native," i.e. possibly less supported, less preferable. I don't think that of the present 2 version the "proxy" one is more native than the "application" version. That's also the reason why I agreed to move the "application" version away from the original "native" naming. And that's also the reason I'm not accepting this renaming. Please, drop or come up with another neutral name.

Update the DP scheduler file name by replacing the proxy postfix to reflect actual content and avoid misleading semantics. The file does not implement any proxy layer, and it is also used when userspace is not active. It contains only the native DP thread. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

kv2019i

k_event looks nice for this use. Couple of small nits, but nothing really blocking.

src/schedule/zephyr_dp_schedule_thread.c

src/schedule/zephyr_dp_schedule.c

lyakh

hard to say with certainty without trying it, but I think this is useful. But you forgot to also update the "application" variant, which would be broken after this change

Replace the DP thread trigger mechanism from semaphore to event to prepare for handling multiple types of signals within the DP thread. The previous semaphore-based approach could only wake the thread without providing context about what needs to be processed. Using an event allows signaling different conditions so the thread can identify which tasks to handle. Currently, no new event types are introduced, but this change lays the groundwork for future extensions without altering existing functionality. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

softwarecki · 2025-12-10T10:57:40Z

k_event looks nice for this use. Couple of small nits, but nothing really blocking.

@kv2019i Thanks for these small suggestions. I always appreciate this kind of feedback and Im happy to apply the changes because it really improves the overall quality of the code.

@lyakh I also modified the "application" variant.

unable to copy file 'modules/sof/libmodules_sof.a'; reason: Success

Looks like CI is having a rough day again :(

lyakh · 2025-12-10T11:11:06Z

src/schedule/zephyr_dp_schedule.h

-	struct k_sem *sem;		/* pointer to semaphore for task scheduling */
-	struct k_sem sem_struct;	/* semaphore for task scheduling for kernel threads */
+	struct k_event *event;		/* pointer to event for task scheduling */
+	struct k_event event_struct;	/* event for task scheduling for kernel threads */


alternatively to changing the "application" version in this commit too, you can add the two new members without removing the semaphore, then I'll migrate the other version later too.

softwarecki requested review from abonislawski, Copilot, serhiy-katsyuba-intel, tmleman and wjablon1 December 8, 2025 16:46

softwarecki requested review from LaurentiuM1234, dbaluta, marcinszkudlinski and pblaszko as code owners December 8, 2025 16:46

Copilot AI reviewed Dec 8, 2025

View reviewed changes

lgirdwood reviewed Dec 8, 2025

View reviewed changes

lyakh requested changes Dec 9, 2025

View reviewed changes

softwarecki force-pushed the dp-event branch from 385ca1f to bd23dee Compare December 9, 2025 09:04

softwarecki force-pushed the dp-event branch from bd23dee to fd42748 Compare December 9, 2025 17:22

softwarecki requested review from kv2019i, lbetlej, mmaka1 and plbossart as code owners December 9, 2025 17:22

softwarecki force-pushed the dp-event branch 2 times, most recently from 06be8c3 to 85d0f01 Compare December 9, 2025 17:26

softwarecki requested a review from lyakh December 9, 2025 17:26

kv2019i reviewed Dec 9, 2025

View reviewed changes

src/schedule/zephyr_dp_schedule_thread.c Outdated Show resolved Hide resolved

src/schedule/zephyr_dp_schedule.c Outdated Show resolved Hide resolved

src/schedule/zephyr_dp_schedule.c Outdated Show resolved Hide resolved

lyakh reviewed Dec 10, 2025

View reviewed changes

softwarecki force-pushed the dp-event branch from 85d0f01 to 9e82b19 Compare December 10, 2025 10:57

lyakh reviewed Dec 10, 2025

View reviewed changes

lyakh approved these changes Dec 10, 2025

View reviewed changes

serhiy-katsyuba-intel approved these changes Dec 10, 2025

View reviewed changes

kv2019i merged commit 23c4020 into thesofproject:main Dec 10, 2025
34 of 42 checks passed

	/* start the thread, it should immediately stop at a event */
	/* start the thread, it should immediately stop at an event */

dp: Switch DP thread trigger from semaphore to event #10419

dp: Switch DP thread trigger from semaphore to event #10419

Conversation

softwarecki commented Dec 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

lgirdwood Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

lyakh Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

softwarecki Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

lyakh Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lyakh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

softwarecki commented Dec 10, 2025

Uh oh!

lyakh Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lyakh left a comment •

edited

Loading