[alsa-devel] [PATCHv2 0/5] FIFO caused playback delay (latency) handling in soc/omap
Hello,
Changes since the RFC round on the alsa-devel list: - core is not limiting the query for the delay to the playback stream. It is no the dai and platform driver's responsibility handle that - delay interface added for platform driver - Capture path delay calculation is added to OMAP McBSP. - omap_mcbsp_get_tx_buffstat renamed as omap_mcbsp_get_tx_delay to reflect what it is actually doing.
Tony: is it still possible to get this taken for 2.6.34? It would be nice to avoid cross tree problems during the 34 cycle...
The initial RFC patch description:
There has been discussion in alsa-devel in this subject several times, but no actual patches has been sent (it was not soc related, but anyway it was discussed).
Based on the available information the latency caused by the HW buffer on some systems can be handled by updating the runtime->delay.
It has been discussed, that the runtime->delay can also be updated dynamically to show more accurate delay.
To further complicate things, in ASoC we could have more buffer in the chain. To handle this we need soc level support.
This series tries to do that in soc by: - introducing a pcm_pointer wrapper - in this wrapper we call the original pcm_pointer functions to get the DMA pointer - introducing a new interface in dai_ops and for platfrom drivers to ask the delay - adding the cpu_dai, codec_dai and platform driver returned delay to form the actual delay - update the runtime->delay with this value.
With this approach none of the existing drivers need change, but they can add support for specifying the FIFO caused delay.
In this series on top of the core changes the omap(3) code is updated to take this delay reporting into use. I have not added the support to the tlv320dac33 codec driver, since it needs a bit more work, but along the same line it can be done, and if the tlv320dac33 is hooked to omap McBSP than applications can know the whole delay/latency on that path.
The series is based on linux-omap master branch (with regcache and sidetone). However the first 3 patch for soc core applies on top of sound-2.6 topic/asoc cleanly as well. Obviously the last patch depends on the OMAP platform patch.
--- Peter Ujfalusi (5): ASoC: core: fix tailing whitespace in soc_pcm_apply_symmetry ASoC: core: soc level wrapper for pcm_pointer callback ASoC: core: Add delay operation to snd_soc_dai_ops OMAP3: McBSP: Add interface for FIFO caused delay query ASoC: OMAP3: Report delay caused by the internal FIFO
arch/arm/plat-omap/include/plat/mcbsp.h | 6 +++ arch/arm/plat-omap/mcbsp.c | 55 +++++++++++++++++++++++++++++++ include/sound/soc-dai.h | 6 +++ include/sound/soc.h | 7 ++++ sound/soc/omap/omap-mcbsp.c | 26 ++++++++++++++ sound/soc/soc-core.c | 39 +++++++++++++++++++++- 6 files changed, 137 insertions(+), 2 deletions(-)
My editor removes the tailing spaces, which causes problems when changing the soc-core.c Removing the space.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/soc-core.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index a03bac9..37c872e 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -315,7 +315,7 @@ static int soc_pcm_apply_symmetry(struct snd_pcm_substream *substream)
if (codec_dai->symmetric_rates || cpu_dai->symmetric_rates || machine->symmetric_rates) { - dev_dbg(card->dev, "Symmetry forces %dHz rate\n", + dev_dbg(card->dev, "Symmetry forces %dHz rate\n", machine->rate);
ret = snd_pcm_hw_constraint_minmax(substream->runtime,
On Wed, 2010-03-03 at 15:08 +0200, Peter Ujfalusi wrote:
My editor removes the tailing spaces, which causes problems when changing the soc-core.c Removing the space.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
Wake-up from McBSP ports are needed, especially when the THRESHOLD dma mode is in use for audio playback.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- arch/arm/mach-omap2/pm34xx.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c index 81ed252..6b17647 100644 --- a/arch/arm/mach-omap2/pm34xx.c +++ b/arch/arm/mach-omap2/pm34xx.c @@ -878,12 +878,16 @@ static void __init prcm_setup_regs(void) /* Enable wakeups in PER */ prm_write_mod_reg(OMAP3430_EN_GPIO2 | OMAP3430_EN_GPIO3 | OMAP3430_EN_GPIO4 | OMAP3430_EN_GPIO5 | - OMAP3430_EN_GPIO6 | OMAP3430_EN_UART3, + OMAP3430_EN_GPIO6 | OMAP3430_EN_UART3 | + OMAP3430_EN_MCBSP2 | OMAP3430_EN_MCBSP3 | + OMAP3430_EN_MCBSP4, OMAP3430_PER_MOD, PM_WKEN); /* and allow them to wake up MPU */ prm_write_mod_reg(OMAP3430_GRPSEL_GPIO2 | OMAP3430_EN_GPIO3 | OMAP3430_GRPSEL_GPIO4 | OMAP3430_EN_GPIO5 | - OMAP3430_GRPSEL_GPIO6 | OMAP3430_EN_UART3, + OMAP3430_GRPSEL_GPIO6 | OMAP3430_EN_UART3 | + OMAP3430_EN_MCBSP2 | OMAP3430_EN_MCBSP3 | + OMAP3430_EN_MCBSP4, OMAP3430_PER_MOD, OMAP3430_PM_MPUGRPSEL);
/* Don't attach IVA interrupts */
On Wednesday 03 March 2010 15:08:05 Ujfalusi Peter (Nokia-D/Tampere) wrote:
Wake-up from McBSP ports are needed, especially when the THRESHOLD dma mode is in use for audio playback.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Hmmm, please disregard this patch, I have forgot to clean up the directory. This has been already taken for 2.6.33 kernel. Only the [PATCHv2] prefix is valid for this series. Sorry for the noise.
On Wed, 3 Mar 2010 15:08:05 +0200 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
Wake-up from McBSP ports are needed, especially when the THRESHOLD dma mode is in use for audio playback.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
arch/arm/mach-omap2/pm34xx.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)
How about the commit e3d9329640e4b291cdd2c6d178774c28bba47d59 in mainline?
Create a soc level wrapper for pcm_pointer callback. This will facilitate the soc level handling of different HW buffers in the audio path.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/soc-core.c | 19 ++++++++++++++++++- 1 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 37c872e..de5223d 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -797,6 +797,23 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) return 0; }
+/* + * soc level wrapper for pointer callback + */ +static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_device *socdev = rtd->socdev; + struct snd_soc_card *card = socdev->card; + struct snd_soc_platform *platform = card->platform; + snd_pcm_uframes_t offset = 0; + + if (platform->pcm_ops->pointer) + offset = platform->pcm_ops->pointer(substream); + + return offset; +} + /* ASoC PCM operations */ static struct snd_pcm_ops soc_pcm_ops = { .open = soc_pcm_open, @@ -805,6 +822,7 @@ static struct snd_pcm_ops soc_pcm_ops = { .hw_free = soc_pcm_hw_free, .prepare = soc_pcm_prepare, .trigger = soc_pcm_trigger, + .pointer = soc_pcm_pointer, };
#ifdef CONFIG_PM @@ -1331,7 +1349,6 @@ static int soc_new_pcm(struct snd_soc_device *socdev, dai_link->pcm = pcm; pcm->private_data = rtd; soc_pcm_ops.mmap = platform->pcm_ops->mmap; - soc_pcm_ops.pointer = platform->pcm_ops->pointer; soc_pcm_ops.ioctl = platform->pcm_ops->ioctl; soc_pcm_ops.copy = platform->pcm_ops->copy; soc_pcm_ops.silence = platform->pcm_ops->silence;
On Wed, 2010-03-03 at 15:08 +0200, Peter Ujfalusi wrote:
Create a soc level wrapper for pcm_pointer callback. This will facilitate the soc level handling of different HW buffers in the audio path.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
The delay callback can be used by the core to query the delay on the dai caused by FIFO or delay in the platform side. In case if both CPU and CODEC dai has FIFO the delay reported by each will be added to form the full delay on the chain. If none of the dai has FIFO, than the delay will be kept as zero.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- include/sound/soc-dai.h | 6 ++++++ include/sound/soc.h | 7 +++++++ sound/soc/soc-core.c | 18 ++++++++++++++++++ 3 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 061f16d..be9cd47 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -182,6 +182,12 @@ struct snd_soc_dai_ops { struct snd_soc_dai *); int (*trigger)(struct snd_pcm_substream *, int, struct snd_soc_dai *); + /* + * For hardware based FIFO caused delay reporting. + * Optional. + */ + snd_pcm_sframes_t (*delay)(struct snd_pcm_substream *, + struct snd_soc_dai *); };
/* diff --git a/include/sound/soc.h b/include/sound/soc.h index 5d234a8..c010824 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -469,6 +469,13 @@ struct snd_soc_platform { struct snd_pcm *); void (*pcm_free)(struct snd_pcm *);
+ /* + * For platform caused delay reporting. + * Optional. + */ + snd_pcm_sframes_t (*delay)(struct snd_pcm_substream *, + struct snd_soc_dai *); + /* platform stream ops */ struct snd_pcm_ops *pcm_ops; }; diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index de5223d..e9c2dc1 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -799,6 +799,8 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
/* * soc level wrapper for pointer callback + * If cpu_dai, codec_dai, platform driver has the delay callback, than + * the runtime->delay will be updated accordingly. */ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream) { @@ -806,11 +808,27 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream) struct snd_soc_device *socdev = rtd->socdev; struct snd_soc_card *card = socdev->card; struct snd_soc_platform *platform = card->platform; + struct snd_soc_dai_link *machine = rtd->dai; + struct snd_soc_dai *cpu_dai = machine->cpu_dai; + struct snd_soc_dai *codec_dai = machine->codec_dai; + struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t offset = 0; + snd_pcm_sframes_t delay = 0;
if (platform->pcm_ops->pointer) offset = platform->pcm_ops->pointer(substream);
+ if (cpu_dai->ops->delay) + delay += cpu_dai->ops->delay(substream, cpu_dai); + + if (codec_dai->ops->delay) + delay += codec_dai->ops->delay(substream, codec_dai); + + if (platform->delay) + delay += platform->delay(substream, codec_dai); + + runtime->delay = delay; + return offset; }
On Wed, 2010-03-03 at 15:08 +0200, Peter Ujfalusi wrote:
The delay callback can be used by the core to query the delay on the dai caused by FIFO or delay in the platform side. In case if both CPU and CODEC dai has FIFO the delay reported by each will be added to form the full delay on the chain. If none of the dai has FIFO, than the delay will be kept as zero.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
New functions for querying the FIFO caused delay on both TX and RX path. On TX path the return value shows the number of used locations in the FIFO. On RX papth it returns the number of locations to be filled to reach the threshold value (DMA will be triggered to read the data out from the FIFO).
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- arch/arm/plat-omap/include/plat/mcbsp.h | 6 +++ arch/arm/plat-omap/mcbsp.c | 55 +++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h index 3974835..1bd7021 100644 --- a/arch/arm/plat-omap/include/plat/mcbsp.h +++ b/arch/arm/plat-omap/include/plat/mcbsp.h @@ -149,6 +149,8 @@ #define OMAP_MCBSP_REG_WAKEUPEN 0xA8 #define OMAP_MCBSP_REG_XCCR 0xAC #define OMAP_MCBSP_REG_RCCR 0xB0 +#define OMAP_MCBSP_REG_XBUFFSTAT 0xB4 +#define OMAP_MCBSP_REG_RBUFFSTAT 0xB8 #define OMAP_MCBSP_REG_SSELCR 0xBC
#define OMAP_ST_REG_REV 0x00 @@ -471,6 +473,8 @@ 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_tx_delay(unsigned int id); +u16 omap_mcbsp_get_rx_delay(unsigned int id); int omap_mcbsp_get_dma_op_mode(unsigned int id); #else static inline void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold) @@ -479,6 +483,8 @@ 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_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; } #endif int omap_mcbsp_request(unsigned int id); diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c index e47686e..5e6d309 100644 --- a/arch/arm/plat-omap/mcbsp.c +++ b/arch/arm/plat-omap/mcbsp.c @@ -561,6 +561,61 @@ u16 omap_mcbsp_get_max_rx_threshold(unsigned int id) } EXPORT_SYMBOL(omap_mcbsp_get_max_rx_threshold);
+#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 + */ +u16 omap_mcbsp_get_tx_delay(unsigned int id) +{ + struct omap_mcbsp *mcbsp; + u16 buffstat; + + 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); + + /* Returns the number of free locations in the buffer */ + 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; +} +EXPORT_SYMBOL(omap_mcbsp_get_tx_delay); + +/* + * omap_mcbsp_get_rx_delay returns the number of free slots in the McBSP FIFO + * to reach the threshold value (when the DMA will be triggered to read it) + */ +u16 omap_mcbsp_get_rx_delay(unsigned int id) +{ + struct omap_mcbsp *mcbsp; + u16 buffstat, threshold; + + 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); + + /* Returns the number of used locations in the buffer */ + buffstat = MCBSP_READ(mcbsp, RBUFFSTAT); + /* RX threshold */ + threshold = MCBSP_READ(mcbsp, THRSH1); + + /* Return the number of location till we reach the threshold limit */ + if (threshold <= buffstat) + return 0; + else + return threshold - buffstat; +} +EXPORT_SYMBOL(omap_mcbsp_get_rx_delay); + /* * omap_mcbsp_get_dma_op_mode just return the current configured * operating mode for the mcbsp channel
On Wed, 3 Mar 2010 15:08:08 +0200 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
New functions for querying the FIFO caused delay on both TX and RX path. On TX path the return value shows the number of used locations in the FIFO. On RX papth it returns the number of locations to be filled to reach the threshold value (DMA will be triggered to read the data out from the FIFO).
Acked-by: Jarkko Nikula jhnikula@gmail.com
Use the new delay calback function to report the delay through ALSA for application caused by the internal FIFO.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/omap/omap-mcbsp.c | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index e814a95..2952fb0 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -256,6 +256,31 @@ static int omap_mcbsp_dai_trigger(struct snd_pcm_substream *substream, int cmd, return err; }
+static snd_pcm_sframes_t omap_mcbsp_dai_delay( + struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + 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); + u16 fifo_use; + snd_pcm_sframes_t delay; + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + fifo_use = omap_mcbsp_get_tx_delay(mcbsp_data->bus_id); + else + fifo_use = omap_mcbsp_get_rx_delay(mcbsp_data->bus_id); + + /* + * Divide the used locations with the channel count to get the + * FIFO usage in samples (don't care about partial samples in the + * buffer). + */ + delay = fifo_use / substream->runtime->channels; + + return delay; +} + static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) @@ -607,6 +632,7 @@ static struct snd_soc_dai_ops omap_mcbsp_dai_ops = { .startup = omap_mcbsp_dai_startup, .shutdown = omap_mcbsp_dai_shutdown, .trigger = omap_mcbsp_dai_trigger, + .delay = omap_mcbsp_dai_delay, .hw_params = omap_mcbsp_dai_hw_params, .set_fmt = omap_mcbsp_dai_set_dai_fmt, .set_clkdiv = omap_mcbsp_dai_set_clkdiv,
On Wed, 2010-03-03 at 15:08 +0200, Peter Ujfalusi wrote:
Use the new delay calback function to report the delay through ALSA for application caused by the internal FIFO.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
On Wed, 03 Mar 2010 13:28:24 +0000 Liam Girdwood lrg@slimlogic.co.uk wrote:
On Wed, 2010-03-03 at 15:08 +0200, Peter Ujfalusi wrote:
Use the new delay calback function to report the delay through ALSA for application caused by the internal FIFO.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
Acked-by: Jarkko Nikula jhnikula@gmail.com
On Wed, Mar 03, 2010 at 03:08:03PM +0200, Peter Ujfalusi wrote:
Peter Ujfalusi (5): ASoC: core: fix tailing whitespace in soc_pcm_apply_symmetry ASoC: core: soc level wrapper for pcm_pointer callback ASoC: core: Add delay operation to snd_soc_dai_ops
I'll apply these just now, thanks!
OMAP3: McBSP: Add interface for FIFO caused delay query ASoC: OMAP3: Report delay caused by the internal FIFO
These also look OK to me but there's the arch/arm dependency.
* Mark Brown broonie@opensource.wolfsonmicro.com [100303 05:34]:
On Wed, Mar 03, 2010 at 03:08:03PM +0200, Peter Ujfalusi wrote:
Peter Ujfalusi (5): ASoC: core: fix tailing whitespace in soc_pcm_apply_symmetry ASoC: core: soc level wrapper for pcm_pointer callback ASoC: core: Add delay operation to snd_soc_dai_ops
I'll apply these just now, thanks!
OMAP3: McBSP: Add interface for FIFO caused delay query ASoC: OMAP3: Report delay caused by the internal FIFO
These also look OK to me but there's the arch/arm dependency.
I think the only one is the McBSP patch above.
Care to take that too to keep things working?
Acked-by: Tony Lindgren tony@atomide.com
On 11 Mar 2010, at 22:26, Tony Lindgren tony@atomide.com wrote:
- Mark Brown broonie@opensource.wolfsonmicro.com [100303 05:34]:
On Wed, Mar 03, 2010 at 03:08:03PM +0200, Peter Ujfalusi wrote:
Peter Ujfalusi (5): ASoC: core: fix tailing whitespace in soc_pcm_apply_symmetry ASoC: core: soc level wrapper for pcm_pointer callback ASoC: core: Add delay operation to snd_soc_dai_ops
I'll apply these just now, thanks!
OMAP3: McBSP: Add interface for FIFO caused delay query ASoC: OMAP3: Report delay caused by the internal FIFO
These also look OK to me but there's the arch/arm dependency.
I think the only one is the McBSP patch above.
Care to take that too to keep things working?
Acked-by: Tony Lindgren tony@atomide.com
I can do, though there was some debate about how useful the information the hardware returns actually is - could folks confirm what the consensus there was, please?
On Friday 12 March 2010 00:43:41 ext Mark Brown wrote:
I can do, though there was some debate about how useful the information the hardware returns actually is - could folks confirm what the consensus there was, please?
Yeah, that thread went silent without final go or no go...
I do agree with Eero, that the BUFFSTAT register is not the most reliable source of information, but I still believe that the way we are going to use it will give pretty close to the reality estimation on the statuses. Eero also mentioned timestamp based estimation, which I think can also be used in this way: In element mode, we just return the delay as the whole size of the buffer (which is not accurate when we initially fill the buffer, and when we drain it at the end, but it is not a real problem). In threshold mode, than we can take timestamp at DMA completion interrupts, and calculate the delay based on the FIFO size (FIFO size - samples played out since the last timstamp). This can also give false numbers at the start of the playback (for few periods, until the buffer utilization settles).
So the timestamp based solution looks good, but it is only usable when the codec is running synchronously. It will definitely falls apart as soon as we use a codec like tlv320dac33 in burst mode (FIFO on the codec as well). We could have ~100 ms between DMA interrupts, so the countdown for McBSP FIFO usage is already broken. Furthermore since there is no way at the moment to actually synchronize the omap-mcbsp, omap-pcm and tlv320dac33 (in DMA, McBSP threshold, McBSP mode and DAC33 mode sense), there could be cases, that this big silence on the bus (when DAC33 is playing from it's buffer) is not happening exactly after the DMA filled up the McBSP FIFO, but a bit later (could be just one sample away from the next McBSP threshold event), which means that the McBSP FIFO is nowhere near to be full. In this case the BUFFSTAT register would give good estimation, than any timestamp based solution for McBSP FIFO.
All in all, I think the usage of the BUFFSTAT register is a good compromise for all cases, and it is needed for the cases, where the codec also have buffer of it's own. Never the less, we can also add the timestamp based estimation later, but we need to make sure, that those can be picked somehow for the given setup.
On Fri, 12 Mar 2010 08:18:30 +0200 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
On Friday 12 March 2010 00:43:41 ext Mark Brown wrote:
I can do, though there was some debate about how useful the information the hardware returns actually is - could folks confirm what the consensus there was, please?
Yeah, that thread went silent without final go or no go...
...
All in all, I think the usage of the BUFFSTAT register is a good compromise for all cases, and it is needed for the cases, where the codec also have buffer of it's own.
And one point is that the BUFFSTAT use doesn't make things worse than currently. Now the application doesn't have any information about the HW buffer caused audio latencies so reporting them makes the things better than currently.
On Fri, Mar 12, 2010 at 10:30:50AM +0200, Jarkko Nikula wrote:
Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
All in all, I think the usage of the BUFFSTAT register is a good compromise for all cases, and it is needed for the cases, where the codec also have buffer of it's own.
And one point is that the BUFFSTAT use doesn't make things worse than currently. Now the application doesn't have any information about the HW buffer caused audio latencies so reporting them makes the things better than currently.
OK, that's what I thought - just wanted to confirm. I've applied both patches now.
participants (5)
-
Jarkko Nikula -
Liam Girdwood -
Mark Brown -
Peter Ujfalusi -
Tony Lindgren