From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vinod Koul Subject: Re: [PATCH v2 1/2] ASoC: dmaengine-pcm: Add support for querying DMA capabilities Date: Tue, 8 Oct 2013 20:36:12 +0530 Message-ID: <20131008150612.GR2954@intel.com> References: <1381237680-32654-1-git-send-email-lars@metafoo.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by alsa0.perex.cz (Postfix) with ESMTP id E0E1A26535C for ; Tue, 8 Oct 2013 17:59:01 +0200 (CEST) Content-Disposition: inline In-Reply-To: <1381237680-32654-1-git-send-email-lars@metafoo.de> 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 To: Lars-Peter Clausen Cc: alsa-devel@alsa-project.org, Mark Brown , Liam Girdwood List-Id: alsa-devel@alsa-project.org On Tue, Oct 08, 2013 at 03:07:59PM +0200, Lars-Peter Clausen wrote: > Currently each platform making use the the generic dmaengine PCM driver still > needs to provide a custom snd_pcm_hardware struct which specifies the > capabilities of the DMA controller, e.g. the maximum period size that can be > supported. This patch adds code which uses the newly introduced > dma_get_slave_caps() API to query this information from the dmaengine driver. > The new code path will only be taken if the 'pcm_hardware' field of the > snd_dmaengine_pcm_config struct is NULL. > > The patch also introduces a new 'fifo_size' field to the > snd_dmaengine_dai_dma_data struct which is used to initialize the > snd_pcm_hardware 'fifo_size' field and needs to be set by the DAI driver. > > Signed-off-by: Lars-Peter Clausen > > --- > Changes since v1: > * Use dma_get_max_seg_size() to get the maximum size of one period > * Assume that a DMA engine driver is able to support an infinite number of > periods > * Increase min_period_bytes from 16 to 256. > --- > include/sound/dmaengine_pcm.h | 2 ++ > sound/soc/soc-generic-dmaengine-pcm.c | 55 ++++++++++++++++++++++++++++------- > 2 files changed, 47 insertions(+), 10 deletions(-) > > diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h > index f11c35c..83b2c3e 100644 > --- a/include/sound/dmaengine_pcm.h > +++ b/include/sound/dmaengine_pcm.h > @@ -61,6 +61,7 @@ struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream) > * @slave_id: Slave requester id for the DMA channel. > * @filter_data: Custom DMA channel filter data, this will usually be used when > * requesting the DMA channel. > + * @fifo_size: FIFO size of the DAI controller in bytes is this supposed to be DMA FIFO or I2S FIFO? And second question for Takakshi or Mark. I see that this value ends up in usermode, what is the intented usage of this? > */ > struct snd_dmaengine_dai_dma_data { > dma_addr_t addr; > @@ -68,6 +69,7 @@ struct snd_dmaengine_dai_dma_data { > u32 maxburst; > unsigned int slave_id; > void *filter_data; > + unsigned int fifo_size; > }; > > void snd_dmaengine_pcm_set_config_from_dai_data( > diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c > index e29ec3c..c39e19e 100644 > --- a/sound/soc/soc-generic-dmaengine-pcm.c > +++ b/sound/soc/soc-generic-dmaengine-pcm.c > @@ -36,6 +36,15 @@ static struct dmaengine_pcm *soc_platform_to_pcm(struct snd_soc_platform *p) > return container_of(p, struct dmaengine_pcm, platform); > } > > +static struct device *dmaengine_dma_dev(struct dmaengine_pcm *pcm, > + struct snd_pcm_substream *substream) > +{ > + if (!pcm->chan[substream->stream]) > + return NULL; > + > + return pcm->chan[substream->stream]->device->dev; > +} > + > /** > * snd_dmaengine_pcm_prepare_slave_config() - Generic prepare_slave_config callback > * @substream: PCM substream > @@ -92,28 +101,54 @@ static int dmaengine_pcm_hw_params(struct snd_pcm_substream *substream, > return snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(params)); > } > > -static int dmaengine_pcm_open(struct snd_pcm_substream *substream) > +static int dmaengine_pcm_set_runtime_hwparams(struct snd_pcm_substream *substream) > { > struct snd_soc_pcm_runtime *rtd = substream->private_data; > struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform); > + struct device *dma_dev = dmaengine_dma_dev(pcm, substream); > struct dma_chan *chan = pcm->chan[substream->stream]; > + struct snd_dmaengine_dai_dma_data *dma_data; > + struct dma_slave_caps dma_caps; > + struct snd_pcm_hardware hw; > int ret; > > - ret = snd_soc_set_runtime_hwparams(substream, > + if (pcm->config->pcm_hardware) > + return snd_soc_set_runtime_hwparams(substream, > pcm->config->pcm_hardware); > - if (ret) > - return ret; > > - return snd_dmaengine_pcm_open(substream, chan); > + dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream); > + > + memset(&hw, 0, sizeof(hw)); > + hw.info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | > + SNDRV_PCM_INFO_INTERLEAVED; > + hw.periods_min = 2; > + hw.periods_max = UINT_MAX; > + hw.period_bytes_min = 256; > + hw.period_bytes_max = dma_get_max_seg_size(dma_dev); I think you should check if the driver has set this, if not goto some default -- ~Vinod