[Sound-open-firmware] [PATCH 1/2] Implement DMA trace level control.
yanwang
yan.wang at linux.intel.com
Fri Apr 13 07:49:39 CEST 2018
On Thu, 2018-04-12 at 11:05 -0500, Pierre-Louis Bossart wrote:
> 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.
I can move it to sof-ipc.h which is shared between kernel and firmware
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.
Based on previous Liam's comments, I will change it. For component
type, trace level will based on component name.
Thanks.
Yan Wang
>
>
> >
> > +};
> > +
> > +#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_MIXE
> > R)
> > +#define DMA_TRACE_LEVEL_MASK_BUFFER BIT(DMA_TRACE_LEVEL_BUF
> > FER)
> > +#define DMA_TRACE_LEVEL_MASK_VOLUME BIT(DMA_TRACE_LEVEL_VOL
> > UME)
> > +#define DMA_TRACE_LEVEL_MASK_SWITCH BIT(DMA_TRACE_LEVEL_SWI
> > TCH)
> > +#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_VALU
> > E)
> > +
> > +#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