alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Cc: 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>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Liam Girdwood <liam.r.girdwood@linux.intel.com>,
	Sam McNally <sammc@chromium.org>, Mark Brown <broonie@kernel.org>,
	"yuhsuan@google.com" <yuhsuan@google.com>,
	Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
	Daniel Stuart <daniel.stuart14@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	"Lu, Brent" <brent.lu@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 09:43:06 +0200	[thread overview]
Message-ID: <s5htux939x1.wl-tiwai@suse.de> (raw)
In-Reply-To: <CAGvk5PogmqfEnFRA8hzby+AGgbOSvbELamh_1=eA9KTpyBMPYQ@mail.gmail.com>

On Tue, 11 Aug 2020 04:29:24 +0200,
Yu-Hsuan Hsu wrote:
> 
> Lu, Brent <brent.lu@intel.com> 於 2020年8月11日 週二 上午10:17寫道:
> >
> > >
> > > 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.
> >
> > I check the code and just realized CRAS does nothing but request maximum buffer
> > size. As I know the application needs to decide the buffer time and period time so
> > ALSA could generate a hw_param structure with proper period size instead of using
> > fixed constraint in machine driver because driver has no idea about the latency you
> > want.
> >
> > You can use snd_pcm_hw_params_set_buffer_time_near() and
> > snd_pcm_hw_params_set_period_time_near() to get a proper configuration of
> > buffer and period parameters according to the latency requirement. In the CRAS
> > code, there is a UCM variable to support this: DmaPeriodMicrosecs. I tested it on
> > Celes and it looks quite promising. It seems to me that adding constraint in machine
> > driver is not necessary.
> >
> > SectionDevice."Speaker".0 {
> >         Value {
> >                 PlaybackPCM "hw:chtrt5650,0"
> >                 DmaPeriodMicrosecs "5000"
> > ...
> >
> > [   52.434761] sound pcmC1D0p: hw_param
> > [   52.434767] sound pcmC1D0p:   ACCESS 0x1
> > [   52.434770] sound pcmC1D0p:   FORMAT 0x4
> > [   52.434772] sound pcmC1D0p:   SUBFORMAT 0x1
> > [   52.434776] sound pcmC1D0p:   SAMPLE_BITS [16:16]
> > [   52.434779] sound pcmC1D0p:   FRAME_BITS [32:32]
> > [   52.434782] sound pcmC1D0p:   CHANNELS [2:2]
> > [   52.434785] sound pcmC1D0p:   RATE [48000:48000]
> > [   52.434788] sound pcmC1D0p:   PERIOD_TIME [5000:5000]
> > [   52.434791] sound pcmC1D0p:   PERIOD_SIZE [240:240]
> > [   52.434794] sound pcmC1D0p:   PERIOD_BYTES [960:960]
> > [   52.434797] sound pcmC1D0p:   PERIODS [852:852]
> > [   52.434799] sound pcmC1D0p:   BUFFER_TIME [4260000:4260000]
> > [   52.434802] sound pcmC1D0p:   BUFFER_SIZE [204480:204480]
> > [   52.434805] sound pcmC1D0p:   BUFFER_BYTES [817920:817920]
> > [   52.434808] sound pcmC1D0p:   TICK_TIME [0:0]
> >
> > Regards,
> > Brent
> Hi Brent,
> 
> Yes, I know we can do it to fix the issue as well. As I mentioned
> before, we wanted to fix it in kernel because it is a real issue,
> isn't it? Basically, a driver should work with any param it supports.
> But in this case, everyone can trigger underrun if he or she does not
> the period size to 240. If you still think it's not necessary, I can
> modify UCM to make CRAS set the appropriate period size.

How does it *not* work if you set other than period size 240, more
exactly?

The hw_constraint to a fixed period size must be really an exception.
If you look at other drivers, you won't find any other doing such.
It already indicates that something is wrong.

Usually the fixed period size comes from the hardware limitation and
defined in snd_pcm_hardware.  Or, sometimes it's an alignment issue.
If you need more than that, you should doubt what's really not
working.


Takashi

  reply	other threads:[~2020-08-11  7:44 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-29 11:03 [PATCH 0/2] Add period size constraint for Atom Chromebook Brent Lu
2020-07-29 11:03 ` [PATCH 1/2] ASoC: intel: atom: Add period size constraint Brent Lu
2020-07-29 11:19   ` Andy Shevchenko
2020-07-29 11:03 ` [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board Brent Lu
2020-07-29 14:08   ` Pierre-Louis Bossart
2020-07-30  8:02     ` Lu, Brent
2020-07-30 15:27       ` Pierre-Louis Bossart
2020-07-30 15:44         ` Pierre-Louis Bossart
2020-07-30 16:17           ` Lu, Brent
2020-07-30 16:56         ` Takashi Iwai
2020-07-31 12:28           ` Lu, Brent
2020-07-29 11:20 ` [PATCH 0/2] Add period size constraint for Atom Chromebook Andy Shevchenko
2020-07-31 12:26 ` [PATCH v3 " Brent Lu
2020-07-31 12:26   ` [PATCH v3 1/2] ASoC: intel: atom: Add period size constraint Brent Lu
2020-07-31 12:26   ` [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board Brent Lu
2020-07-31 13:34     ` Takashi Iwai
2020-08-01  8:58       ` Lu, Brent
2020-08-01  9:26         ` Takashi Iwai
2020-08-03 13:00           ` Lu, Brent
2020-08-03 15:13             ` Pierre-Louis Bossart
2020-08-03 16:45               ` Lu, Brent
2020-08-03 16:56                 ` Takashi Iwai
2020-08-04  4:33                   ` Lu, Brent
2020-08-04 14:24                     ` Pierre-Louis Bossart
2020-08-06 16:41                       ` Lu, Brent
2020-08-10 15:03                         ` Pierre-Louis Bossart
2020-08-10 17:38                           ` Yu-Hsuan Hsu
2020-08-11  2:16                             ` Lu, Brent
2020-08-11  2:29                               ` Yu-Hsuan Hsu
2020-08-11  7:43                                 ` Takashi Iwai [this message]
2020-08-11  8:25                                   ` Yu-Hsuan Hsu
2020-08-11  8:39                                     ` Takashi Iwai
2020-08-11  9:35                                       ` Yu-Hsuan Hsu
2020-08-11 14:53                                         ` Mark Brown
2020-08-11 16:54                                           ` Pierre-Louis Bossart
2020-08-11 17:22                                             ` Mark Brown
2020-08-12  3:09                                               ` Yu-Hsuan Hsu
2020-08-12  6:13                                                 ` Takashi Iwai
2020-08-12  6:53                                                   ` Yu-Hsuan Hsu
2020-08-12  6:58                                                     ` Takashi Iwai
2020-08-12  7:43                                                       ` Yu-Hsuan Hsu
2020-08-12  7:47                                                         ` Takashi Iwai
2020-08-12 14:46                                                           ` Pierre-Louis Bossart
2020-08-12 14:55                                                             ` Takashi Iwai
2020-08-12 15:54                                                               ` Pierre-Louis Bossart
2020-08-12 16:08                                                                 ` Lu, Brent
2020-08-12 16:38                                                                   ` Pierre-Louis Bossart
2020-08-13  6:24                                                                     ` Yu-Hsuan Hsu
2020-08-13  7:55                                                                       ` Lu, Brent
2020-08-13  8:36                                                                         ` Yu-Hsuan Hsu
2020-08-13  8:45                                                                           ` Takashi Iwai
2020-08-13 12:57                                                                             ` Pierre-Louis Bossart
2020-08-13 17:15                                                                               ` Yu-Hsuan Hsu
2020-08-21 16:40   ` [PATCH v3 0/2] Add period size constraint for Atom Chromebook Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5htux939x1.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=brent.lu@intel.com \
    --cc=broonie@kernel.org \
    --cc=cezary.rojewski@intel.com \
    --cc=daniel.stuart14@gmail.com \
    --cc=dj.vsoelen@gmail.com \
    --cc=guennadi.liakhovetski@linux.intel.com \
    --cc=kai.vehmanen@linux.intel.com \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=liam.r.girdwood@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=ranjani.sridharan@linux.intel.com \
    --cc=sammc@chromium.org \
    --cc=tiwai@suse.com \
    --cc=yang.jie@linux.intel.com \
    --cc=yuhsuan@chromium.org \
    --cc=yuhsuan@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).