Re: [PATCH v2 00/24] ALSA compress / ASoC compress / SOF: Compressed audio support with IPC4

[Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>]

On 9/9/26 11:09, Peter Ujfalusi wrote:
> Hi,
> 
> Changes since v1:
> - Add patch to snd_card_file_add/remove for compress
> - Hanlde the SND_COMPR_TRIGGER_NEXT_TRACK in soc_compr_trigger_fe()
> 
> Cover letter:
> 
> This series adds compressed audio support for SOF with IPC4 firmware.
> Supported formats are PCM, MP3, AAC and VORBIS, the feature set is
> PAUSE_PUSH/RELEASE, DRAIN and position reporting.
> 
> Patches 1-5 are prerequisites in the ALSA compress core and in
> soc-compress.c.
> 
> These are core fixes rather than driver level workarounds on purpose.
> The other users of compressed DPCM have DSP internal ports on their
> BEs, driven by code they own. An Intel/SOF BE ends at a third party
> CODEC driver which is shared with the non DPCM case and has no reason
> to know that the FE is compressed. That set of CODECs is open ended,
> so teaching each of them to cope with a compressed FE does not scale.
> The core has to hand a BE the same substream state whether the FE is
> a PCM or a compressed one.
> 
> Two of these patches change behaviour for the existing users of
> compressed DPCM, so they would need a look from Qualcomm and NXP:
> 
>  - "ASoC: soc-compress: Implement trigger FE-BE sequencing as with
>    normal PCMs" makes the FE/BE trigger order follow
>    dai_link->trigger[], the way dpcm_fe_dai_do_trigger() does it.
>    Neither sound/soc/qcom/ nor sound/soc/fsl/ sets it, so both default
>    to SND_SOC_DPCM_TRIGGER_PRE: START keeps the current order, but
>    STOP, SUSPEND and PAUSE_PUSH now trigger the BE before the FE.
> 
>  - "ASoC: soc-compress: Stop running dpcm on free" sends a STOP trigger
>    from soc_compr_free_fe() if the DPCM is still in START state, which
>    is the case when the last trigger the stream received was a DRAIN.
> 
> Patches 6-20 implement the IPC4 compressed support, patch 21 is a
> Kconfig cleanup. The last two patches enable the feature, in Kconfig
> for TGL and later and in the topology parser, so the compressed device
> type only becomes usable once the code behind it is in place.

It's a rather large and broad patchset. I would break it up in two or
three, you have quite a few cleanup/prerequisite patches that could be
submitted earlier. Then add the ALSA/ASOC changes that might impact
others. And last SOF support for Intel IPC4 platforms when all the
required patches are in.

Otherwise nice job, no major issues, just nit-picks and minor comments.

Thanks Peter!

> Testing:
> 
>  - IPC4: Arrow Lake, compressed playback on speaker and headphone
>    (cs42l43), MP3, AAC and VORBIS.
>  - IPC3: tested by NXP. This does not cover the STOP on free path
>    above, as they do not implement DRAIN.
> 
> Regards,
> Peter
> ---
> Peter Ujfalusi (18):
>   ALSA: compress: pin card module while stream is open
>   ALSA: compress: register the open file with the card
>   ALSA: compress: stop active streams on disconnect
>   ASoC: soc-compress: Provide a runtime for the compressed FE substream
>   ASoC: soc-compress: Implement trigger FE-BE sequencing as with normal
>     PCMs
>   ASoC: soc-compress: Stop running dpcm on free
>   ASoC: SOF: ipc4-pcm: harden pipeline teardown races
>   ASoC: SOF: sof-audio: do not dereference swidget->spipe
>     unconditionally on free
>   ASoC: SOF: ipc4: Add definition of module data in init_ext object type
>   ASoC: SOF: ipc4-topology: Support init_ext_module_data for process
>     modules
>   ASoC: SOF: ipc4-pcm: Make the timestamp info usable outside of
>     ipc4-pcm.c
>   ASoC: SOF: ipc4/ipc4-loader: Add SOF_INFO and CODEC_INFO to
>     fw_config_params
>   ASoC: SOF: ipc4-pcm: Handle COMPR DRAIN triggers as EOS pipeline state
>   ASoC: SOF: ipc4-topology: Set FAST_MODE for host copier in compr mode
>   ASoC: SOF: Add support for IPC4 compressed
>   ASoC: SOF: ipc4: Handle compressed drain done notification from
>     firmware
>   ASoC: SOF: Intel: Kconfig: Remove redundant IPC version selects
>   ASoC: SOF: Intel: Kconfig: Select compress support for TGL+ platforms
> 
> Ranjani Sridharan (6):
>   ASoC: SOF: compress: Rename compress ops with ipc3 prefix
>   ASoC: SOF: sof-audio: Expose a couple of functions
>   ASoC: SOF: pcm: Modify the signature of a couple of PCM IPC ops
>   ASoC: SOF: intel: hda-stream: Clear the current position when
>     releasing stream
>   ASoC: SOF: ops: Add new platform-specific ops for compress
>   ASoC: SOF: topology: Add support for decoder and encoder widgets
> 
>  include/sound/compress_driver.h               |   2 +
>  include/sound/sof/ipc4/header.h               |  24 +-
>  sound/core/compress_offload.c                 |  86 +-
>  sound/soc/soc-compress.c                      | 200 ++++-
>  sound/soc/sof/Makefile                        |   4 +-
>  sound/soc/sof/core.c                          |  10 +-
>  sound/soc/sof/intel/Kconfig                   |   9 +-
>  sound/soc/sof/intel/hda-common-ops.c          |   8 +
>  sound/soc/sof/intel/hda-pcm.c                 | 142 ++++
>  sound/soc/sof/intel/hda-stream.c              |  70 +-
>  sound/soc/sof/intel/hda.h                     |  15 +
>  sound/soc/sof/{compress.c => ipc3-compress.c} | 145 +---
>  sound/soc/sof/ipc3-pcm.c                      |  29 +-
>  sound/soc/sof/ipc3-priv.h                     |   3 +
>  sound/soc/sof/ipc4-compress.c                 | 776 ++++++++++++++++++
>  sound/soc/sof/ipc4-loader.c                   |  51 ++
>  sound/soc/sof/ipc4-pcm.c                      | 125 +--
>  sound/soc/sof/ipc4-priv.h                     |  45 +
>  sound/soc/sof/ipc4-topology.c                 | 115 ++-
>  sound/soc/sof/ipc4-topology.h                 |   9 +
>  sound/soc/sof/ipc4.c                          |  13 +-
>  sound/soc/sof/ops.h                           |  74 ++
>  sound/soc/sof/pcm.c                           |  18 +-
>  sound/soc/sof/sof-audio.c                     |  89 +-
>  sound/soc/sof/sof-audio.h                     |  23 +-
>  sound/soc/sof/sof-priv.h                      |  12 +
>  sound/soc/sof/topology.c                      |   2 +
>  27 files changed, 1790 insertions(+), 309 deletions(-)
>  rename sound/soc/sof/{compress.c => ipc3-compress.c} (66%)
>  create mode 100644 sound/soc/sof/ipc4-compress.c
>