[PATCH 1/2] ASoC: SOF: relax PCM period and buffer size constraints

[PATCH 1/2] ASoC: SOF: relax PCM period and buffer size constraints

[Kai Vehmanen]

Current SOF implementation limits period and buffer sizes to multiples
of period_min. Period_min is defined in topology, but is in practise set
to align with the SOF DSP timer tick (typically 1ms).

While this approach helps user-space to avoid period sizes, which are
not aligned to the DSP timer tick, it causes problems to applications
which want to align data processing size to that of ALSA period size.
One example is JACK audio server, which limits period sizes to power of
two values.

Other ALSA drivers where audio data transfer is driven by a timer tick,
like USB, do not constraint period and buffer sizes to exact multiple of
the timer tick.

To align SOF to follow the same behaviour, drop the additional alignment
constraints. As a side-effect, this patch can cause irregularity to
period wakeup timing. This happens when application chooses settings
which were previously forbidden. For example, if application configures
period size to 2^14 bytes and audio config of S32_LE/2ch/48000Hz, one
period represents 42.667ms of audio. Without this patch, this
configuration is not allowed by SOF. With the patch applied,
configuration is allowed but the wakeups are paced by the DSP timer
tick, which is typically 1ms. Application will see period wakeups with a
42/43/42/43ms repeating pattern.

Both approaches are valid within ALSA context, but relaxing the
constraints is better aligned with existing applications and other ALSA
drivers like USB audio.

BugLink: https://github.com/thesofproject/linux/issues/2391
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/pcm.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
index 0a70e685f826..37d12162e448 100644
--- a/sound/soc/sof/pcm.c
+++ b/sound/soc/sof/pcm.c
@@ -478,17 +478,10 @@ static int sof_pcm_open(struct snd_soc_component *component,
 
 	caps = &spcm->pcm.caps[substream->stream];
 
-	/* set any runtime constraints based on topology */
-	snd_pcm_hw_constraint_step(substream->runtime, 0,
-				   SNDRV_PCM_HW_PARAM_BUFFER_BYTES,
-				   le32_to_cpu(caps->period_size_min));
-	snd_pcm_hw_constraint_step(substream->runtime, 0,
-				   SNDRV_PCM_HW_PARAM_PERIOD_BYTES,
-				   le32_to_cpu(caps->period_size_min));
-
 	/* set runtime config */
 	runtime->hw.info = ops->hw_info; /* platform-specific */
 
+	/* set any runtime constraints based on topology */
 	runtime->hw.formats = le64_to_cpu(caps->formats);
 	runtime->hw.period_bytes_min = le32_to_cpu(caps->period_size_min);
 	runtime->hw.period_bytes_max = le32_to_cpu(caps->period_size_max);
-- 
2.28.0

[PATCH 2/2] ASoC: SOF: Intel: add hw specific PCM constraints

[Kai Vehmanen]

Part of PCM constraints are set based on DSP topology, but rest
should be set based on hardware capabilities. Add PCM constraints
for Intel platforms:

- Add constraint for the period count to be integer. This avoids
  wrap-arounds of the DMA circular buffer in middle of a period.

- Align period size to dword/32bit as per HDA spec.

Both constraints are aligned with current implementation in snd-hda-intel
driver.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/intel/hda-pcm.c   | 7 +++++++
 sound/soc/sof/intel/intel-ipc.c | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/sound/soc/sof/intel/hda-pcm.c b/sound/soc/sof/intel/hda-pcm.c
index b527d5958ae5..5d35bb18660a 100644
--- a/sound/soc/sof/intel/hda-pcm.c
+++ b/sound/soc/sof/intel/hda-pcm.c
@@ -225,6 +225,13 @@ int hda_dsp_pcm_open(struct snd_sof_dev *sdev,
 		return -ENODEV;
 	}
 
+	/* minimum as per HDA spec */
+	snd_pcm_hw_constraint_step(substream->runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 4);
+
+	/* avoid circular buffer wrap in middle of period */
+	snd_pcm_hw_constraint_integer(substream->runtime,
+				      SNDRV_PCM_HW_PARAM_PERIODS);
+
 	/* binding pcm substream to hda stream */
 	substream->runtime->private_data = &dsp_stream->hstream;
 	return 0;
diff --git a/sound/soc/sof/intel/intel-ipc.c b/sound/soc/sof/intel/intel-ipc.c
index 310f9168c124..de66f8a82a07 100644
--- a/sound/soc/sof/intel/intel-ipc.c
+++ b/sound/soc/sof/intel/intel-ipc.c
@@ -73,6 +73,13 @@ int intel_pcm_open(struct snd_sof_dev *sdev,
 	/* binding pcm substream to hda stream */
 	substream->runtime->private_data = stream;
 
+	/* align to DMA minimum transfer size */
+	snd_pcm_hw_constraint_step(substream->runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 4);
+
+	/* avoid circular buffer wrap in middle of period */
+	snd_pcm_hw_constraint_integer(substream->runtime,
+				      SNDRV_PCM_HW_PARAM_PERIODS);
+
 	return 0;
 }
 EXPORT_SYMBOL_NS(intel_pcm_open, SND_SOC_SOF_INTEL_HIFI_EP_IPC);
-- 
2.28.0

Re: [PATCH 1/2] ASoC: SOF: relax PCM period and buffer size constraints

[Mark Brown]

On Wed, 18 Nov 2020 16:05:44 +0200, Kai Vehmanen wrote:
> Current SOF implementation limits period and buffer sizes to multiples
> of period_min. Period_min is defined in topology, but is in practise set
> to align with the SOF DSP timer tick (typically 1ms).
> 
> While this approach helps user-space to avoid period sizes, which are
> not aligned to the DSP timer tick, it causes problems to applications
> which want to align data processing size to that of ALSA period size.
> One example is JACK audio server, which limits period sizes to power of
> two values.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: SOF: relax PCM period and buffer size constraints
      commit: 9983ac49b7db34258facf47439463e96522e1d5a
[2/2] ASoC: SOF: Intel: add hw specific PCM constraints
      commit: caebea04b9125c677e6e747793fbc7fab077727b

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark