Compressed offload support for IPC4 #10492

ranj063 · 2026-01-21T04:26:20Z

This PR is part 1 of the compressed offload support which includes splitting up the cadence implementation for IPC3 and IPC4 along with the required patches for setting a DP module's ring buffer size based on what the input buffer size requirements for the codec are and the init data required for module init. The topology patches showing the compressed path will be included in the next part

Add a global ID for module specific init data in the extended init data section and update the decode function to save the data in the module's init_data. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

When a modules advertises the input and output buffer requirements for processing, use that to determine the size of the secondary buffer when attaching them to buffers interfacing with DP modules. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Copilot

Pull request overview

This PR adds compressed offload support for IPC4 by refactoring the Cadence codec implementation. It splits the existing implementation into IPC3 and IPC4-specific variants while keeping common functionality shared.

Changes:

Split Cadence codec implementation into IPC3 (cadence_ipc3.c) and IPC4 (cadence_ipc4.c) specific files
Modified ring buffer creation to account for module-specific buffer size requirements
Added support for module initialization data in IPC4

Reviewed changes

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

Show a summary per file

File	Description
src/ipc/ipc4/helper.c	Updated ring buffer size calculation to use MAX of default sizes and module-specific buffer requirements
src/include/sof/audio/module_adapter/module/cadence.h	Exposed common Cadence codec functions, added IPC4-specific structures, moved API ID enum from .c file
src/include/sof/audio/cadence/mp3_enc/xa_mp3_enc_api.h	Added new MP3 encoder API header with configuration parameters and error codes
src/include/ipc4/module.h	Added MODULE_DATA initialization data type to support codec-specific init data
src/audio/module_adapter/module_adapter_ipc4.c	Added handling for MODULE_DATA initialization data type
src/audio/module_adapter/module/cadence_ipc4.c	New IPC4-specific Cadence codec implementation with DP-aware processing
src/audio/module_adapter/module/cadence_ipc3.c	Extracted IPC3-specific Cadence codec implementation
src/audio/module_adapter/module/cadence.c	Refactored to contain only common codec functionality shared between IPC3/IPC4
src/audio/module_adapter/CMakeLists.txt	Updated build configuration to compile IPC-specific implementations

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

Copilot · 2026-01-21T04:27:15Z

src/audio/module_adapter/module/cadence_ipc4.c

+		cd->direction = *((uint32_t *)codec->cfg.init_data +
+					sizeof(struct snd_codec) / sizeof(uint32_t));


Pointer arithmetic error: adding the result of sizeof(struct snd_codec) / sizeof(uint32_t) to a uint32_t* pointer will skip that many uint32_t elements (multiplying by sizeof(uint32_t) again). This should be cast to uint8_t* first, then offset by sizeof(struct snd_codec), then cast back to uint32_t* and dereferenced.

Suggested change

cd->direction = *((uint32_t *)codec->cfg.init_data +

sizeof(struct snd_codec) / sizeof(uint32_t));

uint8_t *init_bytes = (uint8_t *)codec->cfg.init_data;

cd->direction = *(uint32_t *)(init_bytes + sizeof(struct snd_codec));

suggestion looks cleaner by getting init data uint8 *

Copilot · 2026-01-21T04:27:15Z

src/audio/module_adapter/module/cadence_ipc4.c

+{
+	struct cadence_codec_data *cd = module_get_private_data(mod);
+	struct comp_dev *dev = mod->dev;
+	int word_size = 16;


The hardcoded word size value 16 is used here but then validated against audio_fmt.depth in the switch statement below. Consider using a constant definition or deriving the value from the configuration to make the relationship clearer.

ranj063 · 2026-01-21T17:11:59Z

@dbaluta would you be able to give this PR a try to make sure IPC3 compressed support is still good? On the kernel side, the split was cleaner but I had to retain the common parts in this PR for IPC3 and IPC4. Thanks!

dbaluta · 2026-01-22T11:45:54Z

@ranj063 I get the following compile time errors:

home/daniel/work/sof_dir4/sof/src/audio/module_adapter/module/cadence_ipc3.c: In function 'cadence_codec_resolve_api':
/home/daniel/work/sof_dir4/sof/src/audio/module_adapter/module/cadence_ipc3.c:25:26: error: storage size of 'codec_params' isn't known
   25 |         struct snd_codec codec_params;                                                                                                                                                
      |                          ^~~~~~~~~~~~                                                                                                                                                 
/home/daniel/work/sof_dir4/sof/src/audio/module_adapter/module/cadence_ipc3.c:25:26: warning: unused variable 'codec_params' [-Wunused-variable]
/home/daniel/work/sof_dir4/sof/src/audio/module_adapter/module/cadence_ipc3.c: In function 'cadence_codec_process':
/home/daniel/work/sof_dir4/sof/src/audio/module_adapter/module/cadence_ipc3.c:221:36: warning: unused variable 'cd' [-Wunused-variable]
  221 |         struct cadence_codec_data *cd = codec->private;
      |                                    ^~
[174/335] Building C object modules/sof/CMakeFiles/modules_sof.dir/home/daniel/work/sof_dir4/sof/src/audio/selector/selector.c.obj

Looks like you need to add the following fix:

diff --git a/src/audio/module_adapter/module/cadence_ipc3.c b/src/audio/module_adapter/module/cadence_ipc3.c
index 2bafe766d..77b5b0276 100644
--- a/src/audio/module_adapter/module/cadence_ipc3.c
+++ b/src/audio/module_adapter/module/cadence_ipc3.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include <ipc/compress_params.h>
 #include <sof/audio/module_adapter/module/generic.h>
 #include <sof/audio/module_adapter/module/cadence.h>
 #include <rtos/init.h>
@@ -218,7 +219,6 @@ cadence_codec_process(struct processing_module *mod,
        struct comp_buffer *local_buff;
        struct comp_dev *dev = mod->dev;
        struct module_data *codec = &mod->priv;
-       struct cadence_codec_data *cd = codec->private;
        int free_bytes, output_bytes = cadence_codec_get_samples(mod) *
                                mod->stream_params->sample_container_bytes *
                                mod->stream_params->channels;

Otherwise it works fine.

wjablon1 · 2026-01-22T13:02:51Z

src/audio/module_adapter/module_adapter_ipc4.c

+		{
+			/* set the module init_data */
+			dst->init_data = (const void *)(obj + 1);
+			dst->avail = true;


Did you verify that dst->avail is set to false before the function call (e.g. through memset)? I.e are we prepared for a situation where item "IPC4_MOD_INIT_DATA_ID_MODULE_DATA" is not present for some reason?

@wjablon1 module_ext_init_decode() is called right after we receive the module init IPC for a module. And when we receive the IPC, "mod" data for the module is allocated right then and it is cleared to 0 with a memset. Is that what you're asking?

yep. If mod->priv.cfg is memseted to zeros prior to module_adapter_init_data, I don't see an issue here. Otherwise I would manually set dst->avail to false before module_ext_init_decode.

Split the implementation for IPC3 for the cadence decoder and add a new implementation for IPC4. The IPC4 implementation assumes that the codec info is passed to the firmware during the module init IPC as module specific data. The implementation includes support for MP3 decoder, MP3 encoder and AAC decoder for the time being. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

lyakh · 2026-01-21T08:26:57Z

src/audio/module_adapter/module_adapter_ipc4.c

+		case IPC4_MOD_INIT_DATA_ID_MODULE_DATA:
+		{
+			/* set the module init_data */
+			dst->init_data = (const void *)(obj + 1);


isn't the obj pointer in the IPC buffer, which will be overwritten with the next IPC?

This also caught my attention but then I noticed that the init_data pointer is set back to NULL after initialization in module_adapter_reset_data. Probably we should also set dst->avail to false in the module_adapter_reset_data.

@lyakh the idea for this is to just pass the init data pointer to the module-specific init and it is up to the module to take it or not. For example, in the case of the cadence decoder, we save this data which contains the codec params requested by the kernel like here

sof/src/audio/module_adapter/module/cadence_ipc3.c

Line 76 in 721418e

ret = memcpy_s(setup_cfg->data, setup_cfg->size,

we should probably call this out in the comments that clients must copy if they need to retain the data after IPC

...or maybe we can move that = NULL into this or the calling function and remove module_adapter_reset_data() completely

My votes to set the init_data pointer to NULL after module init is complete and we return from IPC handling. If this cannot be done cleanly, then I think we should do a copy of the data. Otherwise this is like a trap waiting to spring for module developers.

lgirdwood

just some minor opens from me.

lgirdwood · 2026-01-23T17:02:33Z

src/audio/module_adapter/module_adapter_ipc4.c

+		case IPC4_MOD_INIT_DATA_ID_MODULE_DATA:
+		{
+			/* set the module init_data */
+			dst->init_data = (const void *)(obj + 1);


we should probably call this out in the comments that clients must copy if they need to retain the data after IPC

lgirdwood · 2026-01-23T17:16:23Z

src/audio/module_adapter/module/cadence_ipc4.c

+		cd->direction = *((uint32_t *)codec->cfg.init_data +
+					sizeof(struct snd_codec) / sizeof(uint32_t));


suggestion looks cleaner by getting init data uint8 *

kv2019i

Nothing major, but a few questions inline.

kv2019i · 2026-01-26T09:00:32Z

src/audio/module_adapter/module_adapter_ipc4.c

+		case IPC4_MOD_INIT_DATA_ID_MODULE_DATA:
+		{
+			/* set the module init_data */
+			dst->init_data = (const void *)(obj + 1);


My votes to set the init_data pointer to NULL after module init is complete and we return from IPC handling. If this cannot be done cleanly, then I think we should do a copy of the data. Otherwise this is like a trap waiting to spring for module developers.

kv2019i · 2026-01-26T09:21:31Z

src/ipc/ipc4/helper.c

+		ring_buffer = ring_buffer_create(dp, MAX(source_get_min_available(src),
+							 dst_module_data->mpd.in_buff_size),
+						 MAX(sink_get_min_free_space(snk),
+						     src_module_data->mpd.out_buff_size),


Could you explain why this is needed, perhaps with some example? I thought module should advertise its requirements via the sink/source API, so tha tsource_get_min_available() and sink_get_min_free_space() would require bigger values. Directly accessing mpd.in/out_buff_size seems a bit like punching holes through the sink/source (see logic above on L623 and above). I could be missing something, but would be good to record the logic a bit more in the git commit to help the next person that needs to modify this.

kv2019i · 2026-01-26T09:27:39Z

src/audio/module_adapter/module/cadence_ipc4.c

+	case CADENCE_CODEC_AAC_DEC_ID:
+		return cadence_configure_aac_dec_params(mod);
+	default:
+		break;


Would be good return an error if unsupported "api_id" is used.

ranj063 added 2 commits January 20, 2026 20:15

module: Add support for module-specific init data

bba6e8d

Add a global ID for module specific init data in the extended init data section and update the decode function to save the data in the module's init_data. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Copilot AI review requested due to automatic review settings January 21, 2026 04:26

ranj063 requested review from RanderWang, dbaluta, jxstelter, kv2019i, lbetlej, lgirdwood, marcinszkudlinski, mmaka1, pblaszko and plbossart as code owners January 21, 2026 04:26

Copilot AI reviewed Jan 21, 2026

View reviewed changes

wjablon1 reviewed Jan 22, 2026

View reviewed changes

ranj063 force-pushed the topic/ipc4_compress_part1 branch from d137da9 to 721418e Compare January 22, 2026 16:51

lyakh reviewed Jan 23, 2026

View reviewed changes

lgirdwood reviewed Jan 23, 2026

View reviewed changes

kv2019i requested changes Jan 26, 2026

View reviewed changes

		cd->direction = ((uint32_t )codec->cfg.init_data +
		sizeof(struct snd_codec) / sizeof(uint32_t));

Compressed offload support for IPC4 #10492

Are you sure you want to change the base?

Compressed offload support for IPC4 #10492

Conversation

ranj063 commented Jan 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

ranj063 commented Jan 21, 2026

Uh oh!

dbaluta commented Jan 22, 2026

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!

ranj063 Jan 23, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood 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!

kv2019i 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ranj063 Jan 23, 2026 •

edited

Loading