Skip to content

Commit 2ae3328

Browse files
committed
Revert: "dma-trace: allocate trace buffer only after enabling traces"
A logical, not "real" git revert of commit eca2089 ("dma-trace: allocate trace buffer only after enabling traces") Move dma_trace_buffer_init() away from dma_trace_enable() and back to dma_trace_init_early() Restore the early traces removed by that commit. There was no rationale in the commit message and it was merged without review in PR #255. Earlier refactorings tried hard to trace early: - commit 36e425e ("dma-trace: move to earlier initialisation point") - commit 2b86cb3 ("trace: dma: Make sure we can trace platform device initialisation.") ... so it's really not clear why early tracing was removed later. "Rage quit" to avoid unidentified bugs? This could have been to save one or two 4k HOST_PAGE_SIZE when the kernel does not enable DMA traces but I can't see what real-world use case would leverage such savings _at run-time_; what happens then when the user changes her mind? We don't have any validation for a use case so dynamic. Tracing can be disabled at _compile-time_, recently fixed by commit 571cc29 ("xtensa/cmake: fix !CONFIG_TRACE") Signed-off-by: Marc Herbert <marc.herbert@intel.com>
1 parent 0eda430 commit 2ae3328

File tree

1 file changed

+49
-33
lines changed

1 file changed

+49
-33
lines changed

src/trace/dma-trace.c

Lines changed: 49 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ static int dma_trace_get_avail_data(struct dma_trace_data *d,
4646
struct dma_trace_buf *buffer,
4747
int avail);
4848

49+
static int dma_trace_buffer_init(struct dma_trace_data *d);
50+
static void dma_trace_buffer_free(struct dma_trace_data *d);
51+
4952
/** Periodically runs and starts the DMA even when the buffer is not
5053
* full.
5154
*/
@@ -141,6 +144,10 @@ int dma_trace_init_early(struct sof *sof)
141144

142145
sof->dmat = rzalloc(SOF_MEM_ZONE_SYS_SHARED, 0, SOF_MEM_CAPS_RAM, sizeof(*sof->dmat));
143146

147+
ret = dma_trace_buffer_init(sof->dmat);
148+
if (ret)
149+
goto err;
150+
144151
dma_sg_init(&sof->dmat->config.elem_array);
145152
k_spinlock_init(&sof->dmat->lock);
146153

@@ -152,12 +159,42 @@ int dma_trace_init_early(struct sof *sof)
152159
goto err;
153160
}
154161

162+
#ifdef __ZEPHYR__
163+
#define ZEPHYR_VER_OPT " zephyr:" META_QUOTE(BUILD_VERSION)
164+
#else
165+
#define ZEPHYR_VER_OPT
166+
#endif
167+
168+
/* META_QUOTE(SOF_SRC_HASH) is part of the format string so it
169+
* goes to the .ldc file and does not go to the firmware
170+
* binary. It will be different from SOF_SRC_HASH in case of
171+
* mismatch.
172+
*/
173+
#define SOF_BANNER_COMMON \
174+
"FW ABI 0x%x DBG ABI 0x%x tags SOF:" SOF_GIT_TAG ZEPHYR_VER_OPT \
175+
" src hash 0x%08x (ldc hash " META_QUOTE(SOF_SRC_HASH) ")" \
176+
" HACK: hardcoded DMA_ATTR_BUFFER_ADDRESS_ALIGNMENT"
177+
178+
179+
/* It should be the very first sent log for easy identification. */
180+
mtrace_printf(LOG_LEVEL_INFO,
181+
"SHM: " SOF_BANNER_COMMON,
182+
SOF_ABI_VERSION, SOF_ABI_DBG_VERSION, SOF_SRC_HASH);
183+
184+
/* Use a different, DMA: prefix to ease identification of log files */
185+
tr_info(&dt_tr,
186+
"DMA: " SOF_BANNER_COMMON,
187+
SOF_ABI_VERSION, SOF_ABI_DBG_VERSION, SOF_SRC_HASH);
188+
155189
return 0;
156190

157191
err:
158192
mtrace_printf(LOG_LEVEL_ERROR,
159193
"dma_trace_init_early() failed: %d", ret);
160194

195+
if (sof->dmat && sof->dmat->dmatb.addr)
196+
dma_trace_buffer_free(sof->dmat);
197+
161198
/* Cannot rfree(sof->dmat) from the system memory pool, see
162199
* comments in lib/alloc.c
163200
*/
@@ -173,9 +210,10 @@ int dma_trace_init_complete(struct dma_trace_data *d)
173210

174211
tr_info(&dt_tr, "dma_trace_init_complete()");
175212

176-
if (!d) {
213+
if (!dma_trace_initialized(d)) {
177214
mtrace_printf(LOG_LEVEL_ERROR,
178-
"dma_trace_init_complete(): failed, no dma_trace_data");
215+
"dma_trace_init_complete(): failed, dmat=%p, dmat->dmatb.addr=%p",
216+
d, d ? d->dmatb.addr : NULL);
179217
return -ENOMEM;
180218
}
181219

@@ -224,6 +262,8 @@ static int dma_trace_buffer_init(struct dma_trace_data *d)
224262
void *buf;
225263
k_spinlock_key_t key;
226264
uint32_t addr_align;
265+
266+
#if 0 // early DMA trace init HACK
227267
int err;
228268

229269
if (!d || !d->dc.dmac) {
@@ -236,6 +276,9 @@ static int dma_trace_buffer_init(struct dma_trace_data *d)
236276
&addr_align);
237277
if (err < 0)
238278
return err;
279+
#else
280+
addr_align = 0x80;
281+
#endif
239282

240283
/* For DMA to work properly the buffer must be correctly aligned */
241284
buf = rballoc_align(0, SOF_MEM_CAPS_RAM | SOF_MEM_CAPS_DMA,
@@ -418,41 +461,14 @@ static int dma_trace_get_avail_data(struct dma_trace_data *d,
418461
*/
419462
int dma_trace_enable(struct dma_trace_data *d)
420463
{
421-
int err;
422-
423-
/* initialize dma trace buffer */
424-
err = dma_trace_buffer_init(d);
464+
int err = 0;
425465

426-
if (err < 0) {
427-
mtrace_printf(LOG_LEVEL_ERROR, "dma_trace_enable: buffer_init failed");
466+
if (!dma_trace_initialized(d)) {
467+
mtrace_printf(LOG_LEVEL_ERROR, "No buffer, did dma_init fail?");
468+
err = -ENOMEM;
428469
goto out;
429470
}
430471

431-
#ifdef __ZEPHYR__
432-
#define ZEPHYR_VER_OPT " zephyr:" META_QUOTE(BUILD_VERSION)
433-
#else
434-
#define ZEPHYR_VER_OPT
435-
#endif
436-
437-
/* META_QUOTE(SOF_SRC_HASH) is part of the format string so it
438-
* goes to the .ldc file and does not go to the firmware
439-
* binary. It will be different from SOF_SRC_HASH in case of
440-
* mismatch.
441-
*/
442-
#define SOF_BANNER_COMMON \
443-
"FW ABI 0x%x DBG ABI 0x%x tags SOF:" SOF_GIT_TAG ZEPHYR_VER_OPT \
444-
" src hash 0x%08x (ldc hash " META_QUOTE(SOF_SRC_HASH) ")"
445-
446-
/* It should be the very first sent log for easy identification. */
447-
mtrace_printf(LOG_LEVEL_INFO,
448-
"SHM: " SOF_BANNER_COMMON,
449-
SOF_ABI_VERSION, SOF_ABI_DBG_VERSION, SOF_SRC_HASH);
450-
451-
/* Use a different, DMA: prefix to ease identification of log files */
452-
tr_info(&dt_tr,
453-
"DMA: " SOF_BANNER_COMMON,
454-
SOF_ABI_VERSION, SOF_ABI_DBG_VERSION, SOF_SRC_HASH);
455-
456472
#if CONFIG_DMA_GW
457473
/*
458474
* GW DMA need finish DMA config and start before

0 commit comments

Comments
 (0)