From patchwork Mon Apr 13 14:22:49 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 6209931 Return-Path: X-Original-To: patchwork-alsa-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 3DD1A9F1AC for ; Mon, 13 Apr 2015 14:23:10 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 539872034E for ; Mon, 13 Apr 2015 14:23:09 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.kernel.org (Postfix) with ESMTP id 219A620340 for ; Mon, 13 Apr 2015 14:23:08 +0000 (UTC) Received: by alsa0.perex.cz (Postfix, from userid 1000) id C280226517A; Mon, 13 Apr 2015 16:23:06 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,NO_DNS_FOR_FROM, UNPARSEABLE_RELAY autolearn=no version=3.3.1 Received: from alsa0.perex.cz (localhost [IPv6:::1]) by alsa0.perex.cz (Postfix) with ESMTP id B48D82650BC; Mon, 13 Apr 2015 16:22:58 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 721BD2650FE; Mon, 13 Apr 2015 16:22:57 +0200 (CEST) Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 3AA682650BC for ; Mon, 13 Apr 2015 16:22:50 +0200 (CEST) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id A4BCFAC90; Mon, 13 Apr 2015 14:22:49 +0000 (UTC) Date: Mon, 13 Apr 2015 16:22:49 +0200 Message-ID: From: Takashi Iwai To: Dan Carpenter In-Reply-To: <20150413135820.GY10964@mwanda> References: <20150410093336.GA5779@mwanda> <552BAADA.5060401@ti.com> <20150413121639.GU16501@mwanda> <552BC568.7070907@ti.com> <20150413135820.GY10964@mwanda> 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/24.4 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Cc: "Ujfalusi, Peter" , alsa-devel@alsa-project.org, Mark Brown , Jyri Sarha Subject: Re: [alsa-devel] ASoC: davinci-mcasp: Set rule constraints if implicit BCLK divider is used X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP At Mon, 13 Apr 2015 16:58:20 +0300, Dan Carpenter wrote: > > On Mon, Apr 13, 2015 at 04:32:24PM +0300, Jyri Sarha wrote: > > On 04/13/15 15:16, Dan Carpenter wrote: > > >Thanks for looking at this. I haven't double checked your analysis, but > > >512 means the stack is 2k and that's too high. I think there is > > >supposed to be a build warning if we use more than 1k of stack in a > > >function? > > > > > >It's better to allocate it with kmalloc() and free it at the end. > > > > > >regards, > > >dan carpenter > > > > > > > The function may get called hundreds or even thousands of times when > > an audio stream is started. How heavy is kmalloc/free cycle? > > > > Using kmalloc just feels a bit stupid since in any remotely feasible > > case the maximum number of channels is 8. But maybe I just should > > not care. > > I'm a total newbie to this specific so I can't really advise you. I > did find the limit where GCC prints a warning: > > home/kisskb/slave/src/net/rds/iw_rdma.c: warning: the frame size of 1052 bytes is larger than 1024 bytes [-Wframe-larger-than=]: 200:1 => > > Since this could go up to 2k that's pretty huge. One idea is to just > say: > > int small_buffer[64]; > int size = ci->max - ci->min + 1; > int *list = &small_buffer; > > if (size > 64) > list = kmalloc(); > > ... > > free: > if (size > 64) > kfree(list); > > If you've looked at the call trees which call this function and we can't > actually run out of space then that's probably fine too. I don't think > there are any arches which still use 4k stacks. The code there needs just to calculate min and max values instead of the whole discrete value list. The code has really much room to optimize. A totally untested patch is below. Takashi diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index bb4b78eada58..68a1ae801fb8 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -1095,33 +1095,30 @@ static int davinci_mcasp_hw_rule_channels(struct snd_pcm_hw_params *params, int rate = params_rate(params); int max_chan_per_wire = rd->mcasp->tdm_slots < ci->max ? rd->mcasp->tdm_slots : ci->max; - unsigned int list[ci->max - ci->min + 1]; - int c1, c, count = 0; + struct snd_interval range; + int c1; + snd_interval_any(&range); + range.min = range.max = 0; for (c1 = ci->min; c1 <= max_chan_per_wire; c1++) { uint bclk_freq = c1*sbits*rate; int ppm; davinci_mcasp_calc_clk_div(rd->mcasp, bclk_freq, &ppm); if (abs(ppm) < DAVINCI_MAX_RATE_ERROR_PPM) { + if (!range.min) + range.min = c1; + range.max = c1; /* If we can use all tdm_slots, we can put any amount of channels to remaining wires as long as they fit in. */ - if (c1 == rd->mcasp->tdm_slots) { - for (c = c1; c <= rd->serializers*c1 && - c <= ci->max; c++) - list[count++] = c; - } else { - list[count++] = c1; - } + if (c1 == rd->mcasp->tdm_slots) + range.max = rd->serializers * c1; } } - dev_dbg(rd->mcasp->dev, - "%d possible channel counts (%d-%d) for %d Hz and %d sbits\n", - count, ci->min, ci->max, rate, sbits); - return snd_interval_list(hw_param_interval(params, rule->var), - count, list, 0); + return snd_interval_refine(hw_param_interval(params, rule->var), + &range); } static int davinci_mcasp_startup(struct snd_pcm_substream *substream,