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

From: Yu-Hsuan Hsu <yuhsuan@chromium.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: "Lu, Brent" <brent.lu@intel.com>, Takashi Iwai <tiwai@suse.de>,
	Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Kai Vehmanen <kai.vehmanen@linux.intel.com>,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	"Rojewski, Cezary" <cezary.rojewski@intel.com>,
	Takashi Iwai <tiwai@suse.com>,
	Jie Yang <yang.jie@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"yuhsuan@google.com" <yuhsuan@google.com>,
	Liam Girdwood <liam.r.girdwood@linux.intel.com>,
	Sam McNally <sammc@chromium.org>, Mark Brown <broonie@kernel.org>,
	Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
	Daniel Stuart <daniel.stuart14@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Damian van Soelen <dj.vsoelen@gmail.com>
Subject: Re: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
Date: Tue, 11 Aug 2020 01:38:26 +0800	[thread overview]
Message-ID: <CAGvk5PohOP0Yv22tb53EX=ZLB9_vOMb=iujTh64OvHmjC1d4mg@mail.gmail.com> (raw)
In-Reply-To: <3f3baf5e-f73d-9cd6-cbfb-36746071e126@linux.intel.com>

Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> 於
2020年8月10日 週一 下午11:03寫道：
>
>
>
> On 8/6/20 11:41 AM, Lu, Brent wrote:
> >>
> >> I don't get this. If the platform driver already stated 240 and 960 samples why
> >> would 432 be chosen? Doesn't this mean the constraint is not applied?
> >
> > Hi Pierre,
> >
> > Sorry for late reply. I used following constraints in V3 patch so any period which
> > aligns 1ms would be accepted.
> >
> > +     /*
> > +      * Make sure the period to be multiple of 1ms to align the
> > +      * design of firmware. Apply same rule to buffer size to make
> > +      * sure alsa could always find a value for period size
> > +      * regardless the buffer size given by user space.
> > +      */
> > +     snd_pcm_hw_constraint_step(substream->runtime, 0,
> > +                        SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 48);
> > +     snd_pcm_hw_constraint_step(substream->runtime, 0,
> > +                        SNDRV_PCM_HW_PARAM_BUFFER_SIZE, 48);
>
> 432 samples is 9ms, I don't have a clue why/how CRAS might ask for this
> value.
>
> It'd be a bit odd to add constraints just for the purpose of letting
> userspace select a sensible value.

Sorry for the late reply. CRAS does not set the period size when using it.
The default period size is 256, which consumes the samples
quickly(about 49627 fps when the rate is 48000 fps) at the beginning
of the playback.
Since CRAS write samples with the fixed frequency, it triggers
underruns immidiately.

According to Brent, the DSP is using 240 period regardless the
hw_param. If the period size is 256, DSP will read 256 samples each
time but only consume 240 samples until the ring buffer of DSP is
full. This behavior makes the samples in the ring buffer of kernel
consumed quickly. (Not sure whether the explanation is correct. Need
Brent to confirm it.)

Unfortunately, we can not change the behavior of DSP. After some
experiments, we found that the issue can be fixed if we set the period
size to 240. With the same frequency as the DSP, the samples are
consumed stably. Because everyone can trigger this issue when using
the driver without setting the period size, we think it is a general
issue that should be fixed in the kernel.

Thanks,
Yu-Hsuan