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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGvk5Pr7t+TqRuBXbkU2PLTaFSZjDzOQLsx747VW__vEpDztsA@mail.gmail.com>
Date:   Fri, 14 Aug 2020 01:15:54 +0800
From:   Yu-Hsuan Hsu <yuhsuan@...omium.org>
To:     Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
Cc:     Takashi Iwai <tiwai@...e.de>, "Lu, Brent" <brent.lu@...el.com>,
        Guennadi Liakhovetski <guennadi.liakhovetski@...ux.intel.com>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
        Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Rojewski, Cezary" <cezary.rojewski@...el.com>,
        Jie Yang <yang.jie@...ux.intel.com>,
        Takashi Iwai <tiwai@...e.com>,
        Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
        Sam McNally <sammc@...omium.org>,
        Mark Brown <broonie@...nel.org>,
        Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
        Daniel Stuart <daniel.stuart14@...il.com>,
        "yuhsuan@...gle.com" <yuhsuan@...gle.com>,
        Damian van Soelen <dj.vsoelen@...il.com>
Subject: Re: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board

Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com> 於
2020年8月13日 週四 下午8:57寫道：
>
>
>
> On 8/13/20 3:45 AM, Takashi Iwai wrote:
> > On Thu, 13 Aug 2020 10:36:57 +0200,
> > Yu-Hsuan Hsu wrote:
> >>
> >> Lu, Brent <brent.lu@...el.com> 於 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.
>
> the problem is really that we add a constraint that the period size be a
> multiple of 1ms, and it's not respected. 320 samples is not a valid
> choice, I don't get how it ends-up being selected? there's a glitch in
> the matrix here.
>
>
Oh sorry that I applied the wrong patch. With the correct patch, the
default period size is 432.
With period size 432, running aplay with --test-position does not show
any errors. However, by cat `/proc/asound/card1/pcm0p/sub0/status`. We
can see the delay is around 3000.
Here are all period sizes I have tried. All buffer sizes are set to 2
* period size.
period size: 192,  delay is a negative number. Not sure what happened.
period size: 240, delay is fixed at 960
period size: 288, delay is around 27XX
period size: 336, delay is around 27XX
period size: 384, delay is around 24XX (no errors from aplay)
period size: 432, delay is around 30XX (no errors from aplay)
period size: 480, delay is fixed at 3120 (no errors from aplay)
period size: 524, delay is around 31XX (no errors from aplay)

Not sure why the delay is around 50ms except for the period size 240.
Is it normal?

Thanks,
Yu-Hsuan