[Sound-open-firmware] [PATCH v2 0/3] configure dma msize according to burst elems
For designware DMA controller, we should set src_msize and dest_msize according to number of data items(burst_elems), this series implement the setting logic, and configure burst_elems to slot number for dai as peripheral device.
Keyon Jie (3): dw-dma: add burst_elems setting to struct dma_sg_element dai: set dai dma burst_elems according to slot number dw-dma: set msize according to burst_elems setting
src/audio/dai.c | 5 +++++ src/drivers/dw-dma.c | 25 ++++++++++++++++++++++--- src/include/reef/dma.h | 1 + 3 files changed, 28 insertions(+), 3 deletions(-)
We need configure different burst_elems for different dma copy, so here introduce it to dma_sg_element struct.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com --- src/include/reef/dma.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/include/reef/dma.h b/src/include/reef/dma.h index 697e2c6..e33adaa 100644 --- a/src/include/reef/dma.h +++ b/src/include/reef/dma.h @@ -70,6 +70,7 @@ struct dma_sg_elem { struct dma_sg_config { uint32_t src_width; uint32_t dest_width; + uint32_t burst_elems; uint32_t direction; uint32_t src_dev; uint32_t dest_dev;
We should set burst_elems for dma peripheral dev copy, for dai/ssp, it should be set to valid slot number, otherwise, the dma may copy in wrong burst size and data will run with wrong speed.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com --- src/audio/dai.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/audio/dai.c b/src/audio/dai.c index c50a274..9058f5c 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -601,6 +601,11 @@ static int dai_position(struct comp_dev *dev, struct sof_ipc_stream_posn *posn)
static int dai_config(struct comp_dev *dev, struct sof_ipc_dai_config *config) { + struct dai_data *dd = comp_get_drvdata(dev); + + /* set dma burst elems to slot number */ + dd->config.burst_elems = config->num_slots; + /* calc frame bytes */ switch (config->sample_valid_bits) { case 16:
On 12/18/2017 11:06 PM, Keyon Jie wrote:
I don't think the commit message reflects the code, it's really the total number of active slots and not the slot number(s) that we care about, right?
This is a good topic. At the moment, we are using topology file to configure dai, e.g. DAI_TDM(2, 20, 3, 3), which set num_slots(corresponding to 'slot number(s)' you mentioned?) to 2 and tx slot mask to 0x3(slot0and slot1 enabled, corresponding to 'the total number' in your context?), so they are same for us actually at this case.
But imagine that if we set tx slot mask to, e.g. 0x1, should we set burst_elems to 1 or 2? I tend to set 1 at this case, but not 100% sure about it yet, Maybe we need experiment and fix bugs if exist.
So back to your question, I think the answer is yes, using active slots number may be more accurate, though we need experiment on that later.
Thanks, ~Keyon
For memory to memory copy(burst_elems initialized to be 0), we set msize to default value 3, that is 2 ^ 3 = 8 items for each burst transaction.
For DMA copying which related to peripheral device(source or destination), we will use the configured burst_elems, which usually means the number of data items, e.g. slot number for SSP dai fifos.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com --- src/drivers/dw-dma.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/src/drivers/dw-dma.c b/src/drivers/dw-dma.c index a28281a..fdd49c0 100644 --- a/src/drivers/dw-dma.c +++ b/src/drivers/dw-dma.c @@ -426,6 +426,13 @@ static int dw_dma_status(struct dma *dma, int channel, return 0; }
+/* + * use array to get burst_elems for specific slot number setting. + * the relation between msize and burst_elems should be + * 2 ^ burst_elems = burst_elems + */ +static const uint32_t burst_elems[] = {1, 2, 4, 8}; + /* set the DMA channel configuration, source/target address, buffer sizes */ static int dw_dma_set_config(struct dma *dma, int channel, struct dma_sg_config *config) @@ -438,7 +445,8 @@ static int dw_dma_set_config(struct dma *dma, int channel, struct dw_lli2 *lli_desc_tail; uint32_t desc_count = 0; uint32_t flags; - int ret = 0; + uint32_t msize = 3;/* default msize */ + int i, ret = 0;
spin_lock_irq(&dma->lock, flags);
@@ -482,6 +490,17 @@ static int dw_dma_set_config(struct dma *dma, int channel, lli_desc = lli_desc_head = p->chan[channel].lli; lli_desc_tail = p->chan[channel].lli + p->chan[channel].desc_count - 1;
+ /* configure msize if burst_elems is set */ + if (config->burst_elems) { + /* burst_elems set, configure msize */ + for (i = 0; i < ARRAY_SIZE(burst_elems); i++) { + if (burst_elems[i] == config->burst_elems) { + msize = i; + break; + } + } + } + /* fill in lli for the elem in the list */ list_for_item(plist, &config->elem_list) {
@@ -518,8 +537,8 @@ static int dw_dma_set_config(struct dma *dma, int channel, goto out; }
- lli_desc->ctrl_lo |= DW_CTLL_SRC_MSIZE(3); /* config the src msize length 2^2 */ - lli_desc->ctrl_lo |= DW_CTLL_DST_MSIZE(3); /* config the dest msize length 2^2 */ + lli_desc->ctrl_lo |= DW_CTLL_SRC_MSIZE(msize); + lli_desc->ctrl_lo |= DW_CTLL_DST_MSIZE(msize); lli_desc->ctrl_lo |= DW_CTLL_INT_EN; /* enable interrupt */
/* config the SINC and DINC field of CTL_LOn, SRC/DST_PER filed of CFGn */
participants (4)
-
Jie, Yang
-
Keyon Jie
-
Liam Girdwood
-
Pierre-Louis Bossart