Re: [PATCH v4 22/26] ASoC: SOF: Add support for IPC4 compressed

["Péter Ujfalusi" <peter.ujfalusi@linux.intel.com>]

On 06/10/2026 12:37, Mark Brown wrote:
> On Wed, Sep 16, 2026 at 03:03:16PM +0300, Peter Ujfalusi wrote:
>> 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.
> 
>> +static int sof_ipc4_compr_alloc_pages(struct device *dev,
>> +				      struct snd_sof_pcm_stream *sps,
>> +				      struct snd_soc_component *component,
>> +				      struct snd_compr_stream *cstream)
>> +{
> 
>> +	ret = snd_compr_malloc_pages(cstream, crtd->buffer_size);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = snd_sof_compr_create_page_table(component, cstream, crtd->dma_area,
>> +					      crtd->dma_bytes);
>> +	if (ret < 0)
>> +		snd_compr_free_pages(cstream);
> 
> sof_dai_load() allocates 4k for page tables, the limits we have here
> allow for say 64 128k fragments which gives an 8M buffer that on a
> system with 4k pages is going to make more than 4k of PFNs.

True, and this affects the existing IPC3 as well, which does not even
have get_caps callback defined.
I will add a check in snd_sof_create_page_table() and update the ipc4's
get_caps.

> 
>> +static int sof_ipc4_compr_trigger(struct snd_soc_component *component,
>> +				  struct snd_compr_stream *cstream, int cmd)
>> +{
> 
>> +	switch (cmd) {
>> +	case SNDRV_PCM_TRIGGER_START:
>> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>> +		trigger_platform = true;
>> +		break;
>> +	case SNDRV_PCM_TRIGGER_STOP:
>> +	case SNDRV_PCM_TRIGGER_SUSPEND:
>> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>> +		break;
> 
> Do we not need to do something to clean up/reset DMA on STOP?

For IPC4 we need to keep DMA running and stop it later if needed.
Let me see if I'm missing something, but this should (and by tests) be OK.

>> +void sof_ipc4_compr_drain_done(struct snd_sof_dev *sdev, void *ipc_message)
>> +{
> 
>> +	spcm_dbg(spcm, dir, "Entry: EOS done\n");
>> +
>> +	if (spcm->stream[dir].cstream)
>> +		snd_compr_drain_notify(spcm->stream[dir].cstream);
>> +}
> 
> More of an issue further up the stack but snd_compr_drain() triggers the
> drain, then DRAINING is set later in snd_compress_wait_for_drain() and
> there's therefore a window where the DSP could reply to us that the
> drain finished before that happens.

Right, that is true as well. It affects all compress drivers, the core
should set the state for draining before it calls the trigger to be
safe. And roll back in case the trigger fails.

-- 
Péter