ASoC: SOF: Initial support for IPC4 compress offload#5647
Conversation
|
I don't see these errors with |
cea54e7 to
c3f4c47
Compare
|
Will test this on IMX platforms, but we are only using IPC3 so not sure how much it helps. I'm not working on moving IMX to IPC4 but it will take a while. Will be back with the results asap. |
@dbaluta, we want to make sure that we don't break iMX (IPC3) and we are on reverse grounds that we can only test the new IPC4 support.
Thank you! |
lgirdwood
left a comment
There was a problem hiding this comment.
LGTM, some minor opens, some rebasing/squashing probably needed.
|
@ujfalusi Tested this today with sof-dev branch and I get some problems but could be unrelated to the change. Need to check if we have some internal patches not upstreamed. |
|
@ujfalusi Tested the patches with IMX8MP using IPC3 compress implementation with MP3 and AAC and it works fine. I will add my comments to the code later. |
228cd7c to
a57957e
Compare
|
Changes since v2:
|
52d57d7 to
f992ff0
Compare
|
Changes since v3:
|
f992ff0 to
f16f40d
Compare
|
Changes since v4: It is a surprise that this did not blow up, but it somehow managed to 'work', just that the min_fragment_size turned 0 after the compr_open (as we used the data for read only). Added a helper to calculate on spot the minimal fragment size instead, it is used only during open operation. |
f16f40d to
fadeee4
Compare
|
Changes since v5:
|
11cb387 to
e22d68e
Compare
|
Changes since v6:
There is not yet corresponding firmware PR, the WIP branch for testing is https://github.com/ujfalusi/sof/commits/peter/topic/ipc4_compr |
e22d68e to
bc8f6ba
Compare
|
Changes since v7:
With this version end of file drain works correctly without errors from BE. |
|
Changes since v8:
|
I need to move patches around and squash them, also aligning on the param ids in firmware config, plus the notification magic for EOS completion. |
f21f5f1 to
b7122bb
Compare
| err = snd_sof_compr_platform_close(sdev, cstream); | ||
| if (err < 0) { | ||
| spcm_err(spcm, cstream->direction, | ||
| "platform compress close failed %d\n", ret); | ||
| if (!ret) | ||
| ret = err; | ||
| } |
There was a problem hiding this comment.
correct, will fix it
| if (!fe->fe_compr) { | ||
| be_substream->runtime = fe_substream->runtime; | ||
| } else { | ||
| be_substream->runtime = kzalloc(sizeof(*be_substream->runtime), GFP_KERNEL); | ||
| if (!be_substream->runtime) { | ||
| err = -ENOMEM; | ||
| goto unwind; | ||
| } | ||
| } |
Take a module reference in snd_compr_open() and release it in snd_compr_free(). This pins the card driver module for the lifetime of an open compress stream and prevents card removal while the stream file is still in use. Adjust the open() cleanup paths to drop the added module reference only when it was acquired, and keep release ordering safe by dropping the module reference before freeing stream data. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Track open compressed streams per device so disconnect can stop active streams and wake waiters before snd_unregister_device(). This aligns compressed stream teardown with PCM disconnect behavior and prevents active userspace streams from running into unregister races. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
With DPCM when compr is used on FE side, the BE is still running as 'normal' PCM. It is expected that the be_substream->runtime on the BE is not NULL, for example some codec drivers expect to have the substream->runtime valid, to store configuration for example. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
… PCMs The FE-BE trigger sequence should be dynamic, similarly how soc-pcm.c dpcm_fe_dai_do_trigger() does it. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
If the last trigger that the compr device received is a DRAIN then the DPCM is left in running state (no stop trigger is sent). Before we execute the free we need to send a STOP trigger to make sure that both BE and FE is in expected state and prepared for closing. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
In preparation for adding support for compressed offload support for IPC4, rename the current compress implementation with the IPC3 prefix. Introduce a new field in struct sof_ipc_pcm_ops to save the IPC-specific compressed ops pointer. This should be set when the component driver ops are assigned during SOF device probe. Expose a couple of common functions that will be used by both IPC-specific implementations and rename the compress.c file to ipc3-compress.c Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Serialize trigger/free with pipeline_state_mutex and validate pipeline entries before use. Also clear pipeline_list->count when freeing lists to avoid stale entries during concurrent teardown. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
During widget FREE traversal, SOF walks DAPM sink paths recursively while widgets and paths can be torn down. This can lead to stale pointer usage in sof_free_widgets_in_path() when path entries disappear during recursion. Harden the FREE path by: - validating scheduler/pipeline pointers before recursive free - using a safe DAPM path iterator for sink traversal - carrying a stable widget-list snapshot through recursion - skipping NULL sink edges during traversal The safe traversal can leave path->walking set on surviving edges after FREE, which may short-circuit later DAPM walks and break consecutive playback open/stop cycles. Reset walking flags for widgets in the current DAPM list after FREE walks (including error paths) to keep subsequent traversals clean. This keeps teardown robust for module-remove race scenarios while preserving normal consecutive playback behavior. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
These are common functions that will also be needed for the IPC4 compressed support. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
In order to reuse the pipeline triggering logic for compressed support with IPC4, modify the signature of the trigger and hw_free PCM IPC ops so that they can be reused. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
…ng stream After the host DMA ID is released, reset the curr_pos to 0 for a clean start for subsequent stream starts. This is not needed for PCM streams but for compressed streams. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Add new ops in the struct snd_sof_ops for platform-specific ops for compresssed streams. Also, define and set them for the HDA platforms. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Co-developed-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The SOF_IPC4_MOD_INIT_DATA_ID_MODULE_DATA type within the module_init_ext area is module specific init data. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
…dules Add support for handling init_ext_module_data for process modules, which is going to be used by decoder and encoder type of process modules. The support is generic and it can be extended to other type of process modules or other module types than process with a small update of sof_ipc4_add_init_ext_module_data() function. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
…cm.c The support for compressed stream will also need to have access to the same information which is used for delay reporting for DAI data progression tracking. Make the necessary struct and functions to be available and premare them to be called without a valid substream. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
…_params SOF_INFO (id == 35) tuple holds tuple structured information about SOF features. The first entry in SOF_INFO is the SOF_CODEC_INFO (id == 0) which contains information about the supported codecs for decode/encode in the booted firmware. If present in the fw_config payload, make a copy of it and store it sof_ipc4_fw_data->codec_info to be used by the compressed code. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The DRAIN trigger is received by a compr device when user space wrote all the data to the buffer and it is waiting for the decoding to be completed. Set the pipeline state to EOS in firmware so it can expect the stream to be stopping anytime soon. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
FAST_MODE allows the host DMA to work in opportunistic, free running mode, which matches with the bitstream nature of compressed devices. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Set and define the compressed ops for IPC4. The initial implementation supports basic features: PAUSE PUSH/RELEASE, DRAIN and progress reporting. Tested with PCM, MP3, AAC and VORBIS codec. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Co-developed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
The decoder module sends a drain done notification when the last chunk of the stream after the EOS from host has been decoded. The notification is a module notification with 0xc0c0 as magic number in event_id upper 16 bit. Call sof_ipc4_compr_drain_done() when the notification arrives to handle it. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
There is no need to select IPC3 and IPC4 along with INTEL_CNL as INTEL_CNL selects both. Similarly, INTEL_MTL selects IPC4, so there is no need to do that for LNL, PTL and NVL. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Select SOF_COMPRESS for TGL and newer platforms, on Intel devices the compressed support is available with IPC4 only. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Decoder and encoder modules fall under process modules in SOF. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Co-developed-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
|
Changes since v15:
|
444e9f2 to
a844a88
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
sound/soc/sof/ipc4-compress.c:172
- The error message logs the wrong return value: snd_sof_compr_platform_close() returns in 'err', but the log prints 'ret' (from sof_ipc4_compr_stream_free()). This makes failures harder to diagnose.
if (err < 0) {
spcm_err(spcm, cstream->direction,
"platform compress close failed %d\n", ret);
if (!ret)
| } else { | ||
| be_substream->runtime = kzalloc(sizeof(*be_substream->runtime), GFP_KERNEL); | ||
| if (!be_substream->runtime) { | ||
| err = -ENOMEM; | ||
| goto unwind; | ||
| } | ||
| } |
| if (err < 0) { | ||
| be->dpcm[stream].users--; |
| if (crtd->buffer_size < SOF_IPC4_COMPR_MIN_BUFFER_SIZE(min_fragment_size)) { | ||
| dev_err(dev, "%s: Too small buffer size %llu (minimum is %u)\n", | ||
| __func__, crtd->buffer_size, | ||
| SOF_IPC4_COMPR_MIN_BUFFER_SIZE(min_fragment_size)); | ||
| return -EINVAL; | ||
| } | ||
|
|
||
| do_div(fragments, crtd->fragment_size); | ||
| if (fragments < SOF_IPC4_COMPR_MIN_FRAGMENTS) { | ||
| dev_err(dev, | ||
| "%s: Insufficient amount of fragments: %llu (minimum is %d)\n", | ||
| __func__, fragments, SOF_IPC4_COMPR_MIN_FRAGMENTS); | ||
| return -EINVAL; | ||
| } |
There was a problem hiding this comment.
OK, I'll add check for the number of fragments against max, also the minimum fragment size can be validated, I will add these, maximum size is not limited actually, the caps provides sensible limits to user space.
Hi,
Support for compress offload with IPC4 on Intel platforms (MTL+).
This initial version supports basic features, like PAUSE PUSH/RELEASE, DRAIN, progress reporting.
Tested with PCM, MP3, AAC and VORBIS codecs so far with firmware built from the following repo (will be making it's way to sof:main): https://github.com/ujfalusi/sof/commits/peter/topic/ipc4_compr
@dbaluta, if you could be able to test that, we would really appreciate it.
firmware PR (without topology changes): thesofproject/sof#10492 (merged), thesofproject/sof#10534 (open)