* Re: Buffers used in ASoC Intel Atom SST driver
2022-08-18 16:03 ` Takashi Iwai
@ 2022-08-18 16:11 ` Pierre-Louis Bossart
2022-08-23 11:56 ` Takashi Iwai
2022-09-11 18:08 ` Hans de Goede
2 siblings, 0 replies; 6+ messages in thread
From: Pierre-Louis Bossart @ 2022-08-18 16:11 UTC (permalink / raw)
To: Takashi Iwai, Cezary Rojewski
Cc: alsa-devel, Kai Vehmanen, Bard Liao, Ranjani Sridharan,
Liam Girdwood, Hans de Goede, Peter Ujfalusi
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;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Buffers used in ASoC Intel Atom SST driver
2022-08-18 16:03 ` Takashi Iwai
2022-08-18 16:11 ` Pierre-Louis Bossart
@ 2022-08-23 11:56 ` Takashi Iwai
2022-09-11 18:08 ` Hans de Goede
2 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2022-08-23 11:56 UTC (permalink / raw)
To: alsa-devel
Cc: Cezary Rojewski, Kai Vehmanen, Peter Ujfalusi,
Pierre-Louis Bossart, Ranjani Sridharan, Liam Girdwood,
Hans de Goede, Bard Liao
On Thu, 18 Aug 2022 18:03:15 +0200,
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.
I guess we can merge and go forward. The changes are very likely
safe, judging from the code, as there is no evidence of DMA address
restriction there. So I'm going to submit the full patchset for
cleanups.
thanks,
Takashi
>
>
> 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;
> }
> --
> 2.35.3
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Buffers used in ASoC Intel Atom SST driver
2022-08-18 16:03 ` Takashi Iwai
2022-08-18 16:11 ` Pierre-Louis Bossart
2022-08-23 11:56 ` Takashi Iwai
@ 2022-09-11 18:08 ` Hans de Goede
2 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2022-09-11 18:08 UTC (permalink / raw)
To: Takashi Iwai, Cezary Rojewski
Cc: alsa-devel, Kai Vehmanen, Peter Ujfalusi, Pierre-Louis Bossart,
Ranjani Sridharan, Liam Girdwood, Bard Liao
Hi,
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.
I know this has already been merged, but still I have just given this
a quick test-spin and it works ok for me.
Regards,
Hans
>
>
> 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;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread