[alsa-devel] [PATCH] ASoC: generic-dmaengine-pcm: Add an interface to override its functions
Each implementation of gerneric pcm dmaengine has been naturally limited by the pre-defined gerneric functions. Thus add an extra interface for user to create a backdoor so that they can override those fixed functions for some uncommon requirements: using on-chip memory for DMA buffers and accordingly different mmap function for example, while at the meantime, they can continue to benifit from the concise and wise generic pcm dmaengine.
Signed-off-by: Nicolin Chen b42378@freescale.com --- include/sound/dmaengine_pcm.h | 13 +++++++++++++ sound/core/pcm_dmaengine.c | 25 +++++++++++++++++++++++++ sound/soc/soc-generic-dmaengine-pcm.c | 30 ++++++++++++++++++------------ 3 files changed, 56 insertions(+), 12 deletions(-)
diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h index f11c35c..a6e9d04 100644 --- a/include/sound/dmaengine_pcm.h +++ b/include/sound/dmaengine_pcm.h @@ -33,6 +33,18 @@ snd_pcm_substream_to_dma_direction(const struct snd_pcm_substream *substream) return DMA_DEV_TO_MEM; }
+struct dmaengine_pcm { + struct dma_chan *chan[SNDRV_PCM_STREAM_CAPTURE + 1]; + const struct snd_dmaengine_pcm_config *config; + struct snd_soc_platform platform; + unsigned int flags; +}; + +static inline struct dmaengine_pcm *soc_platform_to_pcm(struct snd_soc_platform *p) +{ + return container_of(p, struct dmaengine_pcm, platform); +} + int snd_hwparams_to_dma_slave_config(const struct snd_pcm_substream *substream, const struct snd_pcm_hw_params *params, struct dma_slave_config *slave_config); int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd); @@ -123,6 +135,7 @@ struct snd_dmaengine_pcm_config { struct snd_pcm_substream *substream); dma_filter_fn compat_filter_fn;
+ const struct snd_soc_platform_driver *driver; const struct snd_pcm_hardware *pcm_hardware; unsigned int prealloc_buffer_size; }; diff --git a/sound/core/pcm_dmaengine.c b/sound/core/pcm_dmaengine.c index aa924d9..d78a67e 100644 --- a/sound/core/pcm_dmaengine.c +++ b/sound/core/pcm_dmaengine.c @@ -186,8 +186,14 @@ static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream *substream) int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream); + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform); + const struct snd_dmaengine_pcm_config *config = pcm->config; int ret;
+ if (config->driver->ops->trigger) + return config->driver->ops->trigger(substream, cmd); + switch (cmd) { case SNDRV_PCM_TRIGGER_START: ret = dmaengine_pcm_prepare_and_submit(substream); @@ -224,6 +230,13 @@ EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_trigger); snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream *substream) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream); + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform); + const struct snd_dmaengine_pcm_config *config = pcm->config; + + if (config->driver->ops->pointer) + return config->driver->ops->pointer(substream); + return bytes_to_frames(substream->runtime, prtd->pos); } EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer_no_residue); @@ -238,11 +251,17 @@ EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer_no_residue); snd_pcm_uframes_t snd_dmaengine_pcm_pointer(struct snd_pcm_substream *substream) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream); + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform); + const struct snd_dmaengine_pcm_config *config = pcm->config; struct dma_tx_state state; enum dma_status status; unsigned int buf_size; unsigned int pos = 0;
+ if (config->driver->ops->pointer) + return config->driver->ops->pointer(substream); + status = dmaengine_tx_status(prtd->dma_chan, prtd->cookie, &state); if (status == DMA_IN_PROGRESS || status == DMA_PAUSED) { buf_size = snd_pcm_lib_buffer_bytes(substream); @@ -341,6 +360,12 @@ EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open_request_chan); int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream); + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform); + const struct snd_dmaengine_pcm_config *config = pcm->config; + + if (config->driver->ops->close) + return config->driver->ops->close(substream);
kfree(prtd);
diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c index e29ec3c..6e66c53 100644 --- a/sound/soc/soc-generic-dmaengine-pcm.c +++ b/sound/soc/soc-generic-dmaengine-pcm.c @@ -24,18 +24,6 @@
#include <sound/dmaengine_pcm.h>
-struct dmaengine_pcm { - struct dma_chan *chan[SNDRV_PCM_STREAM_CAPTURE + 1]; - const struct snd_dmaengine_pcm_config *config; - struct snd_soc_platform platform; - unsigned int flags; -}; - -static struct dmaengine_pcm *soc_platform_to_pcm(struct snd_soc_platform *p) -{ - return container_of(p, struct dmaengine_pcm, platform); -} - /** * snd_dmaengine_pcm_prepare_slave_config() - Generic prepare_slave_config callback * @substream: PCM substream @@ -75,9 +63,13 @@ static int dmaengine_pcm_hw_params(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 dma_chan *chan = snd_dmaengine_pcm_get_chan(substream); + const struct snd_dmaengine_pcm_config *config = pcm->config; struct dma_slave_config slave_config; int ret;
+ if (config->driver->ops->hw_params) + return config->driver->ops->hw_params(substream, params); + if (pcm->config->prepare_slave_config) { ret = pcm->config->prepare_slave_config(substream, params, &slave_config); @@ -96,9 +88,13 @@ static int dmaengine_pcm_open(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform); + const struct snd_dmaengine_pcm_config *config = pcm->config; struct dma_chan *chan = pcm->chan[substream->stream]; int ret;
+ if (config->driver->ops->open) + return config->driver->ops->open(substream); + ret = snd_soc_set_runtime_hwparams(substream, pcm->config->pcm_hardware); if (ret) @@ -118,6 +114,13 @@ static struct device *dmaengine_dma_dev(struct dmaengine_pcm *pcm,
static void dmaengine_pcm_free(struct snd_pcm *pcm) { + struct snd_soc_pcm_runtime *rtd = pcm->private_data; + struct dmaengine_pcm *dma_pcm = soc_platform_to_pcm(rtd->platform); + const struct snd_dmaengine_pcm_config *config = dma_pcm->config; + + if (config->driver->pcm_free) + return config->driver->pcm_free(pcm); + snd_pcm_lib_preallocate_free_for_all(pcm); }
@@ -145,6 +148,9 @@ static int dmaengine_pcm_new(struct snd_soc_pcm_runtime *rtd) unsigned int i; int ret;
+ if (config->driver->pcm_new) + return config->driver->pcm_new(rtd); + for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_CAPTURE; i++) { substream = rtd->pcm->streams[i].substream; if (!substream)
On 10/15/2013 09:49 AM, Nicolin Chen wrote:
Each implementation of gerneric pcm dmaengine has been naturally limited by the pre-defined gerneric functions. Thus add an extra interface for user to create a backdoor so that they can override those fixed functions for some uncommon requirements: using on-chip memory for DMA buffers and accordingly different mmap function for example, while at the meantime, they can continue to benifit from the concise and wise generic pcm dmaengine.
Signed-off-by: Nicolin Chen b42378@freescale.com
We do have the dmaengine pcm helper functions (sound/core/pcm_dmaengine.c) and the generic dmaengine pcm ASoC driver (sound/soc/soc-generic-dmaengine-pcm.c). The generic dmaengine pcm ASoC driver uses the dmaengine pcm helper functions, but it is not the only user of them. So your patch breaks all other users of the helper functions.
The helper functions are designed in a way that you can either wrap them in your own pcm driver driver or not use them at all. E.g. take a look at sound/soc/omap/omap-pcm.c on how to overwrite specific functions.
- Lars
include/sound/dmaengine_pcm.h | 13 +++++++++++++ sound/core/pcm_dmaengine.c | 25 +++++++++++++++++++++++++ sound/soc/soc-generic-dmaengine-pcm.c | 30 ++++++++++++++++++------------ 3 files changed, 56 insertions(+), 12 deletions(-)
diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h index f11c35c..a6e9d04 100644 --- a/include/sound/dmaengine_pcm.h +++ b/include/sound/dmaengine_pcm.h @@ -33,6 +33,18 @@ snd_pcm_substream_to_dma_direction(const struct snd_pcm_substream *substream) return DMA_DEV_TO_MEM; }
+struct dmaengine_pcm {
- struct dma_chan *chan[SNDRV_PCM_STREAM_CAPTURE + 1];
- const struct snd_dmaengine_pcm_config *config;
- struct snd_soc_platform platform;
- unsigned int flags;
+};
+static inline struct dmaengine_pcm *soc_platform_to_pcm(struct snd_soc_platform *p) +{
- return container_of(p, struct dmaengine_pcm, platform);
+}
int snd_hwparams_to_dma_slave_config(const struct snd_pcm_substream *substream, const struct snd_pcm_hw_params *params, struct dma_slave_config *slave_config); int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd); @@ -123,6 +135,7 @@ struct snd_dmaengine_pcm_config { struct snd_pcm_substream *substream); dma_filter_fn compat_filter_fn;
- const struct snd_soc_platform_driver *driver; const struct snd_pcm_hardware *pcm_hardware; unsigned int prealloc_buffer_size;
}; diff --git a/sound/core/pcm_dmaengine.c b/sound/core/pcm_dmaengine.c index aa924d9..d78a67e 100644 --- a/sound/core/pcm_dmaengine.c +++ b/sound/core/pcm_dmaengine.c @@ -186,8 +186,14 @@ static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream *substream) int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform);
const struct snd_dmaengine_pcm_config *config = pcm->config; int ret;
if (config->driver->ops->trigger)
return config->driver->ops->trigger(substream, cmd);switch (cmd) { case SNDRV_PCM_TRIGGER_START: ret = dmaengine_pcm_prepare_and_submit(substream);
@@ -224,6 +230,13 @@ EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_trigger); snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream *substream) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform);
- const struct snd_dmaengine_pcm_config *config = pcm->config;
- if (config->driver->ops->pointer)
return config->driver->ops->pointer(substream);- return bytes_to_frames(substream->runtime, prtd->pos);
} EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer_no_residue); @@ -238,11 +251,17 @@ EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer_no_residue); snd_pcm_uframes_t snd_dmaengine_pcm_pointer(struct snd_pcm_substream *substream) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform);
const struct snd_dmaengine_pcm_config *config = pcm->config; struct dma_tx_state state; enum dma_status status; unsigned int buf_size; unsigned int pos = 0;
if (config->driver->ops->pointer)
return config->driver->ops->pointer(substream);status = dmaengine_tx_status(prtd->dma_chan, prtd->cookie, &state); if (status == DMA_IN_PROGRESS || status == DMA_PAUSED) { buf_size = snd_pcm_lib_buffer_bytes(substream);
@@ -341,6 +360,12 @@ EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open_request_chan); int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform);
const struct snd_dmaengine_pcm_config *config = pcm->config;
if (config->driver->ops->close)
return config->driver->ops->close(substream);kfree(prtd);
diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c index e29ec3c..6e66c53 100644 --- a/sound/soc/soc-generic-dmaengine-pcm.c +++ b/sound/soc/soc-generic-dmaengine-pcm.c @@ -24,18 +24,6 @@
#include <sound/dmaengine_pcm.h>
-struct dmaengine_pcm {
- struct dma_chan *chan[SNDRV_PCM_STREAM_CAPTURE + 1];
- const struct snd_dmaengine_pcm_config *config;
- struct snd_soc_platform platform;
- unsigned int flags;
-};
-static struct dmaengine_pcm *soc_platform_to_pcm(struct snd_soc_platform *p) -{
- return container_of(p, struct dmaengine_pcm, platform);
-}
/**
- snd_dmaengine_pcm_prepare_slave_config() - Generic prepare_slave_config callback
- @substream: PCM substream
@@ -75,9 +63,13 @@ static int dmaengine_pcm_hw_params(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 dma_chan *chan = snd_dmaengine_pcm_get_chan(substream);
const struct snd_dmaengine_pcm_config *config = pcm->config; struct dma_slave_config slave_config; int ret;
if (config->driver->ops->hw_params)
return config->driver->ops->hw_params(substream, params);if (pcm->config->prepare_slave_config) { ret = pcm->config->prepare_slave_config(substream, params, &slave_config);
@@ -96,9 +88,13 @@ static int dmaengine_pcm_open(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform);
const struct snd_dmaengine_pcm_config *config = pcm->config; struct dma_chan *chan = pcm->chan[substream->stream]; int ret;
if (config->driver->ops->open)
return config->driver->ops->open(substream);ret = snd_soc_set_runtime_hwparams(substream, pcm->config->pcm_hardware); if (ret)
@@ -118,6 +114,13 @@ static struct device *dmaengine_dma_dev(struct dmaengine_pcm *pcm,
static void dmaengine_pcm_free(struct snd_pcm *pcm) {
- struct snd_soc_pcm_runtime *rtd = pcm->private_data;
- struct dmaengine_pcm *dma_pcm = soc_platform_to_pcm(rtd->platform);
- const struct snd_dmaengine_pcm_config *config = dma_pcm->config;
- if (config->driver->pcm_free)
return config->driver->pcm_free(pcm);- snd_pcm_lib_preallocate_free_for_all(pcm);
}
@@ -145,6 +148,9 @@ static int dmaengine_pcm_new(struct snd_soc_pcm_runtime *rtd) unsigned int i; int ret;
- if (config->driver->pcm_new)
return config->driver->pcm_new(rtd);- for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_CAPTURE; i++) { substream = rtd->pcm->streams[i].substream; if (!substream)
Hi Lars,
On Tue, Oct 15, 2013 at 10:08:52AM +0200, Lars-Peter Clausen wrote:
On 10/15/2013 09:49 AM, Nicolin Chen wrote:
Each implementation of gerneric pcm dmaengine has been naturally limited by the pre-defined gerneric functions. Thus add an extra interface for user to create a backdoor so that they can override those fixed functions for some uncommon requirements: using on-chip memory for DMA buffers and accordingly different mmap function for example, while at the meantime, they can continue to benifit from the concise and wise generic pcm dmaengine.
Signed-off-by: Nicolin Chen b42378@freescale.com
We do have the dmaengine pcm helper functions (sound/core/pcm_dmaengine.c) and the generic dmaengine pcm ASoC driver (sound/soc/soc-generic-dmaengine-pcm.c). The generic dmaengine pcm ASoC driver uses the dmaengine pcm helper functions, but it is not the only user of them. So your patch breaks all other users of the helper functions.
The helper functions are designed in a way that you can either wrap them in your own pcm driver driver or not use them at all. E.g. take a look at sound/soc/omap/omap-pcm.c on how to overwrite specific functions.
- Lars
If this modification to sound/core/pcm_dmaengine.c is not fair, what about moving the modification to soc-gerneric-dmaengine-pcm.c?
For example, instead of using snd_dmaengine_pcm_trigger() directly in gerneric soc dmaengine driver, we create a new one inside gerneric soc dmaengine driver and check the override:
static int snd_soc_dmaengine_pcm_trigger(substream, cmd) { if (config->driver->ops->trigger) return config->driver->ops->trigger(substream, cmd); return snd_dmaengine_pcm_trigger(substream, cmd); }
Since it happens inside generic dmaengine pcm ASoC driver, it won't break those helper functions.
I'm trying this just because I hope generic dmaengine can be more flexible Admittedly, using helper functions might be more plausible way in current ASoC structure. However, there might be so much change for an generic soc dmaengine implementation even if it just wants to override one single func.
Thank you, Nicolin Chen
include/sound/dmaengine_pcm.h | 13 +++++++++++++ sound/core/pcm_dmaengine.c | 25 +++++++++++++++++++++++++ sound/soc/soc-generic-dmaengine-pcm.c | 30 ++++++++++++++++++------------ 3 files changed, 56 insertions(+), 12 deletions(-)
diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h index f11c35c..a6e9d04 100644 --- a/include/sound/dmaengine_pcm.h +++ b/include/sound/dmaengine_pcm.h @@ -33,6 +33,18 @@ snd_pcm_substream_to_dma_direction(const struct snd_pcm_substream *substream) return DMA_DEV_TO_MEM; }
+struct dmaengine_pcm {
- struct dma_chan *chan[SNDRV_PCM_STREAM_CAPTURE + 1];
- const struct snd_dmaengine_pcm_config *config;
- struct snd_soc_platform platform;
- unsigned int flags;
+};
+static inline struct dmaengine_pcm *soc_platform_to_pcm(struct snd_soc_platform *p) +{
- return container_of(p, struct dmaengine_pcm, platform);
+}
int snd_hwparams_to_dma_slave_config(const struct snd_pcm_substream *substream, const struct snd_pcm_hw_params *params, struct dma_slave_config *slave_config); int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd); @@ -123,6 +135,7 @@ struct snd_dmaengine_pcm_config { struct snd_pcm_substream *substream); dma_filter_fn compat_filter_fn;
- const struct snd_soc_platform_driver *driver; const struct snd_pcm_hardware *pcm_hardware; unsigned int prealloc_buffer_size;
}; diff --git a/sound/core/pcm_dmaengine.c b/sound/core/pcm_dmaengine.c index aa924d9..d78a67e 100644 --- a/sound/core/pcm_dmaengine.c +++ b/sound/core/pcm_dmaengine.c @@ -186,8 +186,14 @@ static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream *substream) int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform);
const struct snd_dmaengine_pcm_config *config = pcm->config; int ret;
if (config->driver->ops->trigger)
return config->driver->ops->trigger(substream, cmd);switch (cmd) { case SNDRV_PCM_TRIGGER_START: ret = dmaengine_pcm_prepare_and_submit(substream);
@@ -224,6 +230,13 @@ EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_trigger); snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream *substream) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform);
- const struct snd_dmaengine_pcm_config *config = pcm->config;
- if (config->driver->ops->pointer)
return config->driver->ops->pointer(substream);- return bytes_to_frames(substream->runtime, prtd->pos);
} EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer_no_residue); @@ -238,11 +251,17 @@ EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer_no_residue); snd_pcm_uframes_t snd_dmaengine_pcm_pointer(struct snd_pcm_substream *substream) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform);
const struct snd_dmaengine_pcm_config *config = pcm->config; struct dma_tx_state state; enum dma_status status; unsigned int buf_size; unsigned int pos = 0;
if (config->driver->ops->pointer)
return config->driver->ops->pointer(substream);status = dmaengine_tx_status(prtd->dma_chan, prtd->cookie, &state); if (status == DMA_IN_PROGRESS || status == DMA_PAUSED) { buf_size = snd_pcm_lib_buffer_bytes(substream);
@@ -341,6 +360,12 @@ EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open_request_chan); int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream) { struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform);
const struct snd_dmaengine_pcm_config *config = pcm->config;
if (config->driver->ops->close)
return config->driver->ops->close(substream);kfree(prtd);
diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c index e29ec3c..6e66c53 100644 --- a/sound/soc/soc-generic-dmaengine-pcm.c +++ b/sound/soc/soc-generic-dmaengine-pcm.c @@ -24,18 +24,6 @@
#include <sound/dmaengine_pcm.h>
-struct dmaengine_pcm {
- struct dma_chan *chan[SNDRV_PCM_STREAM_CAPTURE + 1];
- const struct snd_dmaengine_pcm_config *config;
- struct snd_soc_platform platform;
- unsigned int flags;
-};
-static struct dmaengine_pcm *soc_platform_to_pcm(struct snd_soc_platform *p) -{
- return container_of(p, struct dmaengine_pcm, platform);
-}
/**
- snd_dmaengine_pcm_prepare_slave_config() - Generic prepare_slave_config callback
- @substream: PCM substream
@@ -75,9 +63,13 @@ static int dmaengine_pcm_hw_params(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 dma_chan *chan = snd_dmaengine_pcm_get_chan(substream);
const struct snd_dmaengine_pcm_config *config = pcm->config; struct dma_slave_config slave_config; int ret;
if (config->driver->ops->hw_params)
return config->driver->ops->hw_params(substream, params);if (pcm->config->prepare_slave_config) { ret = pcm->config->prepare_slave_config(substream, params, &slave_config);
@@ -96,9 +88,13 @@ static int dmaengine_pcm_open(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; struct dmaengine_pcm *pcm = soc_platform_to_pcm(rtd->platform);
const struct snd_dmaengine_pcm_config *config = pcm->config; struct dma_chan *chan = pcm->chan[substream->stream]; int ret;
if (config->driver->ops->open)
return config->driver->ops->open(substream);ret = snd_soc_set_runtime_hwparams(substream, pcm->config->pcm_hardware); if (ret)
@@ -118,6 +114,13 @@ static struct device *dmaengine_dma_dev(struct dmaengine_pcm *pcm,
static void dmaengine_pcm_free(struct snd_pcm *pcm) {
- struct snd_soc_pcm_runtime *rtd = pcm->private_data;
- struct dmaengine_pcm *dma_pcm = soc_platform_to_pcm(rtd->platform);
- const struct snd_dmaengine_pcm_config *config = dma_pcm->config;
- if (config->driver->pcm_free)
return config->driver->pcm_free(pcm);- snd_pcm_lib_preallocate_free_for_all(pcm);
}
@@ -145,6 +148,9 @@ static int dmaengine_pcm_new(struct snd_soc_pcm_runtime *rtd) unsigned int i; int ret;
- if (config->driver->pcm_new)
return config->driver->pcm_new(rtd);- for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_CAPTURE; i++) { substream = rtd->pcm->streams[i].substream; if (!substream)
On 10/15/2013 10:22 AM, Nicolin Chen wrote:
Hi Lars,
On Tue, Oct 15, 2013 at 10:08:52AM +0200, Lars-Peter Clausen wrote:
On 10/15/2013 09:49 AM, Nicolin Chen wrote:
Each implementation of gerneric pcm dmaengine has been naturally limited by the pre-defined gerneric functions. Thus add an extra interface for user to create a backdoor so that they can override those fixed functions for some uncommon requirements: using on-chip memory for DMA buffers and accordingly different mmap function for example, while at the meantime, they can continue to benifit from the concise and wise generic pcm dmaengine.
Signed-off-by: Nicolin Chen b42378@freescale.com
We do have the dmaengine pcm helper functions (sound/core/pcm_dmaengine.c) and the generic dmaengine pcm ASoC driver (sound/soc/soc-generic-dmaengine-pcm.c). The generic dmaengine pcm ASoC driver uses the dmaengine pcm helper functions, but it is not the only user of them. So your patch breaks all other users of the helper functions.
The helper functions are designed in a way that you can either wrap them in your own pcm driver driver or not use them at all. E.g. take a look at sound/soc/omap/omap-pcm.c on how to overwrite specific functions.
- Lars
If this modification to sound/core/pcm_dmaengine.c is not fair, what about moving the modification to soc-gerneric-dmaengine-pcm.c?
For example, instead of using snd_dmaengine_pcm_trigger() directly in gerneric soc dmaengine driver, we create a new one inside gerneric soc dmaengine driver and check the override:
static int snd_soc_dmaengine_pcm_trigger(substream, cmd) { if (config->driver->ops->trigger) return config->driver->ops->trigger(substream, cmd); return snd_dmaengine_pcm_trigger(substream, cmd); }
Since it happens inside generic dmaengine pcm ASoC driver, it won't break those helper functions.
I'm trying this just because I hope generic dmaengine can be more flexible Admittedly, using helper functions might be more plausible way in current ASoC structure. However, there might be so much change for an generic soc dmaengine implementation even if it just wants to override one single func.
Well the idea of the generic dmaengine driver is to be generic and not require SoC specific hacks since those should not be necessary if things are done right. What exactly do you want to implement in the overwritten ops?
- Lars
On Tue, Oct 15, 2013 at 11:02:44AM +0200, Lars-Peter Clausen wrote:
If this modification to sound/core/pcm_dmaengine.c is not fair, what about moving the modification to soc-gerneric-dmaengine-pcm.c?
For example, instead of using snd_dmaengine_pcm_trigger() directly in gerneric soc dmaengine driver, we create a new one inside gerneric soc dmaengine driver and check the override:
static int snd_soc_dmaengine_pcm_trigger(substream, cmd) { if (config->driver->ops->trigger) return config->driver->ops->trigger(substream, cmd); return snd_dmaengine_pcm_trigger(substream, cmd); }
Since it happens inside generic dmaengine pcm ASoC driver, it won't break those helper functions.
I'm trying this just because I hope generic dmaengine can be more flexible Admittedly, using helper functions might be more plausible way in current ASoC structure. However, there might be so much change for an generic soc dmaengine implementation even if it just wants to override one single func.
Well the idea of the generic dmaengine driver is to be generic and not require SoC specific hacks since those should not be necessary if things are done right. What exactly do you want to implement in the overwritten ops?
- Lars
Just like I mentioned in the commit comments, using on-chip memory would need an alternative memory allocating function to replace the default one from external DDR. imx-pcm-dma.c is now using ASoC generic dmaengine, so if we turn it back to helper functions way, all cpu dai drivers might have to be turned back as well.
ASoC generic dmaengine is quite concise and well-developed for us to use. But after using it, its own limitation might make user waver to choose the old fashion helper functions. So I just wish it could provide a back door for an easy trick. Then we can continue to enjoy the convenience from it and also be able to tackle these uncommon cases.
Thank you, Nicolin Chen
On 10/15/2013 11:23 AM, Nicolin Chen wrote:
On Tue, Oct 15, 2013 at 11:02:44AM +0200, Lars-Peter Clausen wrote:
If this modification to sound/core/pcm_dmaengine.c is not fair, what about moving the modification to soc-gerneric-dmaengine-pcm.c?
For example, instead of using snd_dmaengine_pcm_trigger() directly in gerneric soc dmaengine driver, we create a new one inside gerneric soc dmaengine driver and check the override:
static int snd_soc_dmaengine_pcm_trigger(substream, cmd) { if (config->driver->ops->trigger) return config->driver->ops->trigger(substream, cmd); return snd_dmaengine_pcm_trigger(substream, cmd); }
Since it happens inside generic dmaengine pcm ASoC driver, it won't break those helper functions.
I'm trying this just because I hope generic dmaengine can be more flexible Admittedly, using helper functions might be more plausible way in current ASoC structure. However, there might be so much change for an generic soc dmaengine implementation even if it just wants to override one single func.
Well the idea of the generic dmaengine driver is to be generic and not require SoC specific hacks since those should not be necessary if things are done right. What exactly do you want to implement in the overwritten ops?
- Lars
Just like I mentioned in the commit comments, using on-chip memory would need an alternative memory allocating function to replace the default one from external DDR. imx-pcm-dma.c is now using ASoC generic dmaengine, so if we turn it back to helper functions way, all cpu dai drivers might have to be turned back as well.
ASoC generic dmaengine is quite concise and well-developed for us to use. But after using it, its own limitation might make user waver to choose the old fashion helper functions. So I just wish it could provide a back door for an easy trick. Then we can continue to enjoy the convenience from it and also be able to tackle these uncommon cases.
Wanting to use on-chip memory for the audio buffers is not that uncommon. I'd rather see this implemented generically in the generic driver instead of each SoC implementing its own version.
- Lars
On Tue, Oct 15, 2013 at 11:40:10AM +0200, Lars-Peter Clausen wrote:
On 10/15/2013 11:23 AM, Nicolin Chen wrote:
On Tue, Oct 15, 2013 at 11:02:44AM +0200, Lars-Peter Clausen wrote:
If this modification to sound/core/pcm_dmaengine.c is not fair, what about moving the modification to soc-gerneric-dmaengine-pcm.c?
For example, instead of using snd_dmaengine_pcm_trigger() directly in gerneric soc dmaengine driver, we create a new one inside gerneric soc dmaengine driver and check the override:
static int snd_soc_dmaengine_pcm_trigger(substream, cmd) { if (config->driver->ops->trigger) return config->driver->ops->trigger(substream, cmd); return snd_dmaengine_pcm_trigger(substream, cmd); }
Since it happens inside generic dmaengine pcm ASoC driver, it won't break those helper functions.
I'm trying this just because I hope generic dmaengine can be more flexible Admittedly, using helper functions might be more plausible way in current ASoC structure. However, there might be so much change for an generic soc dmaengine implementation even if it just wants to override one single func.
Well the idea of the generic dmaengine driver is to be generic and not require SoC specific hacks since those should not be necessary if things are done right. What exactly do you want to implement in the overwritten ops?
- Lars
Just like I mentioned in the commit comments, using on-chip memory would need an alternative memory allocating function to replace the default one from external DDR. imx-pcm-dma.c is now using ASoC generic dmaengine, so if we turn it back to helper functions way, all cpu dai drivers might have to be turned back as well.
ASoC generic dmaengine is quite concise and well-developed for us to use. But after using it, its own limitation might make user waver to choose the old fashion helper functions. So I just wish it could provide a back door for an easy trick. Then we can continue to enjoy the convenience from it and also be able to tackle these uncommon cases.
Wanting to use on-chip memory for the audio buffers is not that uncommon. I'd rather see this implemented generically in the generic driver instead of each SoC implementing its own version.
- Lars
Is that so? Actually, I did as you said in my internal branch, but I just fear that might not be easy to upstream, so I tried to figure out another plausible way. It looks like I took a detour :)
Thank you for the advice. Nicolin Chen
On Tue, Oct 15, 2013 at 05:37:17PM +0800, Nicolin Chen wrote:
On Tue, Oct 15, 2013 at 11:40:10AM +0200, Lars-Peter Clausen wrote:
Wanting to use on-chip memory for the audio buffers is not that uncommon. I'd rather see this implemented generically in the generic driver instead of each SoC implementing its own version.
Is that so? Actually, I did as you said in my internal branch, but I just fear that might not be easy to upstream, so I tried to figure out another plausible way. It looks like I took a detour :)
Sounds like your first idea was the best one! I don't know about anyone else but personally I'd be really happy to see the framework support for these on chip mmemories improved in Linux, there's quite a few chips aren't taking advantage of them at the minute due to the lack of a clear way to do so.
On Tue, Oct 15, 2013 at 11:40:10AM +0200, Lars-Peter Clausen wrote:
Wanting to use on-chip memory for the audio buffers is not that uncommon. I'd rather see this implemented generically in the generic driver instead of each SoC implementing its own version.
I agree - to me it looks like the major problm here is that the handling of the on chip memory tends to vary widely between devices rather than that there are different problems.
participants (3)
-
Lars-Peter Clausen -
Mark Brown -
Nicolin Chen