From patchwork Thu Oct 4 09:21:46 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Mack X-Patchwork-Id: 1545601 Return-Path: X-Original-To: patchwork-davinci@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from bear.ext.ti.com (bear.ext.ti.com [192.94.94.41]) by patchwork1.kernel.org (Postfix) with ESMTP id 128CF3FD9C for ; Thu, 4 Oct 2012 09:23:45 +0000 (UTC) Received: from dlelxv30.itg.ti.com ([172.17.2.17]) by bear.ext.ti.com (8.13.7/8.13.7) with ESMTP id q949Lugv005866; Thu, 4 Oct 2012 04:21:57 -0500 Received: from DFLE72.ent.ti.com (dfle72.ent.ti.com [128.247.5.109]) by dlelxv30.itg.ti.com (8.13.8/8.13.8) with ESMTP id q949LuTe000460; Thu, 4 Oct 2012 04:21:56 -0500 Received: from dlelxv23.itg.ti.com (172.17.1.198) by dfle72.ent.ti.com (128.247.5.109) with Microsoft SMTP Server id 14.1.323.3; Thu, 4 Oct 2012 04:21:56 -0500 Received: from linux.omap.com (dlelxs01.itg.ti.com [157.170.227.31]) by dlelxv23.itg.ti.com (8.13.8/8.13.8) with ESMTP id q949LuCS003070; Thu, 4 Oct 2012 04:21:56 -0500 Received: from linux.omap.com (localhost [127.0.0.1]) by linux.omap.com (Postfix) with ESMTP id 21A0D80627; Thu, 4 Oct 2012 04:21:56 -0500 (CDT) X-Original-To: davinci-linux-open-source@linux.davincidsp.com Delivered-To: davinci-linux-open-source@linux.davincidsp.com Received: from dflp52.itg.ti.com (dflp52.itg.ti.com [128.247.22.96]) by linux.omap.com (Postfix) with ESMTP id 5A6B080626 for ; Thu, 4 Oct 2012 04:21:55 -0500 (CDT) Received: from neches.ext.ti.com (neches.ext.ti.com [192.91.81.29]) by dflp52.itg.ti.com (8.13.7/8.13.8) with ESMTP id q949LsPJ004504 for ; Thu, 4 Oct 2012 04:21:54 -0500 (CDT) Received: from psmtp.com (na3sys009amx195.postini.com [74.125.149.176]) by neches.ext.ti.com (8.13.7/8.13.7) with SMTP id q949LrJu024865 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Thu, 4 Oct 2012 04:21:53 -0500 Received: from mail-bk0-f45.google.com ([209.85.214.45]) (using TLSv1) by na3sys009amx195.postini.com ([74.125.148.10]) with SMTP; Thu, 04 Oct 2012 09:21:54 GMT Received: by mail-bk0-f45.google.com with SMTP id jf3so130879bkc.4 for ; Thu, 04 Oct 2012 02:21:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:x-enigmail-version:content-type; bh=Edjdzuu1Wbt/EC0CjGEhWOKAr5rCCpybVPYpBDBDr0w=; b=HsoL9U9d981UxFtyTmpwn+ahxXh3KPPVFW/tRlkmFS/yBSmMNzylHLX59da1AcIgvS hvOkD3E5+k4UvEugRgPXyf0ueSFlQ6NfoiWxAZIW12wGitFHp4Unx1C54cFjP2TWIgNZ 0rySM55GRi1wlDZ5dJMBNopreIQiNCjh1vwNkynGVMUe33wj4CBwh3C5VtiJXbnaV4d2 ZjlTG5As7UpdRJry7xaoCqz5PmPp4JOD0xFdGpox9YPvpZoAwo2goD8pXQzFhwt/NEIP bnflGqAEJPHU97IExOI6+JQ3fcLcodBD9b+VpwGJOTtaiQqb0p/chSa7PnyaZULWuWg+ 4HCg== Received: by 10.204.9.4 with SMTP id j4mr1323107bkj.22.1349342511873; Thu, 04 Oct 2012 02:21:51 -0700 (PDT) Received: from [10.0.1.6] (i59F701CE.versanet.de. [89.247.1.206]) by mx.google.com with ESMTPS id m19sm5035315bkm.8.2012.10.04.02.21.50 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 04 Oct 2012 02:21:51 -0700 (PDT) Message-ID: <506D552A.2000506@gmail.com> Date: Thu, 4 Oct 2012 11:21:46 +0200 From: Daniel Mack User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 MIME-Version: 1.0 To: Matt Porter Subject: Re: [alsa-devel] [PATCH 2/3] ASoC: Davinci: pcm: add support for sram-support-less platforms References: <1346417459-30042-1-git-send-email-gururaja.hebbar@ti.com> <1346417459-30042-3-git-send-email-gururaja.hebbar@ti.com> <20120922153313.GN4495@opensource.wolfsonmicro.com> <506A9C65.5040309@ti.com> <20121002093753.GU4360@opensource.wolfsonmicro.com> <506AC303.9080906@gmail.com> <506ACACE.4030308@ti.com> <506AEF57.2000306@gmail.com> <20121002164116.GT5641@beef> <506B1B46.2070006@gmail.com> In-Reply-To: <506B1B46.2070006@gmail.com> X-Enigmail-Version: 1.4.4 X-pstn-neptune: 0/0/0.00/0 X-pstn-levels: (S:99.90000/99.90000 CV:99.9000 FC:95.5390 LC:95.5390 R:95.9108 P:95.9108 M:97.0282 C:98.6951 ) X-pstn-dkim: 1 skipped:not-enabled X-pstn-settings: 2 (0.5000:0.0050) s cv GT3 gt2 gt1 r p m c X-pstn-addresses: from [82/3] CC: , , Mark Brown , , Peter Ujfalusi , , X-BeenThere: davinci-linux-open-source@linux.davincidsp.com X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: Errors-To: davinci-linux-open-source-bounces@linux.davincidsp.com On 02.10.2012 18:50, Daniel Mack wrote: > On 02.10.2012 18:41, Matt Porter wrote: >> On Tue, Oct 02, 2012 at 03:42:47PM +0200, Daniel Mack wrote: >>> On 02.10.2012 13:06, Sekhar Nori wrote: >>>> On 10/2/2012 4:03 PM, Daniel Mack wrote: >>>>> On 02.10.2012 11:37, Mark Brown wrote: >>>>>> On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: >>>>>> >>>>>>> I also agree that ifdef is not a good solution. >>>>>>> It is better to have this information passed as device_data and via DT it can >>>>>>> be decided based on the compatible property for the device. >>>>>> >>>>>> That's not really the problem here - the problem is that the APIs used >>>>>> to get the SRAM are DaVinci only so it's not possible to build on OMAP >>>>>> or other platforms. The SRAM code needs to move to a standard API. >>>>> >>>>> What about following Matt Porter's idea and ignore the SRAM code >>>>> entirely and port the entire PCM code to generic dmaengine code first? >>>>> The EDMA driver needs to learn support for cyclic DMA for that, and I >>>>> might give that a try in near future. >>>>> >>>>> Later on, the SRAM ping-pong code can get added back using genalloc >>>>> functions, as Sekhar proposed. That needs to be done by someone who has >>>>> access to a Davinci board though, I only have a AM33xx/OMAP here. >>>> >>>> We cannot "get rid" of SRAM code and add it back "later". It is required >>>> for most DaVinci parts. The SRAM code can be converted to use genalloc >>>> (conversion should be straightforward and I can help test it) and the >>>> code that uses SRAM can probably keep using the private EDMA API till >>>> the dmaengine EDMA driver has cyclic DMA support. Matt has already >>>> posted patches to move private EDMA APIs to a common location. That >>>> should keep AM335x build from breaking. Is this something that is feasible? >>> >>> Yes - by "later" I just meant in a subsequent patch. But you're probably >>> right, we can also do that first. >>> >>> I'm looking at that right now and the problem seems that we don't have a >>> sane way to dynamically look up gen_pools independently of the actual >>> run-time platform. All users of gen_pools seem to know which platform >>> they run on and access a platform-specific pool. So I don't currently >>> see how we could implement multi-platform code, gen_pools are fine but >>> don't solve the problem here. >>> >>> Would it be an idea to add a char* member to gen_pools and a function >>> that can be used to dynamically look it up again? If a buffer with a >>> certain name exists, we can use it and install that ping-pong buffer, >>> otherwise we just don't. While that would be easy to do, it's a >>> tree-wide change. >>> >>> Is there a better way that I miss? >> >> At the high level there's two platform models we have to handle, the >> boot from board file !DT case, and then the boot from DT case. Since >> Davinci is just starting DT conversion, we mostly care about the !DT >> base in which the struct gen_pool * is passed in pdata to the ASoC >> driver. It is then selectable on a per-platform basis where the decision >> should be made. >> >> Given a separate discussion with Sekhar, we're only going to have one >> SRAM pool on any DaVinci part right now...this was only a question on >> L138 anyway. But regardless, the created gen_pool will be passed via >> pdata. > > I thought about this too, as mmp does it that way. > >> Since DT conversion is starting and we need to consider that now, >> the idea there is to use the DT-based generic sram driver [1] such that >> when we do boot from DT on Davinci, the genpool is provided via phandle >> and the pointer extracted with the OF helpers that are part of the >> series. > > A phandle is the cleanest way I think, yes. > >> That's pretty much it. I'm reworking the backend support as discussed >> with Sekhar wrt to my uio_pruss series. I can post a standalone series >> that just replaces sram_* with genalloc for davinci ASoC. > > As you can also test it, it would be easiest if you came up with a patch > for that, yes. I can have a look at the dma bits laters, once my OMAP > board finally works with the code as it currently stands. I'm still > fighting with the mcasp driver right now ... I quickly prepared two patches to change that, so that topic is out of the way. But I did only compile-test them on OMAP - could you check on your Davinci platform? Note that these apply on top of the patch in discussion here (which isn't applied to the asoc tree yet). Daniel >From 465846f6fd1ea39534f32b984d352b5bc6928889 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Thu, 4 Oct 2012 11:12:28 +0200 Subject: [PATCH 2/2] ALSA: ASoC: McASP: use gen_pool from platform data This makes the gen_pool SRAM usage a runtime decision. Signed-off-by: Daniel Mack --- sound/soc/davinci/davinci-pcm.c | 59 +++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c index 7ac5a19..f47b8f3 100644 --- a/sound/soc/davinci/davinci-pcm.c +++ b/sound/soc/davinci/davinci-pcm.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -261,26 +262,27 @@ static void davinci_pcm_dma_irq(unsigned link, u16 ch_status, void *data) } } -#if defined(CONFIG_SND_DAVINCI_HAVE_SRAM) +#ifdef CONFIG_GENERIC_ALLOCATOR static int allocate_sram(struct snd_pcm_substream *substream, unsigned size, - struct snd_pcm_hardware *ppcm) + struct snd_pcm_hardware *ppcm, struct gen_pool *pool) { struct snd_dma_buffer *buf = &substream->dma_buffer; struct snd_dma_buffer *iram_dma = NULL; + unsigned long iram_virt = 0; dma_addr_t iram_phys = 0; - void *iram_virt = NULL; if (buf->private_data || !size) return 0; ppcm->period_bytes_max = size; - iram_virt = sram_alloc(size, &iram_phys); + iram_virt = gen_pool_alloc(pool, size); if (!iram_virt) goto exit1; + iram_phys = gen_pool_virt_to_phys(pool, iram_virt); iram_dma = kzalloc(sizeof(*iram_dma), GFP_KERNEL); if (!iram_dma) goto exit2; - iram_dma->area = iram_virt; + iram_dma->area = (void *) iram_virt; iram_dma->addr = iram_phys; memset(iram_dma->area, 0, size); iram_dma->bytes = size; @@ -288,12 +290,40 @@ static int allocate_sram(struct snd_pcm_substream *substream, unsigned size, return 0; exit2: if (iram_virt) - sram_free(iram_virt, size); + gen_pool_free(pool, iram_virt, size); exit1: return -ENOMEM; } + +static void free_sram(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_dma_buffer *buf = &substream->dma_buffer; + struct snd_dma_buffer *iram_dma = buf->private_data; + struct davinci_pcm_dma_params *params, *pa; + + if (!buf || !rtd || !iram_dma) + return; + + pa = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream); + params = &pa[substream->stream]; + + gen_pool_free(params->sram_pool, (unsigned long) iram_dma->area, + iram_dma->bytes); +} +#else +static int allocate_sram(struct snd_pcm_substream *substream, unsigned size, + struct snd_pcm_hardware *ppcm, struct gen_pool *pool) +{ + return 0; +} + +static void free_sram(struct snd_pcm_substream *substream) +{ +} #endif + /* * Only used with ping/pong. * This is called after runtime->dma_addr, period_bytes and data_type are valid @@ -680,9 +710,11 @@ static int davinci_pcm_open(struct snd_pcm_substream *substream) ppcm = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ? &pcm_hardware_playback : &pcm_hardware_capture; -#if defined(CONFIG_SND_DAVINCI_HAVE_SRAM) - allocate_sram(substream, params->sram_size, ppcm); -#endif + + if (params->sram_pool) + allocate_sram(substream, params->sram_size, ppcm, + params->sram_pool); + snd_soc_set_runtime_hwparams(substream, ppcm); /* ensure that buffer size is a multiple of period size */ ret = snd_pcm_hw_constraint_integer(runtime, @@ -811,7 +843,6 @@ static void davinci_pcm_free(struct snd_pcm *pcm) int stream; for (stream = 0; stream < 2; stream++) { - struct snd_dma_buffer *iram_dma; substream = pcm->streams[stream].substream; if (!substream) continue; @@ -823,13 +854,7 @@ static void davinci_pcm_free(struct snd_pcm *pcm) dma_free_writecombine(pcm->card->dev, buf->bytes, buf->area, buf->addr); buf->area = NULL; -#if defined(CONFIG_SND_DAVINCI_HAVE_SRAM) - iram_dma = buf->private_data; - if (iram_dma) { - sram_free(iram_dma->area, iram_dma->bytes); - kfree(iram_dma); - } -#endif + free_sram(substream); } } -- 1.7.11.4