| Message ID | 1248958183-15015-17-git-send-email-eduardo.valentin@nokia.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
On Thu, 30 Jul 2009 15:49:39 +0300 Eduardo Valentin <eduardo.valentin@nokia.com> wrote: > Now this patch implements again the McBSP threshold > usage for OMAP ASoC. > > We figured out that there is no need to have so much > SW control in order to have DMA in idle state during > audio streaming. Configuring McBSP threshold value > and DMA to FRAME_SYNC are sufficient. > > Signed-off-by: Eduardo Valentin <eduardo.valentin@nokia.com> > --- > sound/soc/omap/omap-pcm.c | 37 +++++++++++++++++++++++++++++++++++-- > 1 files changed, 35 insertions(+), 2 deletions(-) > > @@ -192,6 +203,12 @@ static int omap_pcm_trigger(struct snd_pcm_substream *substream, int cmd) > case SNDRV_PCM_TRIGGER_RESUME: > case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: > prtd->period_index = 0; > + /* Configure McBSP internal buffer usage */ > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) > + omap_mcbsp_set_tx_threshold(bus_id, samples - 1); > + else > + omap_mcbsp_set_rx_threshold(bus_id, samples - 1); > + > omap_start_dma(prtd->dma_ch); > break; > Oops, didn't notice this before. This will hard glue the DMA and McBSP together. Even currently there is only McBSP based DAI link driver, there can be others as well. EAC DAI for OMAP2420 would be necessary for instance if one wants to develop ASoC support for Nokia N800. Nokia N810 could use that too.
Hi Jarkko, On Wed, Aug 05, 2009 at 09:48:56AM +0200, ext Jarkko Nikula wrote: > On Thu, 30 Jul 2009 15:49:39 +0300 > Eduardo Valentin <eduardo.valentin@nokia.com> wrote: > > > Now this patch implements again the McBSP threshold > > usage for OMAP ASoC. > > > > We figured out that there is no need to have so much > > SW control in order to have DMA in idle state during > > audio streaming. Configuring McBSP threshold value > > and DMA to FRAME_SYNC are sufficient. > > > > Signed-off-by: Eduardo Valentin <eduardo.valentin@nokia.com> > > --- > > sound/soc/omap/omap-pcm.c | 37 +++++++++++++++++++++++++++++++++++-- > > 1 files changed, 35 insertions(+), 2 deletions(-) > > > > @@ -192,6 +203,12 @@ static int omap_pcm_trigger(struct snd_pcm_substream *substream, int cmd) > > case SNDRV_PCM_TRIGGER_RESUME: > > case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: > > prtd->period_index = 0; > > + /* Configure McBSP internal buffer usage */ > > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) > > + omap_mcbsp_set_tx_threshold(bus_id, samples - 1); > > + else > > + omap_mcbsp_set_rx_threshold(bus_id, samples - 1); > > + > > omap_start_dma(prtd->dma_ch); > > break; > > > Oops, didn't notice this before. This will hard glue the DMA and McBSP > together. Even currently there is only McBSP based DAI link driver, > there can be others as well. EAC DAI for OMAP2420 would be necessary > for instance if one wants to develop ASoC support for Nokia N800. Nokia > N810 could use that too. True. And the same comment is valid for the patch which adds op mode selection. My idea to un-glue them is to put some callbacks in omap_mcbsp_data of omap-mcbsp.c, and share it as something more generic, so that they can be called from omap-pcm.c. Something like: struct omap_dma_data { unsigned int bus_id; unsigned int fmt; /* * Flags indicating is the bus already activated and configured by * another substream */ int active; int configured; /* threshold callbacks can be added then here */ void *data; /* struct omap_mcbsp_reg_cfg regs; can go here */ }; And this goes in omap-pcm.h. what do you think?? > > > -- > Jarkko
On Mon, 10 Aug 2009 11:53:55 +0300 Eduardo Valentin <eduardo.valentin@nokia.com> wrote: > > Oops, didn't notice this before. This will hard glue the DMA and McBSP > > together. Even currently there is only McBSP based DAI link driver, > > there can be others as well. EAC DAI for OMAP2420 would be necessary > > for instance if one wants to develop ASoC support for Nokia N800. Nokia > > N810 could use that too. > > True. And the same comment is valid for the patch which adds op mode selection. > My idea to un-glue them is to put some callbacks in omap_mcbsp_data of omap-mcbsp.c, > and share it as something more generic, so that they can be called from omap-pcm.c. > > Something like: > > struct omap_dma_data { > unsigned int bus_id; > unsigned int fmt; > /* > * Flags indicating is the bus already activated and configured by > * another substream > */ > int active; > int configured; > /* threshold callbacks can be added then here */ > void *data; /* struct omap_mcbsp_reg_cfg regs; can go here */ > }; > > And this goes in omap-pcm.h. > > what do you think?? > I'm thinking is it just enough to share sync_mode between the omap-pcm.c and omap-mcbsp.c? For me it looks that both period_bytes_max (by using snd_pcm_hw_constraint_minmax) adjustment and threshold setting can be done in omap-mcbsp.c. Then there is no need to invent any callbacks to the omap_dma_data (for now) and omap-pcm.c remains cleanear as the threshold is not so much DMA programming specific.
On Tue, Aug 11, 2009 at 07:34:45AM +0200, ext Jarkko Nikula wrote: > On Mon, 10 Aug 2009 11:53:55 +0300 > Eduardo Valentin <eduardo.valentin@nokia.com> wrote: > > > > Oops, didn't notice this before. This will hard glue the DMA and McBSP > > > together. Even currently there is only McBSP based DAI link driver, > > > there can be others as well. EAC DAI for OMAP2420 would be necessary > > > for instance if one wants to develop ASoC support for Nokia N800. Nokia > > > N810 could use that too. > > > > True. And the same comment is valid for the patch which adds op mode selection. > > My idea to un-glue them is to put some callbacks in omap_mcbsp_data of omap-mcbsp.c, > > and share it as something more generic, so that they can be called from omap-pcm.c. > > > > Something like: > > > > struct omap_dma_data { > > unsigned int bus_id; > > unsigned int fmt; > > /* > > * Flags indicating is the bus already activated and configured by > > * another substream > > */ > > int active; > > int configured; > > /* threshold callbacks can be added then here */ > > void *data; /* struct omap_mcbsp_reg_cfg regs; can go here */ > > }; > > > > And this goes in omap-pcm.h. Ok, I was wrong here. My point was to add new stuff in here: struct omap_pcm_dma_data { char *name; /* stream identifier */ int dma_req; /* DMA request line */ unsigned long port_addr; /* transmit/receive register */ /* additional things go here */ }; This one is already shared. > > > > what do you think?? > > > I'm thinking is it just enough to share sync_mode between the > omap-pcm.c and omap-mcbsp.c? For me it looks that both period_bytes_max > (by using snd_pcm_hw_constraint_minmax) adjustment and threshold > setting can be done in omap-mcbsp.c. Then there is no need to invent > any callbacks to the omap_dma_data (for now) and omap-pcm.c remains > cleanear as the threshold is not so much DMA programming specific. True, that would become cleaner. But, there are some points to re-think here. Max threshold value is dependent on mcbsp instance (and possibly, on omap version as well, if something change for next omap versions). The way max threshold is implemented now, it comes first from board file, then you can query it using new added function in this series. So far nothing prevent us to move max threshold usage to omap-mcbsp.c. But at open time, we add a restriction to make buffer size a multiple of period size. And I think to properly make this, we need max threshold value in advance, as we the idea is to bind periods with thresholds. A similar issue may happen whilst setting current threshold value. We want to set it before starting DMA. And that value must be bind to chosen period size for that stream. In which place we can do that in omap-mcbsp.c? Thoughts? > > > -- > Jarkko
diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index 84a1950..03cb420 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -28,10 +28,11 @@ #include <sound/pcm_params.h> #include <sound/soc.h> +#include <mach/mcbsp.h> #include <mach/dma.h> #include "omap-pcm.h" -static const struct snd_pcm_hardware omap_pcm_hardware = { +static struct snd_pcm_hardware omap_pcm_hardware = { .info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_INTERLEAVED | @@ -135,6 +136,7 @@ static int omap_pcm_prepare(struct snd_pcm_substream *substream) struct omap_runtime_data *prtd = runtime->private_data; struct omap_pcm_dma_data *dma_data = prtd->dma_data; struct omap_dma_channel_params dma_params; + int sync_mode; /* return if this is a bufferless transfer e.g. * codec <--> BT codec or GSM modem -- lg FIXME */ @@ -142,13 +144,19 @@ static int omap_pcm_prepare(struct snd_pcm_substream *substream) return 0; memset(&dma_params, 0, sizeof(dma_params)); + + if (cpu_is_omap34xx()) + sync_mode = OMAP_DMA_SYNC_FRAME; + else + sync_mode = OMAP_DMA_SYNC_ELEMENT; + /* * Note: Regardless of interface data formats supported by OMAP McBSP * or EAC blocks, internal representation is always fixed 16-bit/sample */ dma_params.data_type = OMAP_DMA_DATA_TYPE_S16; dma_params.trigger = dma_data->dma_req; - dma_params.sync_mode = OMAP_DMA_SYNC_ELEMENT; + dma_params.sync_mode = sync_mode; if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { dma_params.src_amode = OMAP_DMA_AMODE_POST_INC; dma_params.dst_amode = OMAP_DMA_AMODE_CONSTANT; @@ -183,8 +191,11 @@ static int omap_pcm_trigger(struct snd_pcm_substream *substream, int cmd) { struct snd_pcm_runtime *runtime = substream->runtime; struct omap_runtime_data *prtd = runtime->private_data; + struct snd_soc_pcm_runtime *rtd = substream->private_data; unsigned long flags; int ret = 0; + unsigned int bus_id = *(unsigned int *)rtd->dai->cpu_dai->private_data; + u16 samples = snd_pcm_lib_period_bytes(substream) >> 1; spin_lock_irqsave(&prtd->lock, flags); switch (cmd) { @@ -192,6 +203,12 @@ static int omap_pcm_trigger(struct snd_pcm_substream *substream, int cmd) case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: prtd->period_index = 0; + /* Configure McBSP internal buffer usage */ + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + omap_mcbsp_set_tx_threshold(bus_id, samples - 1); + else + omap_mcbsp_set_rx_threshold(bus_id, samples - 1); + omap_start_dma(prtd->dma_ch); break; @@ -235,7 +252,23 @@ static int omap_pcm_open(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; struct omap_runtime_data *prtd; + struct snd_soc_pcm_runtime *rtd = substream->private_data; + unsigned int bus_id = *(unsigned int *)rtd->dai->cpu_dai->private_data; int ret; + int max_period; + + if (cpu_is_omap34xx()) { + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + max_period = omap_mcbsp_get_max_tx_threshold(bus_id); + else + max_period = omap_mcbsp_get_max_rx_threshold(bus_id); + max_period++; + max_period <<= 1; + } else { + omap_pcm_hardware.period_bytes_max = 64 * 1024; + } + + omap_pcm_hardware.period_bytes_max = max_period; snd_soc_set_runtime_hwparams(substream, &omap_pcm_hardware);
Now this patch implements again the McBSP threshold usage for OMAP ASoC. We figured out that there is no need to have so much SW control in order to have DMA in idle state during audio streaming. Configuring McBSP threshold value and DMA to FRAME_SYNC are sufficient. Signed-off-by: Eduardo Valentin <eduardo.valentin@nokia.com> --- sound/soc/omap/omap-pcm.c | 37 +++++++++++++++++++++++++++++++++++-- 1 files changed, 35 insertions(+), 2 deletions(-)