On Thu, 13 Aug 2020 10:36:57 +0200, Yu-Hsuan Hsu wrote: > > Lu, Brent <brent.lu@xxxxxxxxx> 於 2020年8月13日 週四 下午3:55寫道: > > > > > > > > > > > > CRAS calls snd_pcm_hw_params_set_buffer_size_max() to use as large > > > > > buffer as possible. So the period size is an arbitrary number in > > > > > different platforms. Atom SST platform happens to be 256, and CML > > > > > SOF platform is 1056 for example. > > > > > > > > ok, but earlier in this thread it was mentioned that values such as > > > > 432 are not suitable. the statement above seems to mean the period > > > > actual value is a "don't care", so I don't quite see why this specific > > > > patch2 restricting the value to 240 is necessary. Patch1 is needed for > > > > sure, > > > > Patch2 is where Takashi and I are not convinced. > > > > > > I have downloaded the patch1 but it does not work. After applying patch1, > > > the default period size changes to 320. However, it also has the same issue > > > with period size 320. (It can be verified by aplay.) > > > > The period_size is related to the audio latency so it's decided by application > > according to the use case it's running. That's why there are concerns about > > patch 2 and also you cannot find similar constraints in other machine driver. > You're right. However, the problem here is the provided period size > does not work. Like 256, setting the period size to 320 also makes > users have big latency in the DSP ring buffer. > > localhost ~ # aplay -Dhw:1,0 --period-size=320 --buffer-size=640 > /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 = 2640, buffer = 640 > Suspicious buffer position (2 total): avail = 0, delay = 2640, buffer = 640 > Suspicious buffer position (3 total): avail = 0, delay = 2720, buffer = 640 > ... It means that the delay value returned from the driver is bogus. I suppose it comes pcm_delay value calculated in sst_calc_tstamp(), but haven't followed the code closely yet. Maybe checking the debug outputs can help to trace what's going wrong. Takashi > > > > > Another problem is the buffer size. Too large buffer is not just wasting memories. > > It also creates problems to memory allocator since continuous pages are not > > always there. Using a small period_count like 2 or 4 should be sufficient for audio > > data transfer. > > > > buffer_size = period_size * period_count * 1000000 / sample_rate; > > snd_pcm_hw_params_set_buffer_time_near(mPcmDevice, params, &buffer_size, NULL); > > > > And one more problem here: you need to decide period_size and period_count > > first in order to calculate the buffer size... > It's a good point. I will bring it up to our team and see whether we > can use the smaller buffer size. Thanks! > > > > > > Regards, > > Brent > > Thanks, > Yu-Hsuan >