[alsa-devel] [RFC PATCH 0/5] OMAP/ASoC: McBSP: FIFO handling related fixes
Hello,
This series aims to correct how the McBSP FIFO is viewed, and handled.
Introduction of the problem: OMAP McBSP FIFO is word structured: McBSP2 has 1024 + 256 = 1280 word long buffer, McBSP1,3,4,5 has 128 word long buffer
This means, that the size of the FIFO depends on the McBSP word size configuration. For example on McBSP3: 16bit samples: size is 128 * 2 = 256 bytes 32bit samples: size is 128 * 4 = 512 bytes It is simpler to place constraint for buffer and period based on channels. McBSP3 as example again (16 or 32 bit samples): 1 channel (mono): size is 128 frames (128 words) 2 channels (stereo): size is 128 / 2 = 64 frames (2 * 64 words) 4 channels: size is 128 / 4 = 32 frames (4 * 32 words)
Since now the McBSP codec supports not only 16bit samples (32biut has been recently added), the FIFO size handling is no longer correct, since it has been hard wired for 16bit word length.
The series changes how the users of McBSP are configuring the FIFO: It used to be 0 based (0 meant 1 word threshold). After this series users can configure the threshold in 1 base mode (1 means 1 word threshold). The platform code now provides the _full_ size of the FIFO in words, instead of the already limited value used in the past.
In ASoC omap-mcbsp code hw_rule based constraint refinement is going to be used instead of the hardwired static constraint, which was correct only in case of 16bit word length.
The hw_rule is refining the minimum buffer size based on the channel number going to be used by the coming stream. In case of threshold mode additional hw_rule refines the maximum allowed period size.
The series are generated agains Takashi's sound-2.6: topic/asoc branch.
CCing also Eduardo, and Eero since they have worked on the original FIFO/threshold implementation.
All commetns and testers are welcome! Peter
--- Peter Ujfalusi (5): OMAP: McBSP: Function to query the FIFO size OMAP3: McBSP: Change the way how the FIFO is handled OMAP3: McBSP: Use the port's buffer_size when calculating tx delay ASoC: omap-mcbsp: Save, and use wlen for threshold configuration ASoC: omap-mcbsp: Place correct constraints for streams
arch/arm/mach-omap2/mcbsp.c | 10 ++-- arch/arm/plat-omap/include/plat/mcbsp.h | 2 + arch/arm/plat-omap/mcbsp.c | 31 ++++++--- sound/soc/omap/omap-mcbsp.c | 112 +++++++++++++++++++++++------- 4 files changed, 114 insertions(+), 41 deletions(-)
Users of McBSP can use the omap_mcbsp_get_fifo_size function to query the size of the McBSP FIFO. The function will return the FIFO size in words (the HW maximum).
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- arch/arm/plat-omap/include/plat/mcbsp.h | 2 ++ arch/arm/plat-omap/mcbsp.c | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h index 1bd7021..e126951 100644 --- a/arch/arm/plat-omap/include/plat/mcbsp.h +++ b/arch/arm/plat-omap/include/plat/mcbsp.h @@ -473,6 +473,7 @@ void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold); void omap_mcbsp_set_rx_threshold(unsigned int id, u16 threshold); u16 omap_mcbsp_get_max_tx_threshold(unsigned int id); u16 omap_mcbsp_get_max_rx_threshold(unsigned int id); +u16 omap_mcbsp_get_fifo_size(unsigned int id); u16 omap_mcbsp_get_tx_delay(unsigned int id); u16 omap_mcbsp_get_rx_delay(unsigned int id); int omap_mcbsp_get_dma_op_mode(unsigned int id); @@ -483,6 +484,7 @@ static inline void omap_mcbsp_set_rx_threshold(unsigned int id, u16 threshold) { } static inline u16 omap_mcbsp_get_max_tx_threshold(unsigned int id) { return 0; } static inline u16 omap_mcbsp_get_max_rx_threshold(unsigned int id) { return 0; } +static inline u16 omap_mcbsp_get_fifo_size(unsigned int id) { return 0; } static inline u16 omap_mcbsp_get_tx_delay(unsigned int id) { return 0; } static inline u16 omap_mcbsp_get_rx_delay(unsigned int id) { return 0; } static inline int omap_mcbsp_get_dma_op_mode(unsigned int id) { return 0; } diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c index 4820cab..51d8abf 100644 --- a/arch/arm/plat-omap/mcbsp.c +++ b/arch/arm/plat-omap/mcbsp.c @@ -559,6 +559,20 @@ u16 omap_mcbsp_get_max_rx_threshold(unsigned int id) } EXPORT_SYMBOL(omap_mcbsp_get_max_rx_threshold);
+u16 omap_mcbsp_get_fifo_size(unsigned int id) +{ + struct omap_mcbsp *mcbsp; + + if (!omap_mcbsp_check_valid_id(id)) { + printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1); + return -ENODEV; + } + mcbsp = id_to_mcbsp_ptr(id); + + return mcbsp->pdata->buffer_size; +} +EXPORT_SYMBOL(omap_mcbsp_get_fifo_size); + #define MCBSP2_FIFO_SIZE 0x500 /* 1024 + 256 locations */ #define MCBSP1345_FIFO_SIZE 0x80 /* 128 locations */ /*
Use the actual FIFO size in words as buffer_size on OMAP2. Change the threshold configuration to use 1 based numbering, when specifying the allowed threshold maximum or the McBSP threshold value. Set the default maximum threshold to (buffer_size - 0x10) intialy.
Sicne the platform data's buffer_size now holds the full size of the FIFO, there is no longer need to handle the ports differently.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- arch/arm/plat-omap/mcbsp.c | 7 +------ 1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c index 6462968..f8ffeee 100644 --- a/arch/arm/plat-omap/mcbsp.c +++ b/arch/arm/plat-omap/mcbsp.c @@ -575,8 +575,6 @@ u16 omap_mcbsp_get_fifo_size(unsigned int id) } EXPORT_SYMBOL(omap_mcbsp_get_fifo_size);
-#define MCBSP2_FIFO_SIZE 0x500 /* 1024 + 256 locations */ -#define MCBSP1345_FIFO_SIZE 0x80 /* 128 locations */ /* * omap_mcbsp_get_tx_delay returns the number of used slots in the McBSP FIFO */ @@ -595,10 +593,7 @@ u16 omap_mcbsp_get_tx_delay(unsigned int id) buffstat = MCBSP_READ(mcbsp, XBUFFSTAT);
/* Number of slots are different in McBSP ports */ - if (mcbsp->id == 2) - return MCBSP2_FIFO_SIZE - buffstat; - else - return MCBSP1345_FIFO_SIZE - buffstat; + return mcbsp->pdata->buffer_size - buffstat; } EXPORT_SYMBOL(omap_mcbsp_get_tx_delay);
Save the word length configuration of McBSP, and use that information to calculate, and configure the threshold in McBSP. Previously the calculation was only correct when the stream had 16bit audio.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/omap/omap-mcbsp.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 6f44cb4..b06d8f1 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -59,6 +59,7 @@ struct omap_mcbsp_data { int configured; unsigned int in_freq; int clk_div; + int wlen; };
#define to_mcbsp(priv) container_of((priv), struct omap_mcbsp_data, bus_id) @@ -155,19 +156,21 @@ static void omap_mcbsp_set_threshold(struct snd_pcm_substream *substream) struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai; struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data); int dma_op_mode = omap_mcbsp_get_dma_op_mode(mcbsp_data->bus_id); - int samples; + int words;
/* TODO: Currently, MODE_ELEMENT == MODE_FRAME */ if (dma_op_mode == MCBSP_DMA_MODE_THRESHOLD) - samples = snd_pcm_lib_period_bytes(substream) >> 1; + /* The FIFO size depends on the McBSP word configuration */ + words = snd_pcm_lib_period_bytes(substream) / + (mcbsp_data->wlen / 8); else - samples = 1; + words = 1;
/* Configure McBSP internal buffer usage */ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - omap_mcbsp_set_tx_threshold(mcbsp_data->bus_id, samples - 1); + omap_mcbsp_set_tx_threshold(mcbsp_data->bus_id, words); else - omap_mcbsp_set_rx_threshold(mcbsp_data->bus_id, samples - 1); + omap_mcbsp_set_rx_threshold(mcbsp_data->bus_id, words); }
static int omap_mcbsp_dai_startup(struct snd_pcm_substream *substream, @@ -409,6 +412,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, }
omap_mcbsp_config(bus_id, &mcbsp_data->regs); + mcbsp_data->wlen = wlen; mcbsp_data->configured = 1;
return 0;
OMAP McBSP FIFO is word structured: McBSP2 has 1024 + 256 = 1280 word long buffer, McBSP1,3,4,5 has 128 word long buffer
This means, that the size of the FIFO depends on the McBSP word size configuration. For example on McBSP3: 16bit samples: size is 128 * 2 = 256 bytes 32bit samples: size is 128 * 4 = 512 bytes It is simpler to place constraint for buffer and period based on channels. McBSP3 as example again (16 or 32 bit samples): 1 channel (mono): size is 128 frames (128 words) 2 channels (stereo): size is 128 / 2 = 64 frames (2 * 64 words) 4 channels: size is 128 / 4 = 32 frames (4 * 32 words)
Use the former method to place hw_rule on buffer size, and in threshold mode to period size.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/omap/omap-mcbsp.c | 98 +++++++++++++++++++++++++++++++++--------- 1 files changed, 77 insertions(+), 21 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index b06d8f1..d6ed761 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -173,6 +173,52 @@ static void omap_mcbsp_set_threshold(struct snd_pcm_substream *substream) omap_mcbsp_set_rx_threshold(mcbsp_data->bus_id, words); }
+static int hw_rule_bsize_by_channels(struct snd_pcm_hw_params *params, + struct snd_pcm_hw_rule *rule) +{ + struct snd_interval *bs = hw_param_interval(params, + SNDRV_PCM_HW_PARAM_BUFFER_SIZE); + struct snd_interval *c = hw_param_interval(params, + SNDRV_PCM_HW_PARAM_CHANNELS); + struct omap_mcbsp_data *mcbsp_data = rule->private; + struct snd_interval frames; + int size; + + snd_interval_any(&frames); + size = omap_mcbsp_get_fifo_size(mcbsp_data->bus_id); + + frames.min = size / c->min; + frames.integer = 1; + return snd_interval_refine(bs, &frames); + +} + +static int hw_rule_psize_by_channels(struct snd_pcm_hw_params *params, + struct snd_pcm_hw_rule *rule) +{ + struct snd_interval *ps = hw_param_interval(params, + SNDRV_PCM_HW_PARAM_PERIOD_SIZE); + struct snd_interval *c = hw_param_interval(params, + SNDRV_PCM_HW_PARAM_CHANNELS); + struct snd_pcm_substream *substream = rule->private; + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai; + struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data); + struct snd_interval frames; + int size; + + snd_interval_any(&frames); + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + size = omap_mcbsp_get_max_tx_threshold(mcbsp_data->bus_id); + else + size = omap_mcbsp_get_max_rx_threshold(mcbsp_data->bus_id); + + frames.min = 8; + frames.max = size / c->min; + frames.integer = 1; + return snd_interval_refine(ps, &frames); +} + static int omap_mcbsp_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { @@ -185,33 +231,43 @@ static int omap_mcbsp_dai_startup(struct snd_pcm_substream *substream, if (!cpu_dai->active) err = omap_mcbsp_request(bus_id);
+ /* + * OMAP McBSP FIFO is word structured. + * McBSP2 has 1024 + 256 = 1280 word long buffer, + * McBSP1,3,4,5 has 128 word long buffer + * This means that the size of the FIFO depends on the sample format. + * For example on McBSP3: + * 16bit samples: size is 128 * 2 = 256 bytes + * 32bit samples: size is 128 * 4 = 512 bytes + * It is simpler to place constraint for buffer and period based on + * channels. + * McBSP3 as example again (16 or 32 bit samples): + * 1 channel (mono): size is 128 frames (128 words) + * 2 channels (stereo): size is 128 / 2 = 64 frames (2 * 64 words) + * 4 channels: size is 128 / 4 = 32 frames (4 * 32 words) + */ + + /* + * The first rule is for the buffer size, we should not allow smaller + * buffer than the FIFO size to avoid underruns + */ + snd_pcm_hw_rule_add(substream->runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, + hw_rule_bsize_by_channels, mcbsp_data, + SNDRV_PCM_HW_PARAM_BUFFER_SIZE, -1); + if (cpu_is_omap343x()) { int dma_op_mode = omap_mcbsp_get_dma_op_mode(bus_id); - int max_period;
/* - * McBSP2 in OMAP3 has 1024 * 32-bit internal audio buffer. - * Set constraint for minimum buffer size to the same than FIFO - * size in order to avoid underruns in playback startup because - * HW is keeping the DMA request active until FIFO is filled. + * In case os threshold mode, the rule will ensure, that the + * period size is not bigger than the maximum allowed threshold + * value. */ - if (bus_id == 1) - snd_pcm_hw_constraint_minmax(substream->runtime, - SNDRV_PCM_HW_PARAM_BUFFER_BYTES, - 4096, UINT_MAX); - - 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; - if (dma_op_mode == MCBSP_DMA_MODE_THRESHOLD) - snd_pcm_hw_constraint_minmax(substream->runtime, - SNDRV_PCM_HW_PARAM_PERIOD_BYTES, - 32, max_period); + snd_pcm_hw_rule_add(substream->runtime, 0, + SNDRV_PCM_HW_PARAM_CHANNELS, + hw_rule_psize_by_channels, substream, + SNDRV_PCM_HW_PARAM_PERIOD_SIZE, -1); }
return err;
On Monday 31 May 2010 11:03:26 Ujfalusi Peter (Nokia-D/Tampere) wrote:
OMAP McBSP FIFO is word structured: McBSP2 has 1024 + 256 = 1280 word long buffer, McBSP1,3,4,5 has 128 word long buffer
This means, that the size of the FIFO depends on the McBSP word size configuration. For example on McBSP3: 16bit samples: size is 128 * 2 = 256 bytes 32bit samples: size is 128 * 4 = 512 bytes It is simpler to place constraint for buffer and period based on channels. McBSP3 as example again (16 or 32 bit samples): 1 channel (mono): size is 128 frames (128 words) 2 channels (stereo): size is 128 / 2 = 64 frames (2 * 64 words) 4 channels: size is 128 / 4 = 32 frames (4 * 32 words)
Use the former method to place hw_rule on buffer size, and in threshold mode to period size.
I meant later (second) method ;)
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
sound/soc/omap/omap-mcbsp.c | 98 +++++++++++++++++++++++++++++++++--------- 1 files changed, 77 insertions(+), 21 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index b06d8f1..d6ed761 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -173,6 +173,52 @@ static void omap_mcbsp_set_threshold(struct snd_pcm_substream *substream) omap_mcbsp_set_rx_threshold(mcbsp_data->bus_id, words); }
+static int hw_rule_bsize_by_channels(struct snd_pcm_hw_params *params,
struct snd_pcm_hw_rule *rule)+{
- struct snd_interval *bs = hw_param_interval(params,
SNDRV_PCM_HW_PARAM_BUFFER_SIZE);- struct snd_interval *c = hw_param_interval(params,
SNDRV_PCM_HW_PARAM_CHANNELS);- struct omap_mcbsp_data *mcbsp_data = rule->private;
- struct snd_interval frames;
- int size;
- snd_interval_any(&frames);
- size = omap_mcbsp_get_fifo_size(mcbsp_data->bus_id);
- frames.min = size / c->min;
- frames.integer = 1;
- return snd_interval_refine(bs, &frames);
+}
+static int hw_rule_psize_by_channels(struct snd_pcm_hw_params *params,
struct snd_pcm_hw_rule *rule)+{
- struct snd_interval *ps = hw_param_interval(params,
SNDRV_PCM_HW_PARAM_PERIOD_SIZE);- struct snd_interval *c = hw_param_interval(params,
SNDRV_PCM_HW_PARAM_CHANNELS);- struct snd_pcm_substream *substream = rule->private;
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
- struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data);
- struct snd_interval frames;
- int size;
- snd_interval_any(&frames);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
size = omap_mcbsp_get_max_tx_threshold(mcbsp_data->bus_id);- else
size = omap_mcbsp_get_max_rx_threshold(mcbsp_data->bus_id);- frames.min = 8;
- frames.max = size / c->min;
- frames.integer = 1;
- return snd_interval_refine(ps, &frames);
+}
static int omap_mcbsp_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { @@ -185,33 +231,43 @@ static int omap_mcbsp_dai_startup(struct snd_pcm_substream *substream, if (!cpu_dai->active) err = omap_mcbsp_request(bus_id);
- /*
* OMAP McBSP FIFO is word structured.* McBSP2 has 1024 + 256 = 1280 word long buffer,* McBSP1,3,4,5 has 128 word long buffer* This means that the size of the FIFO depends on the sample format.* For example on McBSP3:* 16bit samples: size is 128 * 2 = 256 bytes* 32bit samples: size is 128 * 4 = 512 bytes* It is simpler to place constraint for buffer and period based on* channels.* McBSP3 as example again (16 or 32 bit samples):* 1 channel (mono): size is 128 frames (128 words)* 2 channels (stereo): size is 128 / 2 = 64 frames (2 * 64 words)* 4 channels: size is 128 / 4 = 32 frames (4 * 32 words)*/- /*
* The first rule is for the buffer size, we should not allow smaller* buffer than the FIFO size to avoid underruns*/- snd_pcm_hw_rule_add(substream->runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
hw_rule_bsize_by_channels, mcbsp_data,SNDRV_PCM_HW_PARAM_BUFFER_SIZE, -1);- if (cpu_is_omap343x()) { int dma_op_mode = omap_mcbsp_get_dma_op_mode(bus_id);
int max_period;/*
* McBSP2 in OMAP3 has 1024 * 32-bit internal audio buffer.* Set constraint for minimum buffer size to the same than FIFO* size in order to avoid underruns in playback startup because* HW is keeping the DMA request active until FIFO is filled.
* In case os threshold mode, the rule will ensure, that the* period size is not bigger than the maximum allowed threshold* value.
if (bus_id == 1)snd_pcm_hw_constraint_minmax(substream->runtime,SNDRV_PCM_HW_PARAM_BUFFER_BYTES,4096, UINT_MAX);if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)max_period = omap_mcbsp_get_max_tx_threshold(bus_id);elsemax_period = omap_mcbsp_get_max_rx_threshold(bus_id);max_period++;max_period <<= 1;- if (dma_op_mode == MCBSP_DMA_MODE_THRESHOLD)
snd_pcm_hw_constraint_minmax(substream->runtime,SNDRV_PCM_HW_PARAM_PERIOD_BYTES,32, max_period);
snd_pcm_hw_rule_add(substream->runtime, 0,SNDRV_PCM_HW_PARAM_CHANNELS,hw_rule_psize_by_channels, substream,SNDRV_PCM_HW_PARAM_PERIOD_SIZE, -1);}
return err;
And I managed to mistype linux-omap... Resending. Sorry for the noise.
On Mon, 31 May 2010 11:03:21 +0300 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
Looks good move forward with the notes by you and others. Few questions below.
This means, that the size of the FIFO depends on the McBSP word size configuration. For example on McBSP3: 16bit samples: size is 128 * 2 = 256 bytes 32bit samples: size is 128 * 4 = 512 bytes It is simpler to place constraint for buffer and period based on channels. McBSP3 as example again (16 or 32 bit samples): 1 channel (mono): size is 128 frames (128 words) 2 channels (stereo): size is 128 / 2 = 64 frames (2 * 64 words) 4 channels: size is 128 / 4 = 32 frames (4 * 32 words)
So McBSP FIFO holds always samples independently of number of channels or sample size? That makes sense why I see DMA pointer advancing at 512 frame steps when max_tx_thres is 1024 (aplay -f dat /dev/zero).
The series changes how the users of McBSP are configuring the FIFO: It used to be 0 based (0 meant 1 word threshold). After this series users can configure the threshold in 1 base mode (1 means 1 word threshold). The platform code now provides the _full_ size of the FIFO in words, instead of the already limited value used in the past.
Definitely good. It's much more easier to remember and define real threshold size than some -1 what's ends up to threshold register value :-)
Btw, I don't remember what was the reason why maximum threshold value must be -0x10 from FIFO size?
participants (2)
-
Jarkko Nikula -
Peter Ujfalusi