[Sound-open-firmware] [PATCH 0/5][RFC] APL/CNL: Enable DMA trace on APL/CNL.
From: Yan Wang yan.wang@linux.intel.com
This patch set is only for initial review and trying. It can work fine on my CNL platform. I also tested on BYT MB, It doesn't cause regression.
Yan Wang (5): SOF: APL/CNL: Add stream_tag variable for saving parameter from kernel driver. Implement host DMA copy in existed DMA copy API. SOF: APL/CNL: Save stream tag and host buffer size from kernel driver. SOF: APL/CNL: Implement host DMA setting and position update on APL/CNL. SOF: APL/CNL: Initialize DMA trace data structure on APL/CNL.
src/include/reef/dma-trace.h | 1 + src/include/reef/dma.h | 4 +++ src/include/uapi/ipc.h | 1 + src/ipc/dma-copy.c | 40 ++++++++++++++++++++- src/ipc/intel-ipc.c | 8 ++++- src/lib/dma-trace.c | 73 ++++++++++++++++++++++++++++++++++++++ src/platform/apollolake/platform.c | 3 ++ src/platform/cannonlake/platform.c | 3 ++ 8 files changed, 131 insertions(+), 2 deletions(-)
From: Yan Wang yan.wang@linux.intel.com
Add stream_tag variable for IPC message and DMA trace data structure.
Signed-off-by: Yan Wang yan.wang@linux.intel.com --- src/include/reef/dma-trace.h | 1 + src/include/uapi/ipc.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/src/include/reef/dma-trace.h b/src/include/reef/dma-trace.h index dcbd491..f17ffe5 100644 --- a/src/include/reef/dma-trace.h +++ b/src/include/reef/dma-trace.h @@ -63,6 +63,7 @@ struct dma_trace_data { struct work dmat_work; uint32_t enabled; uint32_t copy_in_progress; + uint32_t stream_tag; spinlock_t lock; };
diff --git a/src/include/uapi/ipc.h b/src/include/uapi/ipc.h index 0c34013..e44a13f 100644 --- a/src/include/uapi/ipc.h +++ b/src/include/uapi/ipc.h @@ -839,6 +839,7 @@ struct sof_ipc_window { struct sof_ipc_dma_trace_params { struct sof_ipc_hdr hdr; struct sof_ipc_host_buffer buffer; + uint32_t stream_tag; } __attribute__((packed));
/* DMA for Trace params info - SOF_IPC_DEBUG_DMA_PARAMS */
From: Yan Wang yan.wang@linux.intel.com
Get DMA channel based on stream tag when use host DMA. Need also call different API in dma_copy_to_host_nowait().
Signed-off-by: Yan Wang yan.wang@linux.intel.com --- src/include/reef/dma.h | 4 ++++ src/ipc/dma-copy.c | 40 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/src/include/reef/dma.h b/src/include/reef/dma.h index 247bcbf..0f7dbfc 100644 --- a/src/include/reef/dma.h +++ b/src/include/reef/dma.h @@ -278,4 +278,8 @@ int dma_copy_to_host(struct dma_copy *dc, struct dma_sg_config *host_sg, int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg, int32_t host_offset, void *local_ptr, int32_t size);
+#if defined CONFIG_DMA_GW +int dma_set_channel(struct dma_copy *dc, uint32_t stream_tag); +#endif + #endif diff --git a/src/ipc/dma-copy.c b/src/ipc/dma-copy.c index 06f05d5..f11acf1 100644 --- a/src/ipc/dma-copy.c +++ b/src/ipc/dma-copy.c @@ -70,6 +70,8 @@ static struct dma_sg_elem *sg_get_elem_at(struct dma_sg_config *host_sg, return NULL; }
+#if !defined CONFIG_DMA_GW + static void dma_complete(void *data, uint32_t type, struct dma_sg_elem *next) { completion_t *comp = (completion_t *)data; @@ -84,6 +86,8 @@ static void dma_complete(void *data, uint32_t type, struct dma_sg_elem *next) next->size = DMA_RELOAD_END; }
+#endif + /* Copy DSP memory to host memory. * copies DSP memory to host in PAGE_SIZE or smaller blocks and waits/sleeps * between blocks. Can't be used in IRQ context. @@ -172,8 +176,22 @@ int dma_copy_to_host(struct dma_copy *dc, struct dma_sg_config *host_sg, * Copies DSP memory to host in a single PAGE_SIZE or smaller block. Does not * waits/sleeps and can be used in IRQ context. */ +#if defined CONFIG_DMA_GW + int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg, - int32_t host_offset, void *local_ptr, int32_t size) + int32_t host_offset, void *local_ptr, int32_t size) +{ + /* tell gateway to copy */ + dma_copy(dc->dmac, dc->chan, size); + + /* bytes copied */ + return size; +} + +#else + +int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg, + int32_t host_offset, void *local_ptr, int32_t size) { struct dma_sg_config config; struct dma_sg_elem *host_sg_elem; @@ -219,6 +237,8 @@ int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg, return local_sg_elem.size; }
+#endif + /* Copy host memory to DSP memory. * Copies host memory to host in PAGE_SIZE or smaller blocks and waits/sleeps * between blocks. Cant be used in IRQ context. @@ -359,6 +379,7 @@ int dma_copy_new(struct dma_copy *dc, int dmac) return -ENODEV; }
+#if !defined CONFIG_DMA_GW /* get DMA channel from DMAC0 */ dc->chan = dma_channel_get(dc->dmac, 0); if (dc->chan < 0) { @@ -369,6 +390,23 @@ int dma_copy_new(struct dma_copy *dc, int dmac) dc->complete.timeout = 100; /* wait 100 usecs for DMA to finish */ dma_set_cb(dc->dmac, dc->chan, DMA_IRQ_TYPE_LLIST, dma_complete, &dc->complete); +#endif + return 0; }
+#if defined CONFIG_DMA_GW + +int dma_set_channel(struct dma_copy *dc, uint32_t stream_tag) +{ + /* get DMA channel from DMAC */ + dc->chan = dma_channel_get(dc->dmac, stream_tag - 1); + if (dc->chan < 0) { + trace_dma_error("ec1"); + return dc->chan; + } + + return 0; +} + +#endif
On Tue, 2018-03-13 at 18:16 +0800, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@linux.intel.com
Get DMA channel based on stream tag when use host DMA. Need also call different API in dma_copy_to_host_nowait().
Signed-off-by: Yan Wang yan.wang@linux.intel.com
src/include/reef/dma.h | 4 ++++ src/ipc/dma-copy.c | 40 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/src/include/reef/dma.h b/src/include/reef/dma.h index 247bcbf..0f7dbfc 100644 --- a/src/include/reef/dma.h +++ b/src/include/reef/dma.h @@ -278,4 +278,8 @@ int dma_copy_to_host(struct dma_copy *dc, struct dma_sg_config *host_sg, int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg, int32_t host_offset, void *local_ptr, int32_t size);
+#if defined CONFIG_DMA_GW +int dma_set_channel(struct dma_copy *dc, uint32_t stream_tag); +#endif
You dont need any #ifdefs around function declarations in headers.
Please rename this to dma_copy_set_stream_tag()
#endif diff --git a/src/ipc/dma-copy.c b/src/ipc/dma-copy.c index 06f05d5..f11acf1 100644 --- a/src/ipc/dma-copy.c +++ b/src/ipc/dma-copy.c @@ -70,6 +70,8 @@ static struct dma_sg_elem *sg_get_elem_at(struct dma_sg_config *host_sg, return NULL; }
+#if !defined CONFIG_DMA_GW
static void dma_complete(void *data, uint32_t type, struct dma_sg_elem *next) { completion_t *comp = (completion_t *)data; @@ -84,6 +86,8 @@ static void dma_complete(void *data, uint32_t type, struct dma_sg_elem *next) next->size = DMA_RELOAD_END; }
+#endif
/* Copy DSP memory to host memory.
- copies DSP memory to host in PAGE_SIZE or smaller blocks and waits/sleeps
- between blocks. Can't be used in IRQ context.
@@ -172,8 +176,22 @@ int dma_copy_to_host(struct dma_copy *dc, struct dma_sg_config *host_sg,
- Copies DSP memory to host in a single PAGE_SIZE or smaller block. Does not
- waits/sleeps and can be used in IRQ context.
*/ +#if defined CONFIG_DMA_GW
int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg,
- int32_t host_offset, void *local_ptr, int32_t size)
int32_t host_offset, void *local_ptr, int32_tsize) +{
- /* tell gateway to copy */
- dma_copy(dc->dmac, dc->chan, size);
Probably want to check any return value here ?
- /* bytes copied */
- return size;
+}
+#else
+int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg,
int32_t host_offset, void *local_ptr, int32_tsize) { struct dma_sg_config config; struct dma_sg_elem *host_sg_elem; @@ -219,6 +237,8 @@ int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg, return local_sg_elem.size; }
+#endif
/* Copy host memory to DSP memory.
- Copies host memory to host in PAGE_SIZE or smaller blocks and waits/sleeps
- between blocks. Cant be used in IRQ context.
@@ -359,6 +379,7 @@ int dma_copy_new(struct dma_copy *dc, int dmac) return -ENODEV; }
+#if !defined CONFIG_DMA_GW /* get DMA channel from DMAC0 */ dc->chan = dma_channel_get(dc->dmac, 0); if (dc->chan < 0) { @@ -369,6 +390,23 @@ int dma_copy_new(struct dma_copy *dc, int dmac) dc->complete.timeout = 100; /* wait 100 usecs for DMA to finish */ dma_set_cb(dc->dmac, dc->chan, DMA_IRQ_TYPE_LLIST, dma_complete, &dc->complete); +#endif
- return 0;
}
+#if defined CONFIG_DMA_GW
+int dma_set_channel(struct dma_copy *dc, uint32_t stream_tag) +{
- /* get DMA channel from DMAC */
- dc->chan = dma_channel_get(dc->dmac, stream_tag - 1);
- if (dc->chan < 0) {
trace_dma_error("ec1");return dc->chan;- }
- return 0;
return -EINVAL; as 0 is a valid channel number.
+}
+#endif
--------------------------------------------------------------------- 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.
On Tue, 2018-03-13 at 10:48 +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
Get DMA channel based on stream tag when use host DMA. Need also call different API in dma_copy_to_host_nowait().
Signed-off-by: Yan Wang yan.wang@linux.intel.com
src/include/reef/dma.h | 4 ++++ src/ipc/dma-copy.c | 40 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/src/include/reef/dma.h b/src/include/reef/dma.h index 247bcbf..0f7dbfc 100644 --- a/src/include/reef/dma.h +++ b/src/include/reef/dma.h @@ -278,4 +278,8 @@ int dma_copy_to_host(struct dma_copy *dc, struct dma_sg_config *host_sg, int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg, int32_t host_offset, void *local_ptr, int32_t size); +#if defined CONFIG_DMA_GW +int dma_set_channel(struct dma_copy *dc, uint32_t stream_tag); +#endif
You dont need any #ifdefs around function declarations in headers.
Please rename this to dma_copy_set_stream_tag()
Sure.
#endif diff --git a/src/ipc/dma-copy.c b/src/ipc/dma-copy.c index 06f05d5..f11acf1 100644 --- a/src/ipc/dma-copy.c +++ b/src/ipc/dma-copy.c @@ -70,6 +70,8 @@ static struct dma_sg_elem *sg_get_elem_at(struct dma_sg_config *host_sg, return NULL; } +#if !defined CONFIG_DMA_GW
static void dma_complete(void *data, uint32_t type, struct dma_sg_elem *next) { completion_t *comp = (completion_t *)data; @@ -84,6 +86,8 @@ static void dma_complete(void *data, uint32_t type, struct dma_sg_elem *next) next->size = DMA_RELOAD_END; } +#endif
/* Copy DSP memory to host memory. * copies DSP memory to host in PAGE_SIZE or smaller blocks and waits/sleeps * between blocks. Can't be used in IRQ context. @@ -172,8 +176,22 @@ int dma_copy_to_host(struct dma_copy *dc, struct dma_sg_config *host_sg, * Copies DSP memory to host in a single PAGE_SIZE or smaller block. Does not * waits/sleeps and can be used in IRQ context. */ +#if defined CONFIG_DMA_GW
int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg,
- int32_t host_offset, void *local_ptr, int32_t size)
int32_t host_offset, void *local_ptr,int32_t size) +{
- /* tell gateway to copy */
- dma_copy(dc->dmac, dc->chan, size);
Probably want to check any return value here ?
Sure.
- /* bytes copied */
- return size;
+}
+#else
+int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg,
int32_t host_offset, void *local_ptr,int32_t size) { struct dma_sg_config config; struct dma_sg_elem *host_sg_elem; @@ -219,6 +237,8 @@ int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg, return local_sg_elem.size; } +#endif
/* Copy host memory to DSP memory. * Copies host memory to host in PAGE_SIZE or smaller blocks and waits/sleeps * between blocks. Cant be used in IRQ context. @@ -359,6 +379,7 @@ int dma_copy_new(struct dma_copy *dc, int dmac) return -ENODEV; } +#if !defined CONFIG_DMA_GW /* get DMA channel from DMAC0 */ dc->chan = dma_channel_get(dc->dmac, 0); if (dc->chan < 0) { @@ -369,6 +390,23 @@ int dma_copy_new(struct dma_copy *dc, int dmac) dc->complete.timeout = 100; /* wait 100 usecs for DMA to finish */ dma_set_cb(dc->dmac, dc->chan, DMA_IRQ_TYPE_LLIST, dma_complete, &dc->complete); +#endif
return 0; } +#if defined CONFIG_DMA_GW
+int dma_set_channel(struct dma_copy *dc, uint32_t stream_tag) +{
- /* get DMA channel from DMAC */
- dc->chan = dma_channel_get(dc->dmac, stream_tag - 1);
- if (dc->chan < 0) {
trace_dma_error("ec1");return dc->chan;- }
- return 0;
return -EINVAL; as 0 is a valid channel number.
sure.
+}
+#endif
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
From: Yan Wang yan.wang@linux.intel.com
It is necessary to save stream tag and host buffer size for host DMA setting.
Signed-off-by: Yan Wang yan.wang@linux.intel.com --- src/ipc/intel-ipc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/ipc/intel-ipc.c b/src/ipc/intel-ipc.c index ffc6f29..8263fff 100644 --- a/src/ipc/intel-ipc.c +++ b/src/ipc/intel-ipc.c @@ -640,12 +640,12 @@ static int ipc_dma_trace_config(uint32_t header) { #ifdef CONFIG_HOST_PTABLE struct intel_ipc_data *iipc = ipc_get_drvdata(_ipc); - struct sof_ipc_dma_trace_params *params = _ipc->comp_data; struct list_item elem_list; struct dma_sg_elem *elem; struct list_item *plist; uint32_t ring_size; #endif + struct sof_ipc_dma_trace_params *params = _ipc->comp_data; struct sof_ipc_reply reply; int err;
@@ -683,6 +683,12 @@ static int ipc_dma_trace_config(uint32_t header) list_item_del(&elem->list); rfree(elem); } +#else + /* stream tag of capture stream for DMA trace */ + _ipc->dmat->stream_tag = params->stream_tag; + + /* host buffer size for DMA trace */ + _ipc->dmat->host_size = params->buffer.size; #endif trace_ipc("DAp");
From: Yan Wang yan.wang@linux.intel.com
The host DMA has the different config method: 1. Get DMA channel based on stream tag. 2. 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 + /* 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 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 DMA copy */ + + 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 + /* validate DMA context */ if (d->dc.dmac == NULL || d->dc.chan < 0) { trace_error_atomic(TRACE_CLASS_BUFFER, "eem");
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.
- /* 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 ?
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 DMA copy */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.
- /* 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.
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
From: Yan Wang yan.wang@linux.intel.com
Add dma_trace_init_complete() calling on APL/CNL platform.
Signed-off-by: Yan Wang yan.wang@linux.intel.com --- src/platform/apollolake/platform.c | 3 +++ src/platform/cannonlake/platform.c | 3 +++ 2 files changed, 6 insertions(+)
diff --git a/src/platform/apollolake/platform.c b/src/platform/apollolake/platform.c index 9574970..84f8e71 100644 --- a/src/platform/apollolake/platform.c +++ b/src/platform/apollolake/platform.c @@ -262,5 +262,8 @@ int platform_init(struct reef *reef) dai_probe(ssp); }
+ /* Initialize DMA for Trace*/ + dma_trace_init_complete(reef->dmat); + return 0; } diff --git a/src/platform/cannonlake/platform.c b/src/platform/cannonlake/platform.c index c9449ed..8bf2d7a 100644 --- a/src/platform/cannonlake/platform.c +++ b/src/platform/cannonlake/platform.c @@ -283,5 +283,8 @@ int platform_init(struct reef *reef) dai_probe(ssp); }
+ /* Initialize DMA for Trace*/ + dma_trace_init_complete(reef->dmat); + return 0; }
participants (3)
-
Liam Girdwood -
yan.wang＠linux.intel.com -
yanwang