Re: Buffers used in ASoC Intel Atom SST driver

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

On 8/18/22 18:03, Takashi Iwai wrote:
> On Thu, 18 Aug 2022 17:59:36 +0200,
> Cezary Rojewski wrote:
>>
>> On 2022-08-18 5:42 PM, Takashi Iwai wrote:
>>> Hi,
>>>
>>> while cleaning up / fixing the memory allocation code, I noticed that
>>> Intel SST driver is the only driver that currently uses the
>>> SNDRV_DMA_TYPE_CONTINUOUS pages for its PCM buffer.
>>>
>>> Is there any reason not to use the standard device buffer
>>> (SNDRV_DMA_TYPE_DEV)?  i.e. the change like below.
>>
>>
>> Hello,
>>
>> Does not hurt to check the change out. I'm not aware of such
>> limitation for any of Intel's AudioDSP solutions. I cannot test this
>> change though as ATOM devices are not part of my CI. Pierre, do you
>> have any devices where this change could be tested?
>>
>> Adding Hans as he is the key intel/atom maintainer too and I'm pretty
>> sure he has few devices on his own.
> 
> Oh yeah, thanks, Hans might be able to check.
> FWIW, the patch I asked is like below.


It's a 2011 contribution, commit d927fdae5cb2ca, probably from the
Medfield days. I doubt anyone in the Intel CC: list has context on this,
I was the only one on the Intel payroll at the time.

I vaguely remember than in early versions the PCM data was copied over
to the DSP SRAM with the .ack callback, it wasn't a straightforward DMA
transfer, and that created all kinds of issues for PulseAudio integration.

This was changed at some point IIRC, I don't see any reason why a
bog-standard DMA buffer wouldn't work with the released firmware for
Baytrail which is two generations newer and does use the DesignWare DMA.

The best would be to try, I can't volunteer since I don't have any
hardware available at the  moment.


> 
> 
> Takashi
> 
> -- 8< --
> 
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ASoC: Intel: sst: Switch to standard device pages
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/soc/intel/atom/sst-mfld-platform-pcm.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/intel/atom/sst-mfld-platform-pcm.c b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
> index a56dd48c045f..c75616a5fd0a 100644
> --- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
> +++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
> @@ -676,10 +676,9 @@ static int sst_soc_pcm_new(struct snd_soc_component *component,
>  
>  	if (dai->driver->playback.channels_min ||
>  			dai->driver->capture.channels_min) {
> -		snd_pcm_set_managed_buffer_all(pcm,
> -			SNDRV_DMA_TYPE_CONTINUOUS,
> -			snd_dma_continuous_data(GFP_DMA),
> -			SST_MIN_BUFFER, SST_MAX_BUFFER);
> +		snd_pcm_set_managed_buffer_all(pcm, SNDRV_DMA_TYPE_DEV,
> +					       pcm->card->dev,
> +					       SST_MIN_BUFFER, SST_MAX_BUFFER);
>  	}
>  	return 0;
>  }