| Message ID | 1384548866-13141-16-git-send-email-swarren@wwwdotorg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 15, 2013 at 01:54:10PM -0700, Stephen Warren wrote: > - DMA device > This allows requesting DMA channels for a device other than the device > which is registering the "PCM" driver. This is quite unusual, but is > currently useful on Tegra. In much HW, and in Tegra20, each DAI HW Acked-by: Mark Brown <broonie@linaro.org> but this one especially I'd like to get into ASoC as soon as possible since this one should be being used by other things. I'm a bit concerned about anything actually using dma_dev since it indicates that something is being worked around, it'd be a bit nicer to print a warning when doing this to give people a hint that they might not be doing the right thing if they use it (unless someone comes up with a system that has a clear use case for it).
On 11/15/2013 09:54 PM, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > Add fields to struct snd_dmaengine_pcm_config to allow custom: > > - DMA channel names. > > This is useful when the default "tx" and "rx" channel names don't > apply, for example if a HW module supports multiple channels, each > having different DMA channel names. This is the case with the FIFOs > in Tegra's AHUB. This new facility can replace > SND_DMAENGINE_PCM_FLAG_CUSTOM_CHANNEL_NAME. > > - DMA device > > This allows requesting DMA channels for a device other than the device > which is registering the "PCM" driver. This is quite unusual, but is > currently useful on Tegra. In much HW, and in Tegra20, each DAI HW > module contains its own FIFOs which DMA writes to. However, in Tegra30, > the DMA FIFOs were split out AHUB HW module, which then routes the data > through a cross-bar, and into the DAI HW modules. However, the current > ASoC driver structure does not expose this detail, and acts as if the > FIFOs are still part of the DAI HW modules. Consequently, the "PCM" > driver is registered with the DAI HW module, yet the DMA channels must > be looked up in the AHUB HW module's device tree node. This new config > field allows that to happen. Eventually, the Tegra drivers will be > reworked to fully expose the AHUB, and this config field can be > removed. > > Cc: treding@nvidia.com > Cc: linux-tegra@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: alsa-devel@alsa-project.org > Signed-off-by: Stephen Warren <swarren@nvidia.com> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
On 11/16/2013 02:44 AM, Mark Brown wrote: > On Fri, Nov 15, 2013 at 01:54:10PM -0700, Stephen Warren wrote: > >> - DMA device > >> This allows requesting DMA channels for a device other than the >> device which is registering the "PCM" driver. This is quite >> unusual, but is currently useful on Tegra. In much HW, and in >> Tegra20, each DAI HW ... > I'm a bit concerned about anything actually using dma_dev since it > indicates that something is being worked around, it'd be a bit > nicer to print a warning when doing this to give people a hint > that they might not be doing the right thing if they use it > (unless someone comes up with a system that has a clear use case > for it). What if I squash the following into that patch: > diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c > index 1160d1cba133..0e2645dee96a 100644 > --- a/sound/soc/soc-generic-dmaengine-pcm.c > +++ b/sound/soc/soc-generic-dmaengine-pcm.c > @@ -296,8 +296,17 @@ static int dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm, > !dev->of_node) > return 0; > > - if (config->dma_dev) > + if (config->dma_dev) { > + /* > + * If this warning is seen, it probably means that your Linux > + * device structure does not match your HW device structure. > + * It would be best to refactor the Linux device structure to > + * correctly match the HW structure. > + */ > + dev_warn(dev, "DMA channels sourced from device %s", > + dev_name(config->dma_dev)); > dev = config->dma_dev; > + } > > for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_CAPTURE; > i++) { (a few patches later) That yields the following warning on Tegra, for example: > [ 2.629623] tegra30-i2s 70080400.i2s: DMA channels sourced from device 70080000.ahub
On Mon, Nov 18, 2013 at 11:45:17AM -0700, Stephen Warren wrote:
> What if I squash the following into that patch:
Looks good.
diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h index 15017311f2e9..87e9d481d7b6 100644 --- a/include/sound/dmaengine_pcm.h +++ b/include/sound/dmaengine_pcm.h @@ -114,6 +114,10 @@ void snd_dmaengine_pcm_set_config_from_dai_data( * @compat_filter_fn: Will be used as the filter function when requesting a * channel for platforms which do not use devicetree. The filter parameter * will be the DAI's DMA data. + * @dma_dev: If set, request DMA channel on this device rather than the DAI + * device. + * @chan_names: If set, these custom DMA channel names will be requested at + * registration time. * @pcm_hardware: snd_pcm_hardware struct to be used for the PCM. * @prealloc_buffer_size: Size of the preallocated audio buffer. * @@ -130,6 +134,8 @@ struct snd_dmaengine_pcm_config { struct snd_soc_pcm_runtime *rtd, struct snd_pcm_substream *substream); dma_filter_fn compat_filter_fn; + struct device *dma_dev; + const char *chan_names[SNDRV_PCM_STREAM_LAST + 1]; const struct snd_pcm_hardware *pcm_hardware; unsigned int prealloc_buffer_size; diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c index f7e65e1552ef..40e0943a06b8 100644 --- a/sound/soc/soc-generic-dmaengine-pcm.c +++ b/sound/soc/soc-generic-dmaengine-pcm.c @@ -285,7 +285,7 @@ static const char * const dmaengine_pcm_dma_channel_names[] = { }; static int dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm, - struct device *dev) + struct device *dev, const struct snd_dmaengine_pcm_config *config) { unsigned int i; const char *name; @@ -296,12 +296,17 @@ static int dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm, !dev->of_node) return 0; + if (config->dma_dev) + dev = config->dma_dev; + for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_CAPTURE; i++) { if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX) name = "rx-tx"; else name = dmaengine_pcm_dma_channel_names[i]; + if (config->chan_names[i]) + name = config->chan_names[i]; chan = dma_request_slave_channel_or_err(dev, name); if (IS_ERR(chan)) { if (PTR_ERR(pcm->chan[i]) == -EPROBE_DEFER) @@ -352,7 +357,7 @@ int snd_dmaengine_pcm_register(struct device *dev, pcm->config = config; pcm->flags = flags; - ret = dmaengine_pcm_request_chan_of(pcm, dev); + ret = dmaengine_pcm_request_chan_of(pcm, dev, config); if (ret) goto err_free_pcm;