[Sound-open-firmware] [PATCH 0/2] DMA trace level
From: Yan Wang yan.wang@linux.intel.com
Implement DMA trace level based on modules. User can switch on/off for DMA trace of one module.
Yan Wang (2): Implement DMA trace level control. Update trace level status based on received IPC message.
src/include/sof/dma-trace.h | 57 +++++++++++++++++++++++ src/include/uapi/ipc.h | 8 ++++ src/ipc/handler.c | 25 ++++++++++ src/lib/dma-trace.c | 109 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 199 insertions(+)
From: Yan Wang yan.wang@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@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, +}; + +#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); }
On 4/12/18 2:46 AM, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@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@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); }
On Thu, 2018-04-12 at 11:05 -0500, Pierre-Louis Bossart wrote:
On 4/12/18 2:46 AM, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@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@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_tlevel); 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_tlevel) +{
- 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); }
From: Yan Wang yan.wang@linux.intel.com
Need update trace level bit field collection based on IPC messaged from driver side.
Signed-off-by: Yan Wang yan.wang@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/uapi/ipc.h | 8 ++++++++ src/ipc/handler.c | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+)
diff --git a/src/include/uapi/ipc.h b/src/include/uapi/ipc.h index fbf1c07..a562bf7 100644 --- a/src/include/uapi/ipc.h +++ b/src/include/uapi/ipc.h @@ -117,6 +117,7 @@ /* trace and debug */ #define SOF_IPC_TRACE_DMA_PARAMS SOF_CMD_TYPE(0x001) #define SOF_IPC_TRACE_DMA_POSITION SOF_CMD_TYPE(0x002) +#define SOF_IPC_TRACE_DMA_LEVELS SOF_CMD_TYPE(0x003)
/* Get message component id */ #define SOF_IPC_MESSAGE_ID(x) (x & 0xffff) @@ -850,6 +851,13 @@ struct sof_ipc_dma_trace_posn { uint32_t messages; /* total trace messages */ } __attribute__((packed));
+/* DMA for Trace level info - SOF_IPC_DEBUG_DMA_LEVELS */ +struct sof_ipc_dma_trace_levels { + struct sof_ipc_hdr hdr; + uint32_t type; + uint32_t level; +} __attribute__((packed)); + /* * Architecture specific debug */ diff --git a/src/ipc/handler.c b/src/ipc/handler.c index 6a51765..daab3a9 100644 --- a/src/ipc/handler.c +++ b/src/ipc/handler.c @@ -636,6 +636,29 @@ static int ipc_glb_pm_message(uint32_t header) * Debug IPC Operations. */
+static int ipc_dma_trace_levels(uint32_t header) +{ + struct sof_ipc_dma_trace_levels *levels = _ipc->comp_data; + struct sof_ipc_reply reply; + int err; + + err = dma_trace_set_level(_ipc->dmat, levels->type, levels->level); + if (err < 0) + goto error; + + /* write component values to the outbox */ + reply.hdr.size = sizeof(reply); + reply.hdr.cmd = header; + reply.error = 0; + mailbox_hostbox_write(0, &reply, sizeof(reply)); + return 0; + +error: + if (err < 0) + trace_ipc_error("eA!"); + return -EINVAL; +} + static int ipc_dma_trace_config(uint32_t header) { #ifdef CONFIG_HOST_PTABLE @@ -744,6 +767,8 @@ static int ipc_glb_debug_message(uint32_t header) switch (cmd) { case iCS(SOF_IPC_TRACE_DMA_PARAMS): return ipc_dma_trace_config(header); + case iCS(SOF_IPC_TRACE_DMA_LEVELS): + return ipc_dma_trace_levels(header); default: trace_ipc_error("eDc"); trace_error_value(header);
On Thu, 2018-04-12 at 15:46 +0800, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@linux.intel.com
Implement DMA trace level based on modules. User can switch on/off for DMA trace of one module.
Are there any updates to userspace rmbox tools to use this feature ?
Liam
On 4/12/2018 10:14 PM, Liam Girdwood wrote:
On Thu, 2018-04-12 at 15:46 +0800, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@linux.intel.com
Implement DMA trace level based on modules. User can switch on/off for DMA trace of one module.
Are there any updates to userspace rmbox tools to use this feature ?
No currently. IMHO, we could use "echo/cat /sys/kernel/debug/sof/trace_level" to control it. So need I also add another control path to control it by rmbox? Thanks.
Yan Wang
Liam
On Thu, 2018-04-12 at 22:48 +0800, Yan Wang wrote:
On 4/12/2018 10:14 PM, Liam Girdwood wrote:
On Thu, 2018-04-12 at 15:46 +0800, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@linux.intel.com
Implement DMA trace level based on modules. User can switch on/off for DMA trace of one module.
Are there any updates to userspace rmbox tools to use this feature ?
No currently. IMHO, we could use "echo/cat /sys/kernel/debug/sof/trace_level" to control it. So need I also add another control path to control it by rmbox? Thanks.
yeah, we could have a debugFS file "trace_control" and we would send :-
echo "VOL1.1:pipeline:2"
format is component:class:level
This would set trace to level 2 on component "VOL1.1" for pipeline operations
cat trace_control
would list each widget and the trace level for that widget.
Liam
Yan Wang
Liam
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On 4/12/2018 11:07 PM, Liam Girdwood wrote:
On Thu, 2018-04-12 at 22:48 +0800, Yan Wang wrote:
On 4/12/2018 10:14 PM, Liam Girdwood wrote:
On Thu, 2018-04-12 at 15:46 +0800, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@linux.intel.com
Implement DMA trace level based on modules. User can switch on/off for DMA trace of one module.
Are there any updates to userspace rmbox tools to use this feature ?
No currently. IMHO, we could use "echo/cat /sys/kernel/debug/sof/trace_level" to control it. So need I also add another control path to control it by rmbox? Thanks.
yeah, we could have a debugFS file "trace_control" and we would send :-
echo "VOL1.1:pipeline:2"
format is component:class:level
Some trace class may not bind with component. E.g. trace_ipc(). May we ignore component key in this case? Thanks.
Yan Wang
This would set trace to level 2 on component "VOL1.1" for pipeline operations
cat trace_control
would list each widget and the trace level for that widget.
Liam
Yan Wang
Liam
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On 4/12/2018 23:32, Yan Wang wrote:
On 4/12/2018 11:07 PM, Liam Girdwood wrote:
On Thu, 2018-04-12 at 22:48 +0800, Yan Wang wrote:
On 4/12/2018 10:14 PM, Liam Girdwood wrote:
On Thu, 2018-04-12 at 15:46 +0800, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@linux.intel.com
Implement DMA trace level based on modules. User can switch on/off for DMA trace of one module.
Are there any updates to userspace rmbox tools to use this feature ?
No currently. IMHO, we could use "echo/cat /sys/kernel/debug/sof/trace_level" to control it. So need I also add another control path to control it by rmbox? Thanks.
yeah, we could have a debugFS file "trace_control" and we would send :-
echo "VOL1.1:pipeline:2"
format is component:class:level
Some trace class may not bind with component. E.g. trace_ipc(). May we ignore component key in this case? Thanks.
You can handle that in the debugfs file write function. For those comp did not bind to PCM, you can check other formats.
But there is also some risk here if we need to phrase these strings, we may typo and may hard to modify the trace level. I suggest we add a tool alongside the rmbox to set these for us. We can still use string for status check but a tool to generate the string for us.
Thanks Xiuli
Yan Wang
This would set trace to level 2 on component "VOL1.1" for pipeline operations
cat trace_control
would list each widget and the trace level for that widget.
Liam
Yan Wang
Liam
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On Fri, 2018-04-13 at 00:09 +0800, Pan, Xiuli wrote:
On 4/12/2018 23:32, Yan Wang wrote:
On 4/12/2018 11:07 PM, Liam Girdwood wrote:
On Thu, 2018-04-12 at 22:48 +0800, Yan Wang wrote:
On 4/12/2018 10:14 PM, Liam Girdwood wrote:
On Thu, 2018-04-12 at 15:46 +0800, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@linux.intel.com
Implement DMA trace level based on modules. User can switch on/off for DMA trace of one module.
Are there any updates to userspace rmbox tools to use this feature ?
No currently. IMHO, we could use "echo/cat /sys/kernel/debug/sof/trace_level" to control it. So need I also add another control path to control it by rmbox? Thanks.
yeah, we could have a debugFS file "trace_control" and we would send :-
echo "VOL1.1:pipeline:2"
format is component:class:level
Some trace class may not bind with component. E.g. trace_ipc(). May we ignore component key in this case? Thanks.
You can handle that in the debugfs file write function. For those comp did not bind to PCM, you can check other formats.
Yes, I can try it.
But there is also some risk here if we need to phrase these strings, we may typo and may hard to modify the trace level. I suggest we add a tool alongside the rmbox to set these for us. We can still use string for status check but a tool to generate the string for us.
Based on Liam's comments, I will add control into rmbox. I will try to cover this point. Thanks.
Yan Wang
Thanks Xiuli
Yan Wang
This would set trace to level 2 on component "VOL1.1" for pipeline operations
cat trace_control
would list each widget and the trace level for that widget.
Liam
Yan Wang
Liam
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-fir mware
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmwar e
On Thu, 2018-04-12 at 16:07 +0100, Liam Girdwood wrote:
On Thu, 2018-04-12 at 22:48 +0800, Yan Wang wrote:
On 4/12/2018 10:14 PM, Liam Girdwood wrote:
On Thu, 2018-04-12 at 15:46 +0800, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@linux.intel.com
Implement DMA trace level based on modules. User can switch on/off for DMA trace of one module.
Are there any updates to userspace rmbox tools to use this feature ?
No currently. IMHO, we could use "echo/cat /sys/kernel/debug/sof/trace_level" to control it. So need I also add another control path to control it by rmbox? Thanks.
yeah, we could have a debugFS file "trace_control" and we would send :-
echo "VOL1.1:pipeline:2"
format is component:class:level
This would set trace to level 2 on component "VOL1.1" for pipeline operations
cat trace_control
would list each widget and the trace level for that widget.
Hi, Liam, I did some investigation on trace level for widget/component and initial implementation. On kernel side, I iterate sdev->widget_list to get widget name and component id for trace level. The component id can be used as index for component searching. There is also no problem to transfer trace level by IPC based on component id. On firmware side, I add variable into struct comp_dev for saving trace level for every loaded component. But there are 2 possible issues: 1. Current trace macro doesn't include component id in audio component implementation like src/audio/(src.c, host.c, ...). For trace level switch, I may have to change all existed trace with component id. 2. Not all functions of audio component include struct comp_dev as input parameter. E.g. src_buffer_lengths() in src/audo/src.c. For bind to component id, its input parameter may have to be changed. It is possible to load multiply same widgets in one pipeline. So global variable may not cover this issue. I am not sure whether the above 2 requests are acceptable. Or may there be better solution for this? Thanks.
Yan Wang
Liam
Yan Wang
Liam
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmwar e
On 4/19/2018 2:39 PM, yanwang wrote:
On Thu, 2018-04-12 at 16:07 +0100, Liam Girdwood wrote:
On Thu, 2018-04-12 at 22:48 +0800, Yan Wang wrote:
On 4/12/2018 10:14 PM, Liam Girdwood wrote:
On Thu, 2018-04-12 at 15:46 +0800, yan.wang@linux.intel.com wrote:
From: Yan Wang yan.wang@linux.intel.com
Implement DMA trace level based on modules. User can switch on/off for DMA trace of one module.
Are there any updates to userspace rmbox tools to use this feature ?
No currently. IMHO, we could use "echo/cat /sys/kernel/debug/sof/trace_level" to control it. So need I also add another control path to control it by rmbox? Thanks.
yeah, we could have a debugFS file "trace_control" and we would send :-
echo "VOL1.1:pipeline:2"
format is component:class:level
This would set trace to level 2 on component "VOL1.1" for pipeline operations
cat trace_control
would list each widget and the trace level for that widget.
Hi, Liam, I did some investigation on trace level for widget/component and initial implementation. On kernel side, I iterate sdev->widget_list to get widget name and component id for trace level. The component id can be used as index for component searching. There is also no problem to transfer trace level by IPC based on component id. On firmware side, I add variable into struct comp_dev for saving trace level for every loaded component. But there are 2 possible issues:
- Current trace macro doesn't include component id in audio
component implementation like src/audio/(src.c, host.c, ...). For trace level switch, I may have to change all existed trace with component id. 2. Not all functions of audio component include struct comp_dev as input parameter. E.g. src_buffer_lengths() in src/audo/src.c. For bind to component id, its input parameter may have to be changed. It is possible to load multiply same widgets in one pipeline. So global variable may not cover this issue. I am not sure whether the above 2 requests are acceptable. Or may there be better solution for this?
Hi, Liam, Could you please give some comments about my question? Thanks.
Yan Wang
Thanks.
Yan Wang
Liam
Yan Wang
Liam
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmwar e
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On 25.04.2018 08:31, Yan Wang wrote:
2. Not all functions of audio component include struct comp_dev as input parameter. E.g. src_buffer_lengths() in src/audo/src.c.
I think in this case the trace could be handled (or omitted) from upper level function. I've used myself a lot trace prints and some are not really mandatory to have.
Thanks, Seppo
On Wed, 2018-04-25 at 10:56 +0300, Seppo Ingalsuo wrote:
On 25.04.2018 08:31, Yan Wang wrote:
2. Not all functions of audio component include struct comp_dev as input parameter. E.g. src_buffer_lengths() in src/audo/src.c.
I think in this case the trace could be handled (or omitted) from upper level function. I've used myself a lot trace prints and some are not really mandatory to have.
So the function must have compoent id or comp_dev structure parameter if developer want to add trace? If this is OK, I can ignore question 2. Thanks.
Yan Wang
Thanks, Seppo
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On Wed, 2018-04-25 at 13:31 +0800, Yan Wang wrote:
Hi, Liam, I did some investigation on trace level for widget/component and initial implementation. On kernel side, I iterate sdev->widget_list to get widget name and component id for trace level. The component id can be used as index for component searching. There is also no problem to transfer trace level by IPC based on component id. On firmware side, I add variable into struct comp_dev for saving trace level for every loaded component. But there are 2 possible issues: 1. Current trace macro doesn't include component id in audio component implementation like src/audio/(src.c, host.c, ...). For trace level switch, I may have to change all existed trace with component id.
Add uint32_t trace_level to every component. This will be set/enabled by trace IPC from host.
then in trace.h
#define trace_comp(comp_dev, level, trace_data) \ if (comp_dev->trace_level >= level) \ emit_trace(data);
2. Not all functions of audio component include struct comp_devas input parameter. E.g. src_buffer_lengths() in src/audo/src.c.
best to fix them.
For bind to component id, its input parameter may have to be changed. It is possible to load multiply same widgets in one pipeline. So global variable may not cover this issue. I am not sure whether the above 2 requests are acceptable. Or may there be better solution for this?
For non components, a similar local uint32_t trace_level can be added and set by IPC and in cases where we cannot yet use a local trace_level then we can use a global trace_level.
Liam
Hi, Liam, Could you please give some comments about my question? Thanks.
Yan Wang
On Wed, 2018-05-02 at 09:45 +0100, Liam Girdwood wrote:
On Wed, 2018-04-25 at 13:31 +0800, Yan Wang wrote:
Hi, Liam, I did some investigation on trace level for widget/component and initial implementation. On kernel side, I iterate sdev->widget_list to get widget name and component id for trace level. The component id can be used as index for component searching. There is also no problem to transfer trace level by IPC based on component id. On firmware side, I add variable into struct comp_dev for saving trace level for every loaded component. But there are 2 possible issues: 1. Current trace macro doesn't include component id in audio component implementation like src/audio/(src.c, host.c, ...). For trace level switch, I may have to change all existed trace with component id.
Add uint32_t trace_level to every component. This will be set/enabled by trace IPC from host.
then in trace.h
#define trace_comp(comp_dev, level, trace_data) \ if (comp_dev->trace_level >= level) \ emit_trace(data);
Yes. But current existed trace calling need be replaced by trace_comp()?
2. Not all functions of audio component include struct comp_dev as input parameter. E.g. src_buffer_lengths() in src/audo/src.c.
best to fix them.
Sure. I can fix it.
For bind to component id, its input parameter may have to be changed. It is possible to load multiply same widgets in one pipeline. So global variable may not cover this issue. I am not sure whether the above 2 requests are acceptable. Or may there be better solution for this?
For non components, a similar local uint32_t trace_level can be added and set by IPC and in cases where we cannot yet use a local trace_level then we can use a global trace_level.
Sure. Thanks.
Yan Wang
Liam
Hi, Liam, Could you please give some comments about my question? Thanks.
Yan Wang
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On Wed, 2018-05-02 at 10:10 +0100, Liam Girdwood wrote:
On Wed, 2018-05-02 at 16:53 +0800, yanwang wrote:
#define trace_comp(comp_dev, level, trace_data) \ if (comp_dev->trace_level >= level) \ emit_trace(data);
Yes. But current existed trace calling need be replaced by trace_comp()?
Yes.
Thanks for your comments. I will finish the refining and submit as v2.
Yan Wang
participants (7)
-
Liam Girdwood -
Pan, Xiuli -
Pierre-Louis Bossart -
Seppo Ingalsuo -
Yan Wang -
yan.wang＠linux.intel.com
-
yanwang