From fee9f9656d1a78c3b96e30f663038b91fcf4a467 Mon Sep 17 00:00:00 2001 From: Marcin Maka Date: Thu, 6 Sep 2018 21:50:54 +0200 Subject: [PATCH] dma: hda: playback preload phase synced with logical buffer state Removed dma/dsp race condition that happened within preload phase. Min buffer size set to entire buffer to avoid dma and dsp pointers misalignmen in case the period size is not multiple of the hda dma burst size. Signed-off-by: Marcin Maka --- src/audio/dai.c | 28 ++++++-- src/audio/host.c | 2 +- src/drivers/intel/cavs/hda-dma.c | 119 ++++++++++++++++++++++--------- src/include/sof/dma.h | 2 +- 4 files changed, 112 insertions(+), 39 deletions(-) diff --git a/src/audio/dai.c b/src/audio/dai.c index 1330880dc..4563017dc 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -48,6 +48,9 @@ #define DAI_PLAYBACK_STREAM 0 #define DAI_CAPTURE_STREAM 1 +#define DAI_PTR_INIT_DAI 1 /* buffer ptr initialized by dai */ +#define DAI_PTR_INIT_HOST 2 /* buffer ptr initialized by host */ + /* tracing */ #define trace_dai(__e) trace_event(TRACE_CLASS_DAI, __e) #define trace_dai_error(__e) trace_error(TRACE_CLASS_DAI, __e) @@ -523,6 +526,8 @@ static void dai_pointer_init(struct comp_dev *dev) struct comp_buffer *dma_buffer; struct dai_data *dd = comp_get_drvdata(dev); + dd->pointer_init = DAI_PTR_INIT_DAI; + /* not required for capture streams */ if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { dma_buffer = list_first_item(&dev->bsource_list, @@ -532,6 +537,7 @@ static void dai_pointer_init(struct comp_dev *dev) case SOF_COMP_HOST: case SOF_COMP_SG_HOST: /* buffer is preloaded and advanced by host DMA engine */ + dd->pointer_init = DAI_PTR_INIT_HOST; break; default: /* advance source pipeline w_ptr by one period @@ -540,8 +546,6 @@ static void dai_pointer_init(struct comp_dev *dev) break; } } - - dd->pointer_init = 1; } /* used to pass standard and bespoke command (with data) to component */ @@ -563,8 +567,12 @@ static int dai_comp_trigger(struct comp_dev *dev, int cmd) trace_dai("tsa"); if (!dd->pointer_init) dai_pointer_init(dev); - /* only start the DAI if we are not XRUN handling */ - if (dd->xrun == 0) { + /* only start the DAI if we are not XRUN handling + * and the ptr is not initialized by the host as in this + * case start is deferred to the first copy call as the buffer + * is populated by the host only then + */ + if (dd->xrun == 0 && dd->pointer_init != DAI_PTR_INIT_HOST) { /* start the DAI */ ret = dma_start(dd->dma, dd->chan); if (ret < 0) @@ -632,6 +640,18 @@ static int dai_comp_trigger(struct comp_dev *dev, int cmd) /* copy and process stream data from source to sink buffers */ static int dai_copy(struct comp_dev *dev) { + struct dai_data *dd = comp_get_drvdata(dev); + int ret; + + if (dd->pointer_init == DAI_PTR_INIT_HOST) { + /* start the DAI */ + ret = dma_start(dd->dma, dd->chan); + if (ret < 0) + return ret; + dai_trigger(dd->dai, COMP_TRIGGER_START, dev->params.direction); + dd->pointer_init = DAI_PTR_INIT_DAI; /* next copy just quits */ + platform_dai_wallclock(dev, &dd->wallclock); + } return 0; } diff --git a/src/audio/host.c b/src/audio/host.c index a1d31af0b..4c555770a 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -741,7 +741,7 @@ static int host_copy_int(struct comp_dev *dev, bool preload_run) #if defined CONFIG_DMA_GW /* tell gateway to copy another period */ ret = dma_copy(hd->dma, hd->chan, hd->period_bytes, - preload_run ? DMA_COPY_NO_COMMIT : 0); + preload_run ? DMA_COPY_PRELOAD : 0); if (ret < 0) goto out; diff --git a/src/drivers/intel/cavs/hda-dma.c b/src/drivers/intel/cavs/hda-dma.c index 5293ba88b..6545bc701 100644 --- a/src/drivers/intel/cavs/hda-dma.c +++ b/src/drivers/intel/cavs/hda-dma.c @@ -86,11 +86,15 @@ #define HDA_LINK_1MS_US 1000 +#define HDA_STATE_PRELOAD BIT(0) +#define HDA_STATE_BF_WAIT BIT(1) + struct hda_chan_data { struct dma *dma; uint32_t index; uint32_t stream_id; - uint32_t status; + uint32_t status; /* common status */ + uint32_t state; /* hda specific additional state */ uint32_t desc_count; uint32_t desc_avail; uint32_t direction; @@ -186,42 +190,77 @@ static inline uint32_t hda_dma_get_data_size(struct dma *dma, uint32_t chan) return ds; } -static int hda_dma_copy_ch(struct dma *dma, struct hda_chan_data *chan, - int bytes, uint32_t flags) +static int hda_dma_preload(struct dma *dma, struct hda_chan_data *chan) { - struct dma_sg_elem next; + struct dma_sg_elem next = { + .src = DMA_RELOAD_LLI, + .dest = DMA_RELOAD_LLI, + .size = DMA_RELOAD_LLI + }; + int i; + int period_cnt; + + /* waiting for buffer full after start + * first try is unblocking, then blocking + */ + while (!(host_dma_reg_read(dma, chan->index, DGCS) & DGCS_BF) && + (chan->state & HDA_STATE_BF_WAIT)) + ; + + if (host_dma_reg_read(dma, chan->index, DGCS) & DGCS_BF) { + chan->state &= ~(HDA_STATE_PRELOAD | HDA_STATE_BF_WAIT); + if (chan->cb) { + /* loop over each period */ + period_cnt = chan->buffer_bytes / + chan->period_bytes; + for (i = 0; i < period_cnt; i++) + chan->cb(chan->cb_data, + DMA_IRQ_TYPE_LLIST, &next); + /* do not need to test out next in this path */ + } + } else { + /* next call in pre-load state will be blocking */ + chan->state |= HDA_STATE_BF_WAIT; + } + + return 0; +} + +static int hda_dma_copy_ch(struct dma *dma, struct hda_chan_data *chan, + int bytes) +{ + struct dma_sg_elem next = { + .src = DMA_RELOAD_LLI, + .dest = DMA_RELOAD_LLI, + .size = DMA_RELOAD_LLI + }; + uint32_t flags; uint32_t dgcs = 0; tracev_host("GwU"); - if (flags & DMA_COPY_NO_COMMIT) { - /* delay to fill the buffer, only for the first time - * for _input_ dma. - */ - idelay(PLATFORM_DEFAULT_DELAY); - } else { - /* clear link xruns */ - dgcs = host_dma_reg_read(dma, chan->index, DGCS); - if (dgcs & DGCS_BOR) - hda_update_bits(dma, chan->index, - DGCS, DGCS_BOR, DGCS_BOR); - /* make sure that previous transfer is complete */ - if (chan->direction == DMA_DIR_MEM_TO_DEV) { - while (hda_dma_get_free_size(dma, chan->index) < - bytes) - idelay(PLATFORM_DEFAULT_DELAY); - } + /* clear link xruns */ + dgcs = host_dma_reg_read(dma, chan->index, DGCS); + if (dgcs & DGCS_BOR) + hda_update_bits(dma, chan->index, + DGCS, DGCS_BOR, DGCS_BOR); - /* - * set BFPI to let host gateway knows we have read size, - * which will trigger next copy start. - */ - if (chan->direction == DMA_DIR_MEM_TO_DEV) - hda_dma_inc_link_fp(dma, chan->index, bytes); - else - hda_dma_inc_fp(dma, chan->index, bytes); + /* make sure that previous transfer is complete */ + if (chan->direction == DMA_DIR_MEM_TO_DEV) { + while (hda_dma_get_free_size(dma, chan->index) < + bytes) + idelay(PLATFORM_DEFAULT_DELAY); } + /* + * set BFPI to let host gateway knows we have read size, + * which will trigger next copy start. + */ + if (chan->direction == DMA_DIR_MEM_TO_DEV) + hda_dma_inc_link_fp(dma, chan->index, bytes); + else + hda_dma_inc_fp(dma, chan->index, bytes); + spin_lock_irq(&dma->lock, flags); if (chan->cb) { next.src = DMA_RELOAD_LLI; @@ -245,8 +284,8 @@ static uint64_t hda_dma_work(void *data, uint64_t delay) { struct hda_chan_data *chan = (struct hda_chan_data *)data; - hda_dma_copy_ch(chan->dma, chan, chan->period_bytes, 0); - /* next time to re-arm (TODO: should sub elapsed time?) */ + hda_dma_copy_ch(chan->dma, chan, chan->period_bytes); + /* next time to re-arm */ return HDA_LINK_1MS_US; } @@ -254,8 +293,15 @@ static uint64_t hda_dma_work(void *data, uint64_t delay) static int hda_dma_copy(struct dma *dma, int channel, int bytes, uint32_t flags) { struct dma_pdata *p = dma_get_drvdata(dma); + struct hda_chan_data *chan = p->chan + channel; - return hda_dma_copy_ch(dma, &p->chan[channel], bytes, flags); + if (flags & DMA_COPY_PRELOAD) + chan->state |= HDA_STATE_PRELOAD; + + if (chan->state & HDA_STATE_PRELOAD) + return hda_dma_preload(dma, chan); + else + return hda_dma_copy_ch(dma, chan, bytes); } /* acquire the specific DMA channel */ @@ -292,6 +338,7 @@ static void hda_dma_channel_put_unlocked(struct dma *dma, int channel) /* set new state */ p->chan[channel].status = COMP_STATE_INIT; + p->chan[channel].state = 0; p->chan[channel].period_bytes = 0; p->chan[channel].buffer_bytes = 0; p->chan[channel].cb = NULL; @@ -495,6 +542,12 @@ static int hda_dma_set_config(struct dma *dma, int channel, if (buffer_addr == 0) buffer_addr = addr; } + /* buffer size must be multiple of hda dma burst size */ + if (buffer_bytes % PLATFORM_HDA_BUFFER_ALIGNMENT) { + ret = -EINVAL; + goto out; + } + p->chan[channel].period_bytes = period_bytes; p->chan[channel].buffer_bytes = buffer_bytes; @@ -511,7 +564,7 @@ static int hda_dma_set_config(struct dma *dma, int channel, if (config->direction == DMA_DIR_LMEM_TO_HMEM || config->direction == DMA_DIR_HMEM_TO_LMEM) host_dma_reg_write(dma, channel, DGMBS, - ALIGN_UP(period_bytes, + ALIGN_UP(buffer_bytes, PLATFORM_HDA_BUFFER_ALIGNMENT)); /* firmware control buffer */ diff --git a/src/include/sof/dma.h b/src/include/sof/dma.h index e9536c64f..a6fcc9a14 100644 --- a/src/include/sof/dma.h +++ b/src/include/sof/dma.h @@ -81,7 +81,7 @@ #define DMA_IRQ_TYPE_LLIST BIT(1) /* DMA copy flags */ -#define DMA_COPY_NO_COMMIT BIT(0) +#define DMA_COPY_PRELOAD BIT(0) /* We will use this macro in cb handler to inform dma that * we need to stop the reload for special purpose