ipc4: pipeline: Add notification pool and data process error notification #9972

softwarecki · 2025-04-17T14:08:41Z

Add a pool of IPC messages for sending notifications. These messages are dynamically allocated as needed. Add a callback to the ipc_msg structure that is called after sending a message, allowing messages to be returned to the pool once sent.

Using a notification pool simplifies the process of sending sporadic notifications by components. This approach eliminates the need for components to manage the allocation and release of notifications. Additionally, it conserves memory, as notifications that are never sent during normal operation will not be allocated.

Add functionality to send notifications when a data processing function in a module reports an error. Add notifications for both ll and dp modules.

src/include/ipc4/notification.h

src/include/sof/ipc/notification_pool.h

kv2019i

Code changes looks good. A few question/comments inline, please check.

src/ipc/notification_pool.c

src/include/ipc4/notification.h

Move end of the include guard to cover entire file. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

Fix name and formatting for uaol event notification. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

Separate common resource event notification initialization code to a new resource_notif_header_init function. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

Provide a structure and function to initiate notification of an error reported by the module's data processing function. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

Copilot

Pull Request Overview

This PR adds a notification pool mechanism for IPC messages and integrates error notifications into both ll and dp modules when data processing errors occur.

Introduces dynamically allocated notification pool management in src/ipc/notification_pool.c and corresponding header in src/include/sof/ipc/notification_pool.h.
Updates IPC message structure (adding a callback) and adds error notification message initialization functions in src/ipc/ipc4/notification.c and src/include/ipc4/notification.h.
Enhances pipeline error handling by triggering notifications on processing errors in src/audio/pipeline/pipeline-stream.c and src/audio/pipeline/pipeline-schedule.c.

Reviewed Changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/ipc/notification_pool.c	New implementation of the notification pool with dynamic allocation.
src/ipc/ipc4/notification.c	Added header initialization helper and error notification init.
src/ipc/ipc-common.c	Invokes message callback after a successful send.
src/include/sof/ipc/notification_pool.h	Declares the notification pool retrieval function.
src/include/sof/ipc/msg.h	Adds a callback field to the IPC message structure.
src/include/sof/audio/pipeline.h	Declares the function for sending component copy error notifications.
src/include/ipc4/notification.h	Adjusts enum values and adds process data error structures.
src/audio/pipeline/pipeline-stream.c	Implements error notification during pipeline copy operations.
src/audio/pipeline/pipeline-schedule.c	Triggers error notifications upon failure in module processing.

Files not reviewed (2)

src/ipc/CMakeLists.txt: Language not supported
uuid-registry.txt: Language not supported

Comments suppressed due to low confidence (4)

src/include/ipc4/notification.h:41

The enum value has been renamed from UAOL_RSVD_ to UAOL_EVENT; please ensure that all references to this notification type are updated consistently across the project.

SOF_IPC4_UAOL_EVENT		= 13,

src/ipc/notification_pool.c:51

Consider verifying that decrementing 'pool_depth' on memory allocation failure does not cause an inconsistent or negative pool count, which could lead to unexpected behavior when retrieving notifications.

--pool_depth;

src/audio/pipeline/pipeline-stream.c:135

[nitpick] Double-check that triggering pipeline_comp_copy_error_notify only for error conditions (err < 0) is the intended behavior, and that PPL_STATUS_PATH_STOP is handled appropriately without an error notification.

if (err < 0) {

src/audio/pipeline/pipeline-schedule.c:377

[nitpick] Ensure that triggering pipeline_comp_copy_error_notify here correctly distinguishes between recoverable and non-recoverable errors returned by module_process_sink_src.

if (ret)

Copilot

Pull Request Overview

This PR introduces a notification pool for IPC messages along with error notification mechanisms for data processing failures in pipeline components. Key changes include:

Implementation of a dynamic notification pool with callback support.
Addition of new functions to initialize and send error notifications in both ll and dp modules.
Updates to IPC and audio pipeline modules to integrate error notifications.

Reviewed Changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/ipc/notification_pool.c	Implements dynamic allocation for IPC notifications using a pool.
src/ipc/ipc4/notification.c	Adds initialization functions for resource and error notifications.
src/ipc/ipc-common.c	Invokes the notification callback after sending messages.
src/include/sof/ipc/notification_pool.h	Exposes the notification pool API.
src/include/sof/ipc/msg.h	Adds a callback pointer to the IPC message structure.
src/include/sof/audio/pipeline.h	Declares an API for pipeline component copy error notifications.
src/include/ipc4/notification.h	Updates enums and declares error notification initialization.
src/audio/pipeline/pipeline-stream.c	Implements error notification for copy errors in the pipeline copy path.
src/audio/pipeline/pipeline-schedule.c	Notifies error conditions in module processing during scheduling.

Files not reviewed (2)

src/ipc/CMakeLists.txt: Language not supported
uuid-registry.txt: Language not supported

Copilot · 2025-05-06T16:00:38Z

src/ipc/notification_pool.c

+	item = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, sizeof(*item));
+	if (!item) {
+		key = k_spin_lock(&pool_free_list_lock);
+		--pool_depth;


Consider adding a check or assertion to ensure that pool_depth does not become negative after decrementing on allocation failure.

Suggested change

--pool_depth;

if (pool_depth > 0) {

--pool_depth;

} else {

tr_err(&notif_tr, "pool_depth is already zero, cannot decrement");

}

in fact I wanted to comment on this but eventually decided not to - in the larger context when also checking the caller it is clear that pool_depth cannot underrun, but indeed this design choice isn't making it very obvious, logical and safe - the counter is incremented in the caller and on error it's decremented in the called function. I'd say that it would be more logical to do both at the same level.

Add a pool of IPC messages for sending notifications. These messages are dynamically allocated as needed. Add a callback to the ipc_msg structure that is called after sending a message, allowing messages to be returned to the pool once sent. Using a notification pool simplifies the process of sending sporadic notifications by components. This approach eliminates the need for components to manage the allocation and release of notifications. Additionally, it conserves memory, as notifications that are never sent during normal operation will not be allocated. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

Add functionality to send notifications when a data processing function in a module reports an error. Add notifications for both ll and dp modules. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

Copilot

Pull Request Overview

This PR introduces a notification pool for IPC messages, adding a callback mechanism to automatically recycle messages after transmission and integrating data processing error notifications into the pipeline.

Implements a notification pool with dynamic allocation and free-list management in src/ipc/notification_pool.c.
Adds process data error notification support in IPC4 via functions in src/ipc/ipc4/notification.c and integrates it into pipeline error handling in pipeline-stream.c and pipeline-schedule.c.
Updates relevant header files to expose the new functionality.

Reviewed Changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/ipc/notification_pool.c	Implements notification pool allocation and callback mechanism.
src/ipc/ipc4/notification.c	Adds data error notification initialization and header refactor.
src/ipc/ipc-common.c	Integrates callback invocation after successful IPC message send.
src/include/sof/ipc/notification_pool.h	Declares the API for retrieving notification messages.
src/include/sof/ipc/msg.h	Adds a callback member to the ipc_msg structure.
src/include/sof/audio/pipeline.h	Declares the new pipeline error notification function.
src/include/ipc4/notification.h	Updates IPC4 notification definitions to support process data error events.
src/audio/pipeline/pipeline-stream.c	Triggers error notifications on pipeline copy errors.
src/audio/pipeline/pipeline-schedule.c	Invokes error notifications when module processing fails.

Files not reviewed (2)

src/ipc/CMakeLists.txt: Language not supported
uuid-registry.txt: Language not supported

src/ipc/notification_pool.c

lgirdwood · 2025-05-08T08:21:57Z

CI has stalled. rerun.

lgirdwood · 2025-05-08T08:22:03Z

SOFCI TEST

softwarecki requested review from ArsenEloglian, abonislawski, kv2019i, lgirdwood, lyakh, marcinszkudlinski, serhiy-katsyuba-intel and tmleman April 17, 2025 14:08

softwarecki force-pushed the notify-pool branch 2 times, most recently from 73d3988 to 7640863 Compare April 17, 2025 14:56

softwarecki closed this Apr 17, 2025

softwarecki force-pushed the notify-pool branch from 7640863 to 06d7cff Compare April 17, 2025 15:09

softwarecki reopened this Apr 17, 2025

softwarecki force-pushed the notify-pool branch 8 times, most recently from 974d174 to 3beaf43 Compare April 22, 2025 11:13

softwarecki marked this pull request as ready for review April 22, 2025 12:13

softwarecki requested review from RanderWang, bardliao, dbaluta, lbetlej, mmaka1, pblaszko and plbossart as code owners April 22, 2025 12:13

lyakh reviewed Apr 23, 2025

View reviewed changes

src/include/ipc4/notification.h Show resolved Hide resolved

src/include/sof/ipc/notification_pool.h Show resolved Hide resolved

kv2019i reviewed Apr 23, 2025

View reviewed changes

src/ipc/notification_pool.c Outdated Show resolved Hide resolved

src/include/ipc4/notification.h Show resolved Hide resolved

softwarecki force-pushed the notify-pool branch from 3beaf43 to 2404c92 Compare May 5, 2025 12:19

softwarecki added 4 commits May 5, 2025 14:21

ipc4: notification: Fix include guard

c5282ef

Move end of the include guard to cover entire file. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

ipc4: notification: Fix UAOL event notification name

ec8bb13

Fix name and formatting for uaol event notification. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

ipc4: notification: Separate resource_notif_header_init function

867b79e

Separate common resource event notification initialization code to a new resource_notif_header_init function. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

ipc4: notification: Add support for data process error notification

78ee689

Provide a structure and function to initiate notification of an error reported by the module's data processing function. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

softwarecki force-pushed the notify-pool branch 2 times, most recently from 5c22aa4 to de9d279 Compare May 5, 2025 12:26

kv2019i requested a review from Copilot May 6, 2025 14:37

Copilot AI reviewed May 6, 2025

View reviewed changes

kv2019i approved these changes May 6, 2025

View reviewed changes

softwarecki requested a review from Copilot May 6, 2025 15:59

Copilot AI reviewed May 6, 2025

View reviewed changes

serhiy-katsyuba-intel approved these changes May 7, 2025

View reviewed changes

lgirdwood approved these changes May 7, 2025

View reviewed changes

softwarecki added 2 commits May 7, 2025 15:53

pipeline: Add data process error notification

95274dd

Add functionality to send notifications when a data processing function in a module reports an error. Add notifications for both ll and dp modules. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

softwarecki force-pushed the notify-pool branch from de9d279 to 95274dd Compare May 7, 2025 13:53

softwarecki requested review from Copilot and lyakh May 7, 2025 14:18

Copilot AI reviewed May 7, 2025

View reviewed changes

lyakh approved these changes May 8, 2025

View reviewed changes

src/ipc/notification_pool.c Show resolved Hide resolved

lgirdwood merged commit 98a2797 into thesofproject:main May 9, 2025
44 of 49 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ipc4: pipeline: Add notification pool and data process error notification #9972

ipc4: pipeline: Add notification pool and data process error notification #9972

Uh oh!

softwarecki commented Apr 17, 2025

Uh oh!

Uh oh!

Uh oh!

kv2019i left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 6, 2025

Uh oh!

lyakh May 7, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

lgirdwood commented May 8, 2025

Uh oh!

lgirdwood commented May 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

-		--pool_depth;
+		if (pool_depth > 0) {
+			--pool_depth;
+		} else {
+			tr_err(&notif_tr, "pool_depth is already zero, cannot decrement");
+		}

ipc4: pipeline: Add notification pool and data process error notification #9972

ipc4: pipeline: Add notification pool and data process error notification #9972

Uh oh!

Conversation

softwarecki commented Apr 17, 2025

Uh oh!

Uh oh!

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 6, 2025

Choose a reason for hiding this comment

Uh oh!

lyakh May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

lgirdwood commented May 8, 2025

Uh oh!

lgirdwood commented May 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants