P20 eq iir - convert the eq_iir module to only use the sink/source api by piotrhoppeintel · Pull Request #11234 · thesofproject/sof

piotrhoppeintel · 2026-09-25T08:43:34Z

Rework the eq_iir module to only use the sink/source api to
prepare the SOF for the full transition to pipeline 2.0.

Copilot

Copilot review overview

🟢 Approval recommended

No unresolved review issues were identified.

Review effort: Lite
Findings: None

What changed in this PR

Reworks the EQ IIR module to use the sink/source API for Pipeline 2.0 compatibility.

Changes:

Migrates processing and circular-buffer handling to sink/source APIs.
Updates EQ, passthrough, and format-conversion callbacks.
Adapts unit tests to the new processing interface.

File	Description
`test/cmocka/src/audio/eq_iir/eq_iir_process.c`	Updates EQ IIR tests for sink/source processing.
`src/audio/eq_iir/eq_iir.h`	Updates processing interfaces and state.
`src/audio/eq_iir/eq_iir.c`	Integrates sink/source processing and preparation.
`src/audio/eq_iir/eq_iir_ipc3.c`	Adapts IPC3 format-conversion routines.
`src/audio/eq_iir/eq_iir_generic.c`	Migrates generic IIR and passthrough processing.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

lyakh · 2026-09-28T06:17:21Z

+	size_t frame_count;
 	int ret;

+	if (num_input_buffers != 1 || num_output_buffers != 1) {


do we commonly check this in all / most modules? This is a hot path, wouldn't it be enough to check this where buffers / connections are configured?

ack, we could actually put this in a cold path and have a simpler helper too. This shouldn't block this, @piotrhoppeintel we should follow up in another PR as I suspect we have copy and paste for this in more than 1 place.

We dont need to check it every process call. One check in prepare should be fine.

lgirdwood · 2026-09-28T19:32:40Z

@piotrhoppeintel 1 CI open.

softwarecki

Please check variable types in every processing function.

softwarecki · 2026-10-06T15:43:41Z

+
+	source = sources[0];
+	sink = sinks[0];
+	cd->channels = source_get_channels(source);


Do we need this in every process call?

Done. The line has been removed.

softwarecki · 2026-10-06T15:45:13Z

+			      &source_buf.buf_start, &source_buf_size);
+	if (ret < 0)
+		return ret;
+	if (source_buf_size < source_bytes) {


Is it possible?

Done. The redundant check has been removed.

softwarecki · 2026-10-06T15:45:24Z

+		source_release_data(source, 0);
+		return ret;
+	}
+	if (sink_buf_size < sink_bytes) {


Done. The redundant check has been removed.

softwarecki · 2026-10-06T15:46:24Z

-	if (!sourceb || !sinkb) {
+	/* EQ component will only ever have 1 source and 1 sink buffer. */
+	if (num_of_sources != 1 || num_of_sinks != 1) {
 		comp_err(dev, "no source or sink buffer");


Fix error msg

softwarecki · 2026-10-06T15:47:02Z

+		comp_err(dev, "source and sink channel counts do not match");
+		return -EINVAL;
+	}
+	cd->channels = channels;


cd is filled here, so doing it in process is redundant.

Done. The redundant in process function has been removed.

softwarecki · 2026-10-06T15:47:26Z

-			   struct output_stream_buffer *bsink, uint32_t frames);
+typedef void (*eq_iir_func)(struct processing_module *mod,
+			    struct cir_buf_source *source,
+			    struct cir_buf_sink *sink, uint32_t frames);


frames as size_t

softwarecki · 2026-10-06T15:47:36Z

 	int32_t *iir_delay;			/**< pointer to allocated RAM */
 	size_t config_size;			/**< configuration size */
 	size_t iir_delay_size;			/**< allocated size */
+	int channels;				/**< number of channels */


unsigned int

softwarecki · 2026-10-06T15:48:23Z

 	int j;
 	int n;
-	const int nch = audio_stream_get_channels(source);
+	const int nch = cd->channels;


unsigned int

softwarecki · 2026-10-06T15:49:12Z

 	int n;
-	const int nch = audio_stream_get_channels(source);
+	const int nch = cd->channels;
 	const int samples = frames * nch;


size_t here and below

softwarecki · 2026-10-12T20:58:40Z

static size_t eq_iir_init_coef(struct processing_module *mod, unsigned int nch)

softwarecki · 2026-10-12T20:58:55Z

softwarecki · 2026-10-12T20:59:20Z

unsigned int

softwarecki · 2026-10-12T20:59:56Z

unsigned int nch

softwarecki · 2026-10-12T21:00:13Z

unsigned int i;

softwarecki · 2026-10-12T21:00:58Z

-void eq_iir_pass(struct processing_module *mod, struct input_stream_buffer *bsource,
-		 struct output_stream_buffer *bsink, uint32_t frames)
+void eq_iir_pass(struct processing_module *mod, struct cir_buf_source *source,
+		 struct cir_buf_sink *sink, uint32_t frames)


size_t frames

softwarecki · 2026-10-12T21:01:26Z

 int eq_iir_setup(struct processing_module *mod, int nch)
 {
 	struct comp_data *cd = module_get_private_data(mod);
 	int delay_size;


softwarecki · 2026-10-12T21:02:22Z

+			struct cir_buf_sink *sink, uint32_t frames);

 int eq_iir_new_blob(struct processing_module *mod, enum sof_ipc_frame source_format,
 		    enum sof_ipc_frame sink_format, int channels);


unsigned int channels

softwarecki · 2026-10-12T21:03:00Z

+	struct sof_sink *sink;
 	enum sof_ipc_frame source_format;
 	enum sof_ipc_frame sink_format;
 	int channels;


unsigned int

Migrate EQ IIR processing from legacy stream buffers to the source/sink module API. Use circular buffer views for generic processing and IPC3 S32-to-S16/S24 conversion paths, with exact source release and sink commit handling. Signed-off-by: Piotr Hoppe <piotr.hoppe@intel.com>

intel-sofci · 2026-10-13T08:50:18Z

PR 11234: test results

Run date: 2026-10-13 08:13 UTC

Tested commit: 649d255b1f7ef7424d4da53fb67d5e5d5591e7ef

softwarecki · 2026-10-13T09:07:55Z

+	size_t source_bytes;
+	size_t sink_bytes;
+	size_t source_buf_size;
+	size_t sink_buf_size;


source_frame_bytes and sink_frame_bytes seem unnecessary, as they are used only once. The corresponding expressions could be inlined at the call sites, which would reduce the number of temporary variables.

source_buf_size and sink_buf_size could be merged into a single variable.

@piotrhoppeintel can you do this as a followup, I'd like to give full e2e P20 pipelines a good run in CI to shake out any integration issues over next few weeks.

Copilot AI lite review requested due to automatic review settings September 25, 2026 08:43

piotrhoppeintel requested review from dbaluta, kv2019i, lbetlej, lgirdwood, mmaka1, plbossart and singalsu as code owners September 25, 2026 08:43

Copilot started reviewing on behalf of piotrhoppeintel September 25, 2026 08:44 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

lgirdwood approved these changes Sep 25, 2026

View reviewed changes

lyakh reviewed Sep 28, 2026

View reviewed changes

softwarecki reviewed Oct 6, 2026

View reviewed changes

softwarecki reviewed Oct 12, 2026

View reviewed changes

piotrhoppeintel force-pushed the p20-eq-iir branch from 3796c56 to 649d255 Compare October 13, 2026 07:44

lgirdwood approved these changes Oct 13, 2026

View reviewed changes

softwarecki approved these changes Oct 13, 2026

View reviewed changes

lgirdwood merged commit 1061bcd into thesofproject:main Oct 13, 2026
46 checks passed

Conversation

piotrhoppeintel commented Sep 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood Sep 28, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Sep 28, 2026

Uh oh!

softwarecki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

lgirdwood Sep 28, 2026 •

edited

Loading