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

Keyon Jie yang.jie at linux.intel.com
Fri Mar 16 05:43:52 CET 2018



On 2018年03月16日 12:34, Keyon Jie wrote:
> 
> 
> 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, 

sorry, I mean that the 'mailbox trace' may be helpful for ...

Thanks,
~Keyon

> 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