On Tue, 2018-03-13 at 10:53 +0000, Liam Girdwood wrote:
On Tue, 2018-03-13 at 18:16 +0800, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@linux.intel.com
The host DMA has the different config method:
- Get DMA channel based on stream tag.
- Prepare DMA elem list based on local DMA trace buffer which make
sure that one trace record data can be transferred at least. 3. Need start DMA on firmware side before trigger start on kernel side. 4. Because no DMA callback in host DMA, add position update calling at the end of trace_work().
Signed-off-by: Yan Wang yan.wang@linux.intel.com
src/lib/dma-trace.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+)
diff --git a/src/lib/dma-trace.c b/src/lib/dma-trace.c index b08e7fb..51e413c 100644 --- a/src/lib/dma-trace.c +++ b/src/lib/dma-trace.c @@ -120,6 +120,10 @@ out: spin_unlock_irq(&d->lock, flags); +#if defined CONFIG_DMA_GW
- ipc_dma_trace_send_position();
+#endif
Why do we only send trace position to host with GW DMA ? If there is a valid reason then we need to comment that reason next to the code.
Because GW DMA hasn't dma completion callback for doing this. I will add comments for this.
/* reschedule the trace copying work */ return DMA_TRACE_PERIOD; } @@ -163,7 +167,11 @@ int dma_trace_init_complete(struct dma_trace_data *d) trace_buffer("dtn"); /* init DMA copy context */ +#if defined CONFIG_DMA_GW
- ret = dma_copy_new(&d->dc, DMA_HOST_IN_DMAC);
+#else ret = dma_copy_new(&d->dc, PLATFORM_TRACE_DMAC); +#endif
Why not define PLATFORM_TRACE_DMAC as DMA_HOST_IN_DMAC in platform.h for APL and CNL ?
sure.
if (ret < 0) { trace_buffer_error("edm"); rfree(buffer->addr); @@ -195,8 +203,73 @@ int dma_trace_host_buffer(struct dma_trace_data *d, struct dma_sg_elem *elem, return 0; } +#if defined CONFIG_DMA_GW
+static int dma_trace_start(struct dma_trace_data *d) +{
- struct dma_sg_config config;
- struct dma_sg_elem *e;
- uint32_t elem_size, elem_addr, elem_num;
- int err = 0;
- int i;
- err = dma_set_channel(&d->dc, d->stream_tag);
- if (err < 0)
return err;- /* size of every trace record */
- elem_size = sizeof(uint64_t) * 2;
- /* Initialize address of local elem */
- elem_addr = (uint32_t)d->dmatb.addr;
- /* the number of elem list */
- elem_num = DMA_TRACE_LOCAL_SIZE / elem_size;
- config.direction = DMA_DIR_LMEM_TO_HMEM;
- config.src_width = sizeof(uint32_t);
- config.dest_width = sizeof(uint32_t);
- config.cyclic = 0;
- list_init(&config.elem_list);
- /* generate local elem list for local trace buffer */
- e = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM, sizeof(*e) *
elem_num);
- if (!e)
return -ENOMEM;- for (i = 0; i < elem_num; i++) {
e[i].dest = 0;e[i].src = elem_addr;e[i].size = elem_size; /* the minimum size of DMAcopy */
list_item_append(&e[i].list, &config.elem_list);elem_addr += elem_size;- }
- err = dma_set_config(d->dc.dmac, d->dc.chan, &config);
- if (err < 0) {
rfree(e);return err;- }
- err = dma_start(d->dc.dmac, d->dc.chan);
- rfree(e);
- return err;
+}
+#endif
int dma_trace_enable(struct dma_trace_data *d) { +#if defined CONFIG_DMA_GW
- int err;
- err = dma_trace_start(d);
- if (err < 0)
return err;+#endif
Can you comment why we start trace for GW DMA here.
Yes. Thanks.
Yan Wang
/* validate DMA context */ if (d->dc.dmac == NULL || d->dc.chan < 0) { trace_error_atomic(TRACE_CLASS_BUFFER, "eem");
Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware