| Message ID | 20200330175210.47518-1-stephan@gerhold.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7f2430cda819a9ecb1df5a0f3ef4f1c20db3f811 |
| Headers | show |
| Series | ASoC: qcom: q6asm-dai: Add SNDRV_PCM_INFO_BATCH flag | expand |
On 3/30/20 7:52 PM, Stephan Gerhold wrote: > At the moment, playing audio with PulseAudio with the qdsp6 driver > results in distorted sound. It seems like its timer-based scheduling > does not work properly with qdsp6 since setting tsched=0 in > the PulseAudio configuration avoids the issue. > > Apparently this happens when the pointer() callback is not accurate > enough. There is a SNDRV_PCM_INFO_BATCH flag that can be used to stop > PulseAudio from using timer-based scheduling by default. > > According to https://www.alsa-project.org/pipermail/alsa-devel/2014-March/073816.html: > > The flag is being used in the sense explained in the previous audio > meeting -- the data transfer granularity isn't fine enough but aligned > to the period size (or less). > > q6asm-dai reports the position as multiple of > > prtd->pcm_count = snd_pcm_lib_period_bytes(substream) > > so it indeed just a multiple of the period size. > > Therefore adding the flag here seems appropriate and makes audio > work out of the box. > > Fixes: 2a9e92d371db ("ASoC: qdsp6: q6asm: Add q6asm dai driver") > Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- > I'm still quite confused about the true meaning of SNDRV_PCM_INFO_BATCH, > so please correct me if I'm wrong :) The meaning might have changed over the years, but the way it is used right now is that it means that the position pointer has limited granularity. With 'limited' being a bit fuzzy, but typically means that the granularity is worse than a few samples. This driver definitely falls into the limited category as the granularity seems to be period size. - Lars
On 3/30/20 1:15 PM, Lars-Peter Clausen wrote: > On 3/30/20 7:52 PM, Stephan Gerhold wrote: >> At the moment, playing audio with PulseAudio with the qdsp6 driver >> results in distorted sound. It seems like its timer-based scheduling >> does not work properly with qdsp6 since setting tsched=0 in >> the PulseAudio configuration avoids the issue. >> >> Apparently this happens when the pointer() callback is not accurate >> enough. There is a SNDRV_PCM_INFO_BATCH flag that can be used to stop >> PulseAudio from using timer-based scheduling by default. >> >> According to >> https://www.alsa-project.org/pipermail/alsa-devel/2014-March/073816.html: >> >> The flag is being used in the sense explained in the previous audio >> meeting -- the data transfer granularity isn't fine enough but >> aligned >> to the period size (or less). >> >> q6asm-dai reports the position as multiple of >> >> prtd->pcm_count = snd_pcm_lib_period_bytes(substream) >> >> so it indeed just a multiple of the period size. >> >> Therefore adding the flag here seems appropriate and makes audio >> work out of the box. >> >> Fixes: 2a9e92d371db ("ASoC: qdsp6: q6asm: Add q6asm dai driver") >> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> Signed-off-by: Stephan Gerhold <stephan@gerhold.net> >> --- >> I'm still quite confused about the true meaning of SNDRV_PCM_INFO_BATCH, >> so please correct me if I'm wrong :) > > The meaning might have changed over the years, but the way it is used > right now is that it means that the position pointer has limited > granularity. With 'limited' being a bit fuzzy, but typically means that > the granularity is worse than a few samples. > > This driver definitely falls into the limited category as the > granularity seems to be period size. Agree, we added this INFO_BATCH flag for SOF Broadwell and Baytrail platforms as well for the same reason of large granularity.
Thanks Stephan for finding this flag! On 30/03/2020 18:52, Stephan Gerhold wrote: > At the moment, playing audio with PulseAudio with the qdsp6 driver > results in distorted sound. It seems like its timer-based scheduling > does not work properly with qdsp6 since setting tsched=0 in > the PulseAudio configuration avoids the issue. > > Apparently this happens when the pointer() callback is not accurate > enough. There is a SNDRV_PCM_INFO_BATCH flag that can be used to stop > PulseAudio from using timer-based scheduling by default. > > According to https://www.alsa-project.org/pipermail/alsa-devel/2014-March/073816.html: > > The flag is being used in the sense explained in the previous audio > meeting -- the data transfer granularity isn't fine enough but aligned > to the period size (or less). > > q6asm-dai reports the position as multiple of > > prtd->pcm_count = snd_pcm_lib_period_bytes(substream) > > so it indeed just a multiple of the period size. > > Therefore adding the flag here seems appropriate and makes audio > work out of the box. > > Fixes: 2a9e92d371db ("ASoC: qdsp6: q6asm: Add q6asm dai driver") > Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c index f6c7cddf08e8..125af00bba53 100644 --- a/sound/soc/qcom/qdsp6/q6asm-dai.c +++ b/sound/soc/qcom/qdsp6/q6asm-dai.c @@ -78,7 +78,7 @@ struct q6asm_dai_data { }; static const struct snd_pcm_hardware q6asm_dai_hardware_capture = { - .info = (SNDRV_PCM_INFO_MMAP | + .info = (SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_BATCH | SNDRV_PCM_INFO_BLOCK_TRANSFER | SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_INTERLEAVED | @@ -100,7 +100,7 @@ static const struct snd_pcm_hardware q6asm_dai_hardware_capture = { }; static struct snd_pcm_hardware q6asm_dai_hardware_playback = { - .info = (SNDRV_PCM_INFO_MMAP | + .info = (SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_BATCH | SNDRV_PCM_INFO_BLOCK_TRANSFER | SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_INTERLEAVED |
At the moment, playing audio with PulseAudio with the qdsp6 driver results in distorted sound. It seems like its timer-based scheduling does not work properly with qdsp6 since setting tsched=0 in the PulseAudio configuration avoids the issue. Apparently this happens when the pointer() callback is not accurate enough. There is a SNDRV_PCM_INFO_BATCH flag that can be used to stop PulseAudio from using timer-based scheduling by default. According to https://www.alsa-project.org/pipermail/alsa-devel/2014-March/073816.html: The flag is being used in the sense explained in the previous audio meeting -- the data transfer granularity isn't fine enough but aligned to the period size (or less). q6asm-dai reports the position as multiple of prtd->pcm_count = snd_pcm_lib_period_bytes(substream) so it indeed just a multiple of the period size. Therefore adding the flag here seems appropriate and makes audio work out of the box. Fixes: 2a9e92d371db ("ASoC: qdsp6: q6asm: Add q6asm dai driver") Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- I'm still quite confused about the true meaning of SNDRV_PCM_INFO_BATCH, so please correct me if I'm wrong :) The tsched=0 workaround can be found in Linaro distributions for QCOM devices for example: - https://git.linaro.org/ci/fai.git/commit/?id=63494268b654d80df033f4cdeccf8f115801b756 - https://github.com/ndechesne/meta-qcom/commit/7035dfeadd1c434fc7613730ac38004670553ec0 This patch allows removing that workaround since audio then works without any configuration changes. --- sound/soc/qcom/qdsp6/q6asm-dai.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)