From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 38745C433E0 for ; Tue, 11 Aug 2020 08:39:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 11AA9206B5 for ; Tue, 11 Aug 2020 08:39:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728370AbgHKIjI (ORCPT ); Tue, 11 Aug 2020 04:39:08 -0400 Received: from mx2.suse.de ([195.135.220.15]:32886 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728237AbgHKIjG (ORCPT ); Tue, 11 Aug 2020 04:39:06 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id CB812B1ED; Tue, 11 Aug 2020 08:39:25 +0000 (UTC) Date: Tue, 11 Aug 2020 10:39:04 +0200 Message-ID: From: Takashi Iwai To: Yu-Hsuan Hsu Cc: "Lu, Brent" , Guennadi Liakhovetski , "alsa-devel@alsa-project.org" , Andy Shevchenko , Kuninori Morimoto , Kai Vehmanen , "linux-kernel@vger.kernel.org" , "Rojewski, Cezary" , Jie Yang , Pierre-Louis Bossart , Takashi Iwai , Liam Girdwood , Sam McNally , Mark Brown , Ranjani Sridharan , Daniel Stuart , "yuhsuan@google.com" , Damian van Soelen Subject: Re: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board In-Reply-To: References: <1596020585-11517-1-git-send-email-brent.lu@intel.com> <1596198365-10105-1-git-send-email-brent.lu@intel.com> <1596198365-10105-3-git-send-email-brent.lu@intel.com> <63bca214-3434-16c6-1b60-adf323aec554@linux.intel.com> <6466847a-8aae-24f7-d727-36ba75e95f98@linux.intel.com> <3f3baf5e-f73d-9cd6-cbfb-36746071e126@linux.intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 11 Aug 2020 10:25:22 +0200, Yu-Hsuan Hsu wrote: > > Takashi Iwai 於 2020年8月11日 週二 下午3:43寫道： > > > > On Tue, 11 Aug 2020 04:29:24 +0200, > > Yu-Hsuan Hsu wrote: > > > > > > Lu, Brent 於 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 > Thank Takashi, > > As I mentioned before, if the period size is set to 256, the measured > rate of sample-consuming will be around 49627 fps. It causes underrun > because the rate we set is 48000 fps. But this explanation rather sounds like the alignment problem. However... > This behavior also happen on the > other period rate except for 240. ... Why only 240? That's the next logical question. If you have a clarification for it, it may be the rigid reason to introduce such a hw constraint. Takashi