Skip to content

Commit 64c7b98

Browse files
ujfalusilgirdwood
authored andcommitted
dma-trace: Make dmatb and DMA_GW dma_sg persistent after first allocation
Do not free the dmatb so that the trace code can continue collecting information continuously in it's buffer. We also have another memory leak (when CONFIG_DMA_GW is set) which can be handled this way: Every time the dma_trace_start() is called we allocate a new dma_sg. This is fine when the dma_trace_start() is only called once right after the DSP finished booting, but multiple calls would eventually going to lead to OOM. Move the dma_sg allocation at the same place where the dmatb is allocated to make it persistent as well. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
1 parent 1ae525a commit 64c7b98

File tree

2 files changed

+50
-39
lines changed

2 files changed

+50
-39
lines changed

src/include/sof/trace/dma-trace.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ struct dma_trace_buf {
3030
struct dma_trace_data {
3131
struct dma_sg_config config;
3232
struct dma_trace_buf dmatb;
33+
#if CONFIG_DMA_GW
34+
struct dma_sg_config gw_config;
35+
#endif
3336
struct dma_copy dc;
3437
struct sof_ipc_dma_trace_posn posn;
3538
struct ipc_msg *msg;

src/trace/dma-trace.c

Lines changed: 47 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ static enum task_state trace_work(void *data)
5959
int32_t size;
6060
uint32_t overflow;
6161

62+
/* The host DMA channel is not available */
63+
if (!d->dc.chan)
64+
return SOF_TASK_STATE_RESCHEDULE;
65+
6266
/* make sure we don't write more than buffer */
6367
if (avail > DMA_TRACE_LOCAL_SIZE) {
6468
overflow = avail - DMA_TRACE_LOCAL_SIZE;
@@ -218,8 +222,26 @@ int dma_trace_host_buffer(struct dma_trace_data *d,
218222
}
219223
#endif
220224

225+
static void dma_trace_buffer_free(struct dma_trace_data *d)
226+
{
227+
struct dma_trace_buf *buffer = &d->dmatb;
228+
k_spinlock_key_t key;
229+
230+
key = k_spin_lock(&d->lock);
231+
232+
rfree(buffer->addr);
233+
memset(buffer, 0, sizeof(*buffer));
234+
235+
k_spin_unlock(&d->lock, key);
236+
}
237+
221238
static int dma_trace_buffer_init(struct dma_trace_data *d)
222239
{
240+
#if CONFIG_DMA_GW
241+
struct dma_sg_config *config = &d->gw_config;
242+
uint32_t elem_size, elem_addr, elem_num;
243+
int ret;
244+
#endif
223245
struct dma_trace_buf *buffer = &d->dmatb;
224246
void *buf;
225247
k_spinlock_key_t key;
@@ -271,6 +293,30 @@ static int dma_trace_buffer_init(struct dma_trace_data *d)
271293

272294
k_spin_unlock(&d->lock, key);
273295

296+
#if CONFIG_DMA_GW
297+
/* size of every trace record */
298+
elem_size = sizeof(uint64_t) * 2;
299+
300+
/* Initialize address of local elem */
301+
elem_addr = (uint32_t)buffer->addr;
302+
303+
/* the number of elem list */
304+
elem_num = DMA_TRACE_LOCAL_SIZE / elem_size;
305+
306+
config->direction = DMA_DIR_LMEM_TO_HMEM;
307+
config->src_width = sizeof(uint32_t);
308+
config->dest_width = sizeof(uint32_t);
309+
config->cyclic = 0;
310+
311+
ret = dma_sg_alloc(&config->elem_array, SOF_MEM_ZONE_SYS,
312+
config->direction, elem_num, elem_size,
313+
elem_addr, 0);
314+
if (ret < 0) {
315+
dma_trace_buffer_free(d);
316+
return ret;
317+
}
318+
#endif
319+
274320
#ifdef __ZEPHYR__
275321
#define ZEPHYR_VER_OPT " zephyr:" META_QUOTE(BUILD_VERSION)
276322
#else
@@ -299,25 +345,10 @@ static int dma_trace_buffer_init(struct dma_trace_data *d)
299345
return 0;
300346
}
301347

302-
static void dma_trace_buffer_free(struct dma_trace_data *d)
303-
{
304-
struct dma_trace_buf *buffer = &d->dmatb;
305-
k_spinlock_key_t key;
306-
307-
key = k_spin_lock(&d->lock);
308-
309-
rfree(buffer->addr);
310-
memset(buffer, 0, sizeof(*buffer));
311-
312-
k_spin_unlock(&d->lock, key);
313-
}
314-
315348
#if CONFIG_DMA_GW
316349

317350
static int dma_trace_start(struct dma_trace_data *d)
318351
{
319-
struct dma_sg_config config;
320-
uint32_t elem_size, elem_addr, elem_num;
321352
int err = 0;
322353

323354
/* DMA Controller initialization is platform-specific */
@@ -361,27 +392,7 @@ static int dma_trace_start(struct dma_trace_data *d)
361392

362393
d->active_stream_tag = d->stream_tag;
363394

364-
/* size of every trace record */
365-
elem_size = sizeof(uint64_t) * 2;
366-
367-
/* Initialize address of local elem */
368-
elem_addr = (uint32_t)d->dmatb.addr;
369-
370-
/* the number of elem list */
371-
elem_num = DMA_TRACE_LOCAL_SIZE / elem_size;
372-
373-
config.direction = DMA_DIR_LMEM_TO_HMEM;
374-
config.src_width = sizeof(uint32_t);
375-
config.dest_width = sizeof(uint32_t);
376-
config.cyclic = 0;
377-
378-
err = dma_sg_alloc(&config.elem_array, SOF_MEM_ZONE_SYS,
379-
config.direction,
380-
elem_num, elem_size, elem_addr, 0);
381-
if (err < 0)
382-
goto error;
383-
384-
err = dma_set_config(d->dc.chan, &config);
395+
err = dma_set_config(d->dc.chan, &d->gw_config);
385396
if (err < 0) {
386397
mtrace_printf(LOG_LEVEL_ERROR, "dma_set_config() failed: %d", err);
387398
goto error;
@@ -501,9 +512,6 @@ void dma_trace_disable(struct dma_trace_data *d)
501512
d->dc.chan = NULL;
502513
}
503514

504-
/* free trace buffer */
505-
dma_trace_buffer_free(d);
506-
507515
#if (CONFIG_HOST_PTABLE)
508516
/* Free up the host SG if it is set */
509517
if (d->host_size) {

0 commit comments

Comments
 (0)