From fa37322b9bb6f6e773e73d4f05e66312ec172d6b Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 7 Oct 2021 10:21:46 +0300 Subject: [PATCH 1/2] dma-trace: Implement clenaup on error in dma_trace_start() After the dma_copy_set_stream_tag() a DMA channel is requested, release it in any of the error cases later. Signed-off-by: Peter Ujfalusi --- src/trace/dma-trace.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/trace/dma-trace.c b/src/trace/dma-trace.c index e757c1a94fae..4f77e08d721b 100644 --- a/src/trace/dma-trace.c +++ b/src/trace/dma-trace.c @@ -300,15 +300,24 @@ static int dma_trace_start(struct dma_trace_data *d) config.direction, elem_num, elem_size, elem_addr, 0); if (err < 0) - return err; + goto err_alloc; err = dma_set_config(d->dc.chan, &config); if (err < 0) { mtrace_printf(LOG_LEVEL_ERROR, "dma_set_config() failed: %d", err); - return err; + goto err_config; } err = dma_start(d->dc.chan); + if (err == 0) + return 0; + +err_config: + dma_sg_free(&config.elem_array); + +err_alloc: + dma_channel_put(d->dc.chan); + d->dc.chan = NULL; return err; } From 69f2d54b620a906ff0a18da5df28182fe30cb6e0 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Wed, 6 Oct 2021 16:03:54 +0300 Subject: [PATCH 2/2] dma-trace: Allow DMA re-configuration The host will receive an error if it tries to re-configure the dtrace: [ 22736132.117381] ( 28.645832) c0 hda-dma ..../intel/hda/hda-dma.c:489 ERROR hda-dmac: 4 no free channel 0 [ 22736147.377797] ( 15.260416) c0 dma-copy src/ipc/dma-copy.c:163 ERROR dma_copy_set_stream_tag(): dc->chan is NULL [ 22736180.346545] ( 32.968750) c0 ipc src/ipc/ipc3/handler.c:805 ERROR ipc: failed to enable trace -22 Since we try to request an already used channel. If we skip the requesting then we will fail with the configuration: [ 31707957.542122] ( 27.447916) c0 dma-copy src/ipc/dma-copy.c:162 ERROR dma_copy_set_stream_tag(): already have chan for stream_tag 1 [ 31708021.917120] ( 64.375000) c0 hda-dma ..../intel/hda/hda-dma.c:539 ERROR hda-dmac: 4 channel 0 busy. dgcs 0x800000 status 5 [ 31708056.083785] ( 34.166664) c0 ipc src/ipc/ipc3/handler.c:805 ERROR ipc: failed to enable trace -16 To support the reconfiguration: if we have DMA channel for dtrace, stop it to allow the reconfiguration. In the unlikely case when the stream_id has changed, release the channel as well and request a new one. Tested with the SOF client kernel by rmmod and modprobe the dtrace client during active audio playback to prevent the DSP to be turned off. Signed-off-by: Peter Ujfalusi --- src/include/sof/trace/dma-trace.h | 1 + src/trace/dma-trace.c | 27 ++++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/include/sof/trace/dma-trace.h b/src/include/sof/trace/dma-trace.h index 53cd8bac2fb8..95c48c73da62 100644 --- a/src/include/sof/trace/dma-trace.h +++ b/src/include/sof/trace/dma-trace.h @@ -39,6 +39,7 @@ struct dma_trace_data { uint32_t enabled; uint32_t copy_in_progress; uint32_t stream_tag; + uint32_t active_stream_tag; uint32_t dma_copy_align; /**< Minimal chunk of data possible to be * copied by dma connected to host */ diff --git a/src/trace/dma-trace.c b/src/trace/dma-trace.c index 4f77e08d721b..16c8037402b1 100644 --- a/src/trace/dma-trace.c +++ b/src/trace/dma-trace.c @@ -278,10 +278,35 @@ static int dma_trace_start(struct dma_trace_data *d) return -ENODEV; } - err = dma_copy_set_stream_tag(&d->dc, d->stream_tag); + if (d->dc.chan) { + /* We already have DMA channel for dtrace, stop it */ + mtrace_printf(LOG_LEVEL_WARNING, + "dma_trace_start(): DMA reconfiguration (active stream_tag: %u)", + d->active_stream_tag); + + err = dma_stop(d->dc.chan); + if (err < 0) { + mtrace_printf(LOG_LEVEL_ERROR, + "dma_trace_start(): DMA channel failed to stop"); + } else if (d->active_stream_tag != d->stream_tag) { + /* Re-request a channel if different tag is provided */ + mtrace_printf(LOG_LEVEL_WARNING, + "dma_trace_start(): stream_tag change from %u to %u", + d->active_stream_tag, d->stream_tag); + + dma_channel_put(d->dc.chan); + d->dc.chan = NULL; + err = dma_copy_set_stream_tag(&d->dc, d->stream_tag); + } + } else { + err = dma_copy_set_stream_tag(&d->dc, d->stream_tag); + } + if (err < 0) return err; + d->active_stream_tag = d->stream_tag; + /* size of every trace record */ elem_size = sizeof(uint64_t) * 2;