[Sound-open-firmware] [PATCH 1/2] Implement DMA trace level control.
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Thu Apr 12 18:05:29 CEST 2018
On 4/12/18 2:46 AM, yan.wang at linux.intel.com wrote:
> From: Yan Wang <yan.wang at linux.intel.com>
>
> For DMA trace level control module by module, check its trace class
> when the trace adding API is called.
> And it also need one bit field collection to save trace level status
> for every module.
>
> Signed-off-by: Yan Wang <yan.wang at linux.intel.com>
> ---
> Test with:
> Mininow max rt5651 and APL UP^2 nocodec and CNL nocodec
> SOF master: 949c4339ab8cb10f2edd497a671af989d7ea80f3
> SOF-Tool master: 8e8cd884c36e297f5996a7bb4f1e830a0f1ee84c
> https://github.com/plbossart/sound/tree/topic/sof-v4.14:
> 2cd03d26b66f8fee25b8b21ea3f60db11def5b13
> ---
> src/include/sof/dma-trace.h | 57 +++++++++++++++++++++++
> src/lib/dma-trace.c | 109 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 166 insertions(+)
>
> diff --git a/src/include/sof/dma-trace.h b/src/include/sof/dma-trace.h
> index 81ce0a2..265214c 100644
> --- a/src/include/sof/dma-trace.h
> +++ b/src/include/sof/dma-trace.h
> @@ -43,6 +43,60 @@
> #include <platform/platform.h>
> #include <platform/timer.h>
>
> +/* DMA trace level */
> +enum dma_trace_level_type {
> + DMA_TRACE_LEVEL_IRQ = 0,
> + DMA_TRACE_LEVEL_IPC,
> + DMA_TRACE_LEVEL_PIPE,
> + DMA_TRACE_LEVEL_HOST,
> + DMA_TRACE_LEVEL_DAI,
> + DMA_TRACE_LEVEL_DMA,
> + DMA_TRACE_LEVEL_SSP,
> + DMA_TRACE_LEVEL_COMP,
> + DMA_TRACE_LEVEL_WAIT,
> + DMA_TRACE_LEVEL_LOCK,
> + DMA_TRACE_LEVEL_MEM,
> + DMA_TRACE_LEVEL_MIXER,
> + DMA_TRACE_LEVEL_BUFFER,
> + DMA_TRACE_LEVEL_VOLUME,
> + DMA_TRACE_LEVEL_SWITCH,
> + DMA_TRACE_LEVEL_MUX,
> + DMA_TRACE_LEVEL_SRC,
> + DMA_TRACE_LEVEL_TONE,
> + DMA_TRACE_LEVEL_EQ_FIR,
> + DMA_TRACE_LEVEL_EQ_IIR,
> + DMA_TRACE_LEVEL_SA,
> + DMA_TRACE_LEVEL_VALUE,
> + DMA_TRACE_LEVEL_ALL,
It makes no sense to me to put these definitions in a private header (on
both kernel and firmware sides). There should be a shared file that is
updated at the same time on both side.
But at the same time, I am concerned about adding new definitions for
every component just for tracing. this mixes infrastructure and
processing components, this looks unmanageable to me.
> +};
> +
> +#define BIT(x) (1 << (x))
> +
> +#define DMA_TRACE_LEVEL_MASK_IRQ BIT(DMA_TRACE_LEVEL_IRQ)
> +#define DMA_TRACE_LEVEL_MASK_IPC BIT(DMA_TRACE_LEVEL_IPC)
> +#define DMA_TRACE_LEVEL_MASK_PIPE BIT(DMA_TRACE_LEVEL_PIPE)
> +#define DMA_TRACE_LEVEL_MASK_HOST BIT(DMA_TRACE_LEVEL_HOST)
> +#define DMA_TRACE_LEVEL_MASK_DAI BIT(DMA_TRACE_LEVEL_DAI)
> +#define DMA_TRACE_LEVEL_MASK_DMA BIT(DMA_TRACE_LEVEL_DMA)
> +#define DMA_TRACE_LEVEL_MASK_SSP BIT(DMA_TRACE_LEVEL_SSP)
> +#define DMA_TRACE_LEVEL_MASK_COMP BIT(DMA_TRACE_LEVEL_COMP)
> +#define DMA_TRACE_LEVEL_MASK_WAIT BIT(DMA_TRACE_LEVEL_WAIT)
> +#define DMA_TRACE_LEVEL_MASK_LOCK BIT(DMA_TRACE_LEVEL_LOCK)
> +#define DMA_TRACE_LEVEL_MASK_MEM BIT(DMA_TRACE_LEVEL_MEM)
> +#define DMA_TRACE_LEVEL_MASK_MIXER BIT(DMA_TRACE_LEVEL_MIXER)
> +#define DMA_TRACE_LEVEL_MASK_BUFFER BIT(DMA_TRACE_LEVEL_BUFFER)
> +#define DMA_TRACE_LEVEL_MASK_VOLUME BIT(DMA_TRACE_LEVEL_VOLUME)
> +#define DMA_TRACE_LEVEL_MASK_SWITCH BIT(DMA_TRACE_LEVEL_SWITCH)
> +#define DMA_TRACE_LEVEL_MASK_MUX BIT(DMA_TRACE_LEVEL_MUX)
> +#define DMA_TRACE_LEVEL_MASK_SRC BIT(DMA_TRACE_LEVEL_SRC)
> +#define DMA_TRACE_LEVEL_MASK_TONE BIT(DMA_TRACE_LEVEL_TONE)
> +#define DMA_TRACE_LEVEL_MASK_EQ_FIR BIT(DMA_TRACE_LEVEL_EQ_FIR)
> +#define DMA_TRACE_LEVEL_MASK_EQ_IIR BIT(DMA_TRACE_LEVEL_EQ_IIR)
> +#define DMA_TRACE_LEVEL_MASK_SA BIT(DMA_TRACE_LEVEL_SA)
> +#define DMA_TRACE_LEVEL_MASK_VALUE BIT(DMA_TRACE_LEVEL_VALUE)
> +
> +#define DMA_TRACE_LEVEL_MASK_ALL 0xFFFFFFFF
> +
> struct dma_trace_buf {
> void *w_ptr; /* buffer write pointer */
> void *r_ptr; /* buffer read position */
> @@ -65,6 +119,7 @@ struct dma_trace_data {
> uint32_t enabled;
> uint32_t copy_in_progress;
> uint32_t stream_tag;
> + uint32_t trace_level;
> spinlock_t lock;
> };
>
> @@ -73,6 +128,8 @@ int dma_trace_init_complete(struct dma_trace_data *d);
> int dma_trace_host_buffer(struct dma_trace_data *d, struct dma_sg_elem *elem,
> uint32_t host_size);
> int dma_trace_enable(struct dma_trace_data *d);
> +int dma_trace_set_level(struct dma_trace_data *d,
> + enum dma_trace_level_type type, uint32_t level);
> void dma_trace_flush(void *t);
>
> void dtrace_event(const char *e, uint32_t size);
> diff --git a/src/lib/dma-trace.c b/src/lib/dma-trace.c
> index 01a9925..e143aed 100644
> --- a/src/lib/dma-trace.c
> +++ b/src/lib/dma-trace.c
> @@ -269,6 +269,23 @@ static int dma_trace_start(struct dma_trace_data *d)
>
> #endif
>
> +int dma_trace_set_level(struct dma_trace_data *d,
> + enum dma_trace_level_type type, uint32_t level)
> +{
> + if (type == DMA_TRACE_LEVEL_ALL) {
> + d->trace_level = (level == 1) ? 0xFFFFFFFF : 0;
> + return 0;
> + }
> +
> + /* set trace level based on module */
> + if (level == 1)
> + d->trace_level |= (1 << type);
> + else
> + d->trace_level &= ~(1 << type);
> +
> + return 0;
> +}
> +
> int dma_trace_enable(struct dma_trace_data *d)
> {
> #if defined CONFIG_DMA_GW
> @@ -290,6 +307,7 @@ int dma_trace_enable(struct dma_trace_data *d)
> }
>
> d->enabled = 1;
> + d->trace_level = 0xFFFFFFFF;
> work_schedule_default(&d->dmat_work, DMA_TRACE_PERIOD);
> return 0;
> }
> @@ -354,6 +372,91 @@ static void dtrace_add_event(const char *e, uint32_t length)
> trace_data->messages++;
> }
>
> +static int check_trace_level(const char *e, uint32_t length)
> +{
> + uint64_t *dt = (uint64_t *)e;
> + uint32_t class, mask;
> +
> + if (trace_data->trace_level == 0)
> + return 0;
> + else if (trace_data->trace_level == 0xffffffff)
> + return 1;
> +
> + /* check whether this event should be sent */
> + class = (uint32_t)(*(dt + 1)) & 0xff000000;
> +
> + switch (class) {
> + case TRACE_CLASS_IRQ:
> + mask = DMA_TRACE_LEVEL_MASK_IRQ;
> + break;
> + case TRACE_CLASS_IPC:
> + mask = DMA_TRACE_LEVEL_MASK_IPC;
> + break;
> + case TRACE_CLASS_PIPE:
> + mask = DMA_TRACE_LEVEL_MASK_PIPE;
> + break;
> + case TRACE_CLASS_HOST:
> + mask = DMA_TRACE_LEVEL_MASK_HOST;
> + break;
> + case TRACE_CLASS_DAI:
> + mask = DMA_TRACE_LEVEL_MASK_DAI;
> + break;
> + case TRACE_CLASS_DMA:
> + mask = DMA_TRACE_LEVEL_MASK_DMA;
> + break;
> + case TRACE_CLASS_SSP:
> + mask = DMA_TRACE_LEVEL_MASK_SSP;
> + break;
> + case TRACE_CLASS_COMP:
> + mask = DMA_TRACE_LEVEL_MASK_COMP;
> + break;
> + case TRACE_CLASS_WAIT:
> + mask = DMA_TRACE_LEVEL_MASK_WAIT;
> + break;
> + case TRACE_CLASS_LOCK:
> + mask = DMA_TRACE_LEVEL_MASK_LOCK;
> + break;
> + case TRACE_CLASS_MEM:
> + mask = DMA_TRACE_LEVEL_MASK_MEM;
> + break;
> + case TRACE_CLASS_MIXER:
> + mask = DMA_TRACE_LEVEL_MASK_MIXER;
> + break;
> + case TRACE_CLASS_BUFFER:
> + mask = DMA_TRACE_LEVEL_MASK_BUFFER;
> + break;
> + case TRACE_CLASS_VOLUME:
> + mask = DMA_TRACE_LEVEL_MASK_VOLUME;
> + break;
> + case TRACE_CLASS_SWITCH:
> + mask = DMA_TRACE_LEVEL_MASK_SWITCH;
> + break;
> + case TRACE_CLASS_MUX:
> + mask = DMA_TRACE_LEVEL_MASK_MUX;
> + break;
> + case TRACE_CLASS_SRC:
> + mask = DMA_TRACE_LEVEL_MASK_SRC;
> + break;
> + case TRACE_CLASS_TONE:
> + mask = DMA_TRACE_LEVEL_MASK_TONE;
> + break;
> + case TRACE_CLASS_EQ_FIR:
> + mask = DMA_TRACE_LEVEL_MASK_EQ_FIR;
> + break;
> + case TRACE_CLASS_EQ_IIR:
> + mask = DMA_TRACE_LEVEL_MASK_EQ_IIR;
> + break;
> + case TRACE_CLASS_SA:
> + mask = DMA_TRACE_LEVEL_MASK_SA;
> + break;
> + default:
> + mask = DMA_TRACE_LEVEL_MASK_VALUE;
> + break;
> + }
> +
> + return (trace_data->trace_level & mask) ? 1 : 0;
> +}
> +
> void dtrace_event(const char *e, uint32_t length)
> {
> struct dma_trace_buf *buffer = NULL;
> @@ -363,6 +466,9 @@ void dtrace_event(const char *e, uint32_t length)
> length > DMA_TRACE_LOCAL_SIZE / 8 || length == 0)
> return;
>
> + if (!check_trace_level(e, length))
> + return;
> +
> buffer = &trace_data->dmatb;
>
> spin_lock_irq(&trace_data->lock, flags);
> @@ -395,5 +501,8 @@ void dtrace_event_atomic(const char *e, uint32_t length)
> length > DMA_TRACE_LOCAL_SIZE / 8 || length == 0)
> return;
>
> + if (!check_trace_level(e, length))
> + return;
> +
> dtrace_add_event(e, length);
> }
>
More information about the Sound-open-firmware
mailing list