[Sound-open-firmware] [PATCH] trace: dma: remove disable DMA trace option
Keyon Jie
yang.jie at linux.intel.com
Fri Mar 16 05:34:08 CET 2018
On 2018年03月16日 11:40, Pan, Xiuli wrote:
>
>
> On 3/15/2018 23:09, Liam Girdwood wrote:
>> DMA based trace is the standard default trace method. mailbox trace is
>> no longer supported.
>>
>> Signed-off-by: Liam Girdwood <liam.r.girdwood at linux.intel.com>
>> ---
>> configure.ac | 10 --------
>> src/ipc/dma-copy.c | 2 --
>> src/ipc/intel-ipc.c | 4 ---
>> src/lib/Makefile.am | 6 +----
>> src/lib/trace.c | 70
>> +----------------------------------------------------
>> 5 files changed, 2 insertions(+), 90 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index bb4d2e9f..9b74c6a9 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -241,16 +241,6 @@ case "$with_dsp_core" in
>> esac
>> -# dma trace support (Optional), dma trace by default
>> -AC_ARG_ENABLE([dma-trace],
>> - AS_HELP_STRING([--disable-dma-trace], [Disabled dma trace and
>> use fallback mailbox trace]))
>> -
>> -AS_IF([test "x$enable_dma_trace" != "xno"], [
>> - AC_DEFINE([CONFIG_DMA_TRACE], [1], [Configure DMA trace])
>> - ])
>> -
>> -AM_CONDITIONAL(BUILD_DMA_TRACE, test "x$enable_dma_trace" != "xno")
>> -
>> PLATFORM_BOOT_LDR_LDSCRIPT="boot_ldr.x"
>> AC_SUBST(PLATFORM_BOOT_LDR_LDSCRIPT)
>
> Hi Liam,
>
> I think we should keep this feature. We may need this feature to debug
> the dma related bugs.
> If anything goes wrong with the DMA, or we need to enable DMA on new
> platform, the DMA trace will not work and we may need fall back to mail
> box trace to get the debug info.
Agree to reserve the possibility for mailbox trace, we can set default
to use dma trace, which may be helpful for debugging to special cases,
e.g. bugs for dma(gpdma or hda dma) itself, new platforms enabling as
Xiuli mentioned, the fallback trace do have helped us a lot on these
cases and hope it can help in the future also.
Thanks,
~Keyon
>
> Thanks
> Xiuli
>
>> diff --git a/src/ipc/dma-copy.c b/src/ipc/dma-copy.c
>> index 3c073567..ba7592af 100644
>> --- a/src/ipc/dma-copy.c
>> +++ b/src/ipc/dma-copy.c
>> @@ -79,9 +79,7 @@ static void dma_complete(void *data, uint32_t type,
>> struct dma_sg_elem *next)
>> if (type == DMA_IRQ_TYPE_LLIST)
>> wait_completed(comp);
>> -#if defined(CONFIG_DMA_TRACE)
>> ipc_dma_trace_send_position();
>> -#endif
>> next->size = DMA_RELOAD_END;
>> }
>> diff --git a/src/ipc/intel-ipc.c b/src/ipc/intel-ipc.c
>> index 8263fffe..4173a276 100644
>> --- a/src/ipc/intel-ipc.c
>> +++ b/src/ipc/intel-ipc.c
>> @@ -631,7 +631,6 @@ static int ipc_glb_pm_message(uint32_t header)
>> }
>> }
>> -#if defined(CONFIG_DMA_TRACE)
>> /*
>> * Debug IPC Operations.
>> */
>> @@ -747,7 +746,6 @@ static int ipc_glb_debug_message(uint32_t header)
>> return -EINVAL;
>> }
>> }
>> -#endif
>> /*
>> * Topology IPC Operations.
>> @@ -963,10 +961,8 @@ int ipc_cmd(void)
>> return ipc_glb_stream_message(hdr->cmd);
>> case iGS(SOF_IPC_GLB_DAI_MSG):
>> return ipc_glb_dai_message(hdr->cmd);
>> -#if defined(CONFIG_DMA_TRACE)
>> case iGS(SOF_IPC_GLB_TRACE_MSG):
>> return ipc_glb_debug_message(hdr->cmd);
>> -#endif
>> default:
>> trace_ipc_error("eGc");
>> trace_value(type);
>> diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am
>> index c4da98c9..ea91904a 100644
>> --- a/src/lib/Makefile.am
>> +++ b/src/lib/Makefile.am
>> @@ -8,12 +8,8 @@ libcore_a_SOURCES = \
>> trace.c \
>> schedule.c \
>> agent.c \
>> - interrupt.c
>> -
>> -if BUILD_DMA_TRACE
>> -libcore_a_SOURCES += \
>> + interrupt.c \
>> dma-trace.c
>> -endif
>> libcore_a_CFLAGS = \
>> $(ARCH_CFLAGS) \
>> diff --git a/src/lib/trace.c b/src/lib/trace.c
>> index bfac6d32..de99a4e6 100644
>> --- a/src/lib/trace.c
>> +++ b/src/lib/trace.c
>> @@ -49,9 +49,7 @@ void _trace_error(uint32_t event)
>> {
>> unsigned long flags;
>> volatile uint64_t *t;
>> -#if defined(CONFIG_DMA_TRACE)
>> uint64_t dt[2];
>> -#endif
>> uint64_t time;
>> if (!trace.enable)
>> @@ -59,12 +57,10 @@ void _trace_error(uint32_t event)
>> time = platform_timer_get(platform_timer);
>> -#if defined(CONFIG_DMA_TRACE)
>> /* save event to DMA tracing buffer */
>> dt[0] = time;
>> dt[1] = event;
>> dtrace_event((const char*)dt, sizeof(uint64_t) * 2);
>> -#endif
>> /* send event by mail box too. */
>> spin_lock_irq(&trace.lock, flags);
>> @@ -88,9 +84,8 @@ void _trace_error(uint32_t event)
>> void _trace_error_atomic(uint32_t event)
>> {
>> volatile uint64_t *t;
>> -#if defined(CONFIG_DMA_TRACE)
>> uint64_t dt[2];
>> -#endif
>> +
>> uint64_t time;
>> if (!trace.enable)
>> @@ -98,12 +93,10 @@ void _trace_error_atomic(uint32_t event)
>> time = platform_timer_get(platform_timer);
>> -#if defined(CONFIG_DMA_TRACE)
>> /* save event to DMA tracing buffer */
>> dt[0] = time;
>> dt[1] = event;
>> dtrace_event_atomic((const char*)dt, sizeof(uint64_t) * 2);
>> -#endif
>> /* write timestamp and event to trace buffer */
>> t = (volatile uint64_t*)(MAILBOX_TRACE_BASE + trace.pos);
>> @@ -119,8 +112,6 @@ void _trace_error_atomic(uint32_t event)
>> dcache_writeback_region((void*)t, sizeof(uint64_t) * 2);
>> }
>> -#if defined(CONFIG_DMA_TRACE)
>> -
>> void _trace_event(uint32_t event)
>> {
>> uint64_t dt[2];
>> @@ -145,62 +136,6 @@ void _trace_event_atomic(uint32_t event)
>> dtrace_event_atomic((const char*)dt, sizeof(uint64_t) * 2);
>> }
>> -#else
>> -
>> -void _trace_event(uint32_t event)
>> -{
>> - unsigned long flags;
>> - uint64_t time, *t;
>> -
>> - if (!trace.enable)
>> - return;
>> -
>> - time = platform_timer_get(platform_timer);
>> -
>> - /* send event by mail box too. */
>> - spin_lock_irq(&trace.lock, flags);
>> -
>> - /* write timestamp and event to trace buffer */
>> - t = (uint64_t *)(MAILBOX_TRACE_BASE + trace.pos);
>> - trace.pos += (sizeof(uint64_t) << 1);
>> -
>> - if (trace.pos > MAILBOX_TRACE_SIZE - sizeof(uint64_t) * 2)
>> - trace.pos = 0;
>> -
>> - spin_unlock_irq(&trace.lock, flags);
>> -
>> - t[0] = time;
>> - t[1] = event;
>> -
>> - /* writeback trace data */
>> - dcache_writeback_region((void *)t, sizeof(uint64_t) * 2);
>> -}
>> -
>> -void _trace_event_atomic(uint32_t event)
>> -{
>> - uint64_t time, *t;
>> -
>> - if (!trace.enable)
>> - return;
>> -
>> - time = platform_timer_get(platform_timer);
>> -
>> - /* write timestamp and event to trace buffer */
>> - t = (uint64_t *)(MAILBOX_TRACE_BASE + trace.pos);
>> - trace.pos += (sizeof(uint64_t) << 1);
>> -
>> - if (trace.pos > MAILBOX_TRACE_SIZE - sizeof(uint64_t) * 2)
>> - trace.pos = 0;
>> -
>> - t[0] = time;
>> - t[1] = event;
>> -
>> - /* writeback trace data */
>> - dcache_writeback_region((void *)t, sizeof(uint64_t) * 2);
>> -}
>> -
>> -#endif
>> -
>> void trace_off(void)
>> {
>> trace.enable = 0;
>> @@ -208,10 +143,7 @@ void trace_off(void)
>> void trace_init(struct reef *reef)
>> {
>> -
>> -#if defined(CONFIG_DMA_TRACE)
>> dma_trace_init_early(reef);
>> -#endif
>> trace.enable = 1;
>> trace.pos = 0;
>> spinlock_init(&trace.lock);
>
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>
More information about the Sound-open-firmware
mailing list