Re: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board

From: Pierre-Louis Bossart
Date: Wed Aug 12 2020 - 10:46:46 EST



After doing some experiments, I think I can identify the problem more precisely.
1. aplay can not reproduce this issue because it writes samples
immediately when there are some space in the buffer. However, you can
add --test-position to see how the delay grows with period size 256.
aplay -Dhw:1,0 --period-size=256 --buffer-size=480 /dev/zero -d 1 -f dat --test-position
Playing raw data '/dev/zero' : Signed 16 bit Little Endian, Rate 48000
Hz, Stereo
Suspicious buffer position (1 total): avail = 0, delay = 2064, buffer = 512
Suspicious buffer position (2 total): avail = 0, delay = 2064, buffer = 512
Suspicious buffer position (3 total): avail = 0, delay = 2096, buffer = 512
...

Isn't this about the alignment of the buffer size against the period
size, not the period size itself? i.e. in the example above, the
buffer size isn't a multiple of period size, and DSP can't handle if
the position overlaps the buffer size in a half way.

If that's the problem (and it's an oft-seen restriction), the right
constraint is
snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);


Takashi
Oh sorry for my typo. The issue happens no matter what buffer size is
set. Actually, even if I want to set 480, it will change to 512
automatically.
Suspicious buffer position (1 total): avail = 0, delay = 2064, buffer
= 512 <-this one is the buffer size

OK, then it means that the buffer size alignment is already in place.

And this large delay won't happen if you use period size 240?


Takashi
Yes! If I set the period size to 240, it will not print "Suspicious
buffer position ..."

So it sounds like DSP handles the delay report incorrectly.
Then it comes to another question: the driver supports both SOF and
SST. Is there the behavior difference between both DSPs wrt this
delay issue?

I still don't get what the issue is. The two following cases work fine with the SST/Atom driver:

root@chrx:~# aplay -Dhw:0,0 --period-size=240 --buffer-size=480 /dev/zero -d 2 -f dat --test-position
Playing raw data '/dev/zero' : Signed 16 bit Little Endian, Rate 48000 Hz, Stereo
root@chrx:~# aplay -Dhw:0,0 --period-size=960 --buffer-size=4800 /dev/zero -d 2 -f dat --test-position
Playing raw data '/dev/zero' : Signed 16 bit Little Endian, Rate 48000 Hz, Stereo

The existing code has this:

/* Make sure, that the period size is always even */
snd_pcm_hw_constraint_step(substream->runtime, 0,
SNDRV_PCM_HW_PARAM_PERIODS, 2);

return snd_pcm_hw_constraint_integer(runtime,
SNDRV_PCM_HW_PARAM_PERIODS);

and with the addition of period size being a multiple of 1ms all requirements should be met?