[Sound-open-firmware] [PATCH] trace: dma: remove disable DMA trace option

Pan, Xiuli xiuli.pan at linux.intel.com
Fri Mar 16 04:40:46 CET 2018



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.

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);



More information about the Sound-open-firmware mailing list