[Sound-open-firmware] [RFC PATCH v2 05/29] Change trace macro definition and API for trace level checking.

Ranjani Sridharan ranjani.sridharan at linux.intel.com
Fri May 11 07:43:45 CEST 2018


On Thu, 2018-05-10 at 19:31 +0800, sound-open-firmware-bounces at alsa-
project.org wrote:
> From: Yan Wang <yan.wang at linux.intel.com>
> 
> TRACEV and TRACEE are unnecessary because new trace API could
> cover every level (ERROR is 0, NORMAL is 1 and VERBOSE is 2).
> And global, component, buffer and pipeline have different trace
> level checking.:
> For scheduler trace, it uses pipeline trace class previously but
> no pipeline structure as input parameter. So add a new global trace
> class for it.
> 
> Signed-off-by: Yan Wang <yan.wang at linux.intel.com>
> ---
> Test with:
> CNL nocodec
> SOF master: c1f2682c210201bbbc96fce7b382b25b90d19775
> SOF-Tool master: 86fe688a2b4f68a1ce87e0951686be12a00f1a3c
> https://github.com/plbossart/sound/tree/topic/sof-v4.14:
> d09db67c5a9d6dfc85f700669fa2e43c678c8d51
> ---
>  src/include/sof/trace.h | 197 +++++++++++++++++++++++++++++++++++++-
> ----------
>  src/lib/trace.c         | 101 +++++++++++++++++++++++++
>  2 files changed, 254 insertions(+), 44 deletions(-)
> 
> diff --git a/src/include/sof/trace.h b/src/include/sof/trace.h
> index 2fd77fe..c2d874c 100644
> --- a/src/include/sof/trace.h
> +++ b/src/include/sof/trace.h
> @@ -96,13 +96,16 @@
>  #define TRACE_CLASS_EQ_IIR      (20 << 24)
>  #define TRACE_CLASS_SA		(21 << 24)
>  #define TRACE_CLASS_DMIC	(22 << 24)
> +#define TRACE_CLASS_SCH		(23 << 24)
>  
>  /* move to config.h */
>  #define TRACE	1
> -#define TRACEV	0
> -#define TRACEE	1
>  #define TRACEM	0 /* send all trace messages to mbox and local
> trace buffer */
>  
> +#define ERROR_LEVEL	0
> +#define NORMAL_LEVEL	1
> +#define VERBOSE_LEVEL	2
> +
>  void _trace_event(uint32_t event);
>  void _trace_event_mbox(uint32_t event);
>  void _trace_event_atomic(uint32_t event);
> @@ -111,63 +114,169 @@ void trace_flush(void);
>  void trace_off(void);
>  void trace_init(struct sof *sof);
>  
> +#define DECLARE_MODULE_TRACE_FUNCTION(NAME, MODULE) \
> +	void _trace_##NAME##_event(struct MODULE *m, \
> +				   uint32_t level, uint32_t event);
> \
> +	void _trace_##NAME##_event_mbox(struct MODULE *m, \
> +					uint32_t level, uint32_t
> event); \
> +	void _trace_##NAME##_event_atomic(struct MODULE *m, \
> +					  uint32_t level, uint32_t
> event); \
> +	void _trace_##NAME##_event_mbox_atomic(struct MODULE *m, \
> +					       uint32_t level,
> uint32_t event);
> +
> +struct comp_dev;
> +struct comp_buffer;
> +struct pipeline;
> +
> +DECLARE_MODULE_TRACE_FUNCTION(comp, comp_dev)
> +DECLARE_MODULE_TRACE_FUNCTION(buf, comp_buffer)
> +DECLARE_MODULE_TRACE_FUNCTION(pipe, pipeline)

Yan, just curious, why do we need to define these here in trace.h?
Cant we define them in buffer.c, pipeline.c and component.c? And have
them call a generic trace function in trace.h? 

> +
> +void _trace_global_event(uint32_t level, uint32_t event);
> +void _trace_global_event_mbox(uint32_t level, uint32_t event);
> +void _trace_global_event_atomic(uint32_t level, uint32_t event);
> +void _trace_global_event_mbox_atomic(uint32_t level, uint32_t
> event);
> +
> +void _trace_global_value(uint32_t level, uint32_t value);
> +void _trace_global_value_mbox(uint32_t level, uint32_t value);
> +void _trace_global_value_atomic(uint32_t level, uint32_t value);
> +void _trace_global_value_mbox_atomic(uint32_t level, uint32_t
> value);
> +
>  #if TRACE
>  
> -/* send all trace to mbox and local trace buffer */
> +#define trace_point(x) platform_trace_point(x)
> +
> +/* send component trace to mbox and local trace buffer */
>  #if TRACEM
> -#define trace_event(__c, __e) \
> -	_trace_event_mbox(__c | (__e[0] << 16) | (__e[1] << 8) |
> __e[2])
> -#define trace_event_atomic(__c, __e) \
> -	_trace_event_mbox_atomic(__c | (__e[0] << 16) | (__e[1] <<
> 8) | __e[2])
> -/* send trace events only to the local trace buffer */
> +#define trace_comp_event(__cd, __l, __c, __e) \
> +	_trace_comp_event_mbox(__cd, __l, \
> +			       __c | (__e[0] << 16) | (__e[1] << 8)
> | __e[2])
> +#define trace_comp_event_atomic(__cd, __l, __c, __e) \
> +	_trace_comp_event_mbox_atomic_(__cd, __l, \
> +				       __c | (__e[0] << 16) | \
> +				       (__e[1] << 8) | __e[2])
> +#define trace_comp_value(cd, l, x) \
> +	_trace_comp_event_mbox(cd, l, x)
> +#define trace_comp_value_atomic(cd, l, x) \
> +	_trace_comp_event_mobox_atomic(cd, l, x)
> +/* send component trace only to the local trace buffer */
>  #else
> -#define trace_event(__c, __e) \
> -	_trace_event(__c | (__e[0] << 16) | (__e[1] <<8) | __e[2])
> -#define trace_event_atomic(__c, __e) \
> -	_trace_event_atomic(__c | (__e[0] << 16) | (__e[1] <<8) |
> __e[2])
> +#define trace_comp_event(__cd, __l, __c, __e) \
> +	_trace_comp_event(__cd, __l, \
> +			  __c | (__e[0] << 16) | (__e[1] << 8) |
> __e[2])
> +#define trace_comp_event_atomic(__cd, __l, __c, __e) \
> +	_trace_comp_event_atomic(__cd, __l, \
> +				 __c | (__e[0] << 16) | (__e[1] <<
> 8) | __e[2])
> +#define trace_comp_value(cd, l, x) \
> +	_trace_comp_event(cd, l, x)
> +#define trace_comp_value_atomic(cd, l, x) \
> +	_trace_comp_event_atomic(cd, l, x)
>  #endif
> -#define trace_value(x)	_trace_event(x)
> -#define trace_value_atomic(x)	_trace_event_atomic(x)
>  
> -#define trace_point(x) platform_trace_point(x)
> +/* send buffer trace to mbox and local trace buffer */
> +#if TRACEM
> +#define trace_buf_event(__cb, __l, __c, __e) \
> +	_trace_buf_event_mbox(__cb, __l, \
> +			      __c | (__e[0] << 16) | (__e[1] << 8) |
> __e[2])
> +#define trace_buf_event_atomic(__cb, __l, __c, __e) \
> +	_trace_buf_event_mbox_atomic_(__cb, __l, \
> +				      __c | (__e[0] << 16) | \
> +				      (__e[1] << 8) | __e[2])
> +#define trace_buf_value(cb, l, x) \
> +	_trace_buf_event_mbox(cb, l, x)
> +#define trace_buf_value_atomic(cb, l, x) \
> +	_trace_buf_event_mbox_atomic(cb, l, x)
> +/* send buffer trace only to the local trace buffer */
> +#else
> +#define trace_buf_event(__cb, __l, __c, __e) \
> +	_trace_buf_event(__cb, __l, \
> +			 __c | (__e[0] << 16) | (__e[1] << 8) |
> __e[2])
> +#define trace_buf_event_atomic(__cb, __l, __c, __e) \
> +	_trace_buf_event_atomic(__cb, __l, \
> +				__c | (__e[0] << 16) | (__e[1] << 8)
> | __e[2])
> +#define trace_buf_value(cb, l, x) \
> +	_trace_buf_event(cb, l, x)
> +#define trace_buf_value_atomic(cb, l, x) \
> +	_trace_buf_event_atomic(cb, l, x)
> +#endif
>  
> -/* verbose tracing */
> -#if TRACEV
> -#define tracev_event(__c, __e) trace_event(__c, __e)
> -#define tracev_value(x)	_trace_event(x)
> -#define tracev_event_atomic(__c, __e) trace_event_atomic(__c, __e)
> -#define tracev_value_atomic(x)	_trace_event_atomic(x)
> +/* send pipeline trace to mbox and local trace pipeline */
> +#if TRACEM
> +#define trace_pipeline_event(__cp, __l, __c, __e) \
> +	_trace_pipe_event_mbox(__cp, __l, \
> +			       __c | (__e[0] << 16) | (__e[1] << 8)
> | __e[2])
> +#define trace_pipeline_event_atomic(__cp, __l, __c, __e) \
> +	_trace_pipe_event_mbox_atomic_(__cp, __l, \
> +				       __c | (__e[0] << 16) | \
> +				       (__e[1] << 8) | __e[2])
> +#define trace_pipeline_value(cb, l, x) \
> +	_trace_pipe_event_mbox(cb, l, x)
> +#define trace_pipeline_value_atomic(cb, l, x) \
> +	_trace_pipe_event_mbox_atomic(cb, l, x)
> +/* send pipeline trace only to the local trace pipeline */
>  #else
> -#define tracev_event(__c, __e)
> -#define tracev_value(x)
> -#define tracev_event_atomic(__c, __e)
> -#define tracev_value_atomic(x)
> +#define trace_pipeline_event(__cp, __l, __c, __e) \
> +	_trace_pipe_event(__cp, __l, \
> +			  __c | (__e[0] << 16) | (__e[1] << 8) |
> __e[2])
> +#define trace_pipeline_event_atomic(__cp, __l, __c, __e) \
> +	_trace_pipe_event_atomic(__cp, __l, \
> +				 __c | (__e[0] << 16) | (__e[1] <<
> 8) | __e[2])
> +#define trace_pipeline_value(cb, l, x) \
> +	_trace_pipe_event(cb, l, x)
> +#define trace_pipeline_value_atomic(cb, l, x) \
> +	_trace_pipe_event_atomic(cb, l, x)
>  #endif
>  
> -/* error tracing */
> -#if TRACEE
> -#define trace_error(__c, __e) \
> -	_trace_event_mbox_atomic(__c | (__e[0] << 16) | (__e[1] <<
> 8) | __e[2])
> -#define trace_error_atomic(__c, __e) \
> -	_trace_event_mbox_atomic(__c | (__e[0] << 16) | (__e[1] <<
> 8) | __e[2])
> -/* write back error value to mbox */
> -#define trace_error_value(x) _trace_event_mbox_atomic(x)
> -#define trace_error_value_atomic(x) _trace_event_mbox_atomic(x)
> +/* send global trace to mbox and local trace buffer */
> +#if TRACEM
> +#define trace_global_event(__l, __c, __e) \
> +	_trace_global_event_mbox(__l, \
> +				 __c | (__e[0] << 16) | (__e[1] <<
> 8) | __e[2])
> +#define trace_global_event_atomic(__l, __c, __e) \
> +	_trace_global_event_mbox_atomic_(__l, \
> +					 __c | (__e[0] << 16) | \
> +					 (__e[1] << 8) | __e[2])
> +#define trace_global_value(l, x) \
> +	_trace_global_value_mbox(l, x)
> +#define trace_global_value_atomic(l, x) \
> +	_trace_global_value_mbox_atomic(l, x)
> +/* send global trace only to the local trace buffer */
>  #else
> -#define trace_error(__c, __e)
> -#define trace_error_atomic(__c, __e)
> -#define trace_error_value(x)
> -#define trace_error_value_atomic(x)
> +#define trace_global_event(__l, __c, __e) \
> +	_trace_global_event(__l, \
> +			    __c | (__e[0] << 16) | \
> +			    (__e[1] << 8) | __e[2])
> +#define trace_global_event_atomic(__l, __c, __e) \
> +	_trace_global_event_atomic(__l, \
> +				   __c | (__e[0] << 16) | \
> +				   (__e[1] << 8) | __e[2])
> +#define trace_global_value(l, x) \
> +	_trace_global_value(l, x)
> +#define trace_global_value_atomic(l, x) \
> +	_trace_global_value_atomic(l, x)
>  #endif
>  
>  #else
>  
> -#define trace_event(x, e)
> -#define trace_error(c, e)
> -#define trace_value(x)
> -#define trace_point(x)
> -#define tracev_event(__c, __e)
> -#define tracev_value(x)
> +#define trace_comp_event(cd, l, x, e)
> +#define trace_comp_event_atomic(cd, l, c, e)
> +#define trace_comp_value(cd, l, x)
> +#define trace_comp_value_atomic(cd, l, x)
> +
> +#define trace_buf_event(cd, l, x, e)
> +#define trace_buf_event_atomic(cd, l, c, e)
> +#define trace_buf_value(cd, l, x)
> +#define trace_buf_value_atomic(cd, l, x)
> +
> +#define trace_pipeline_event(cd, l, x, e)
> +#define trace_pipeline_event_atomic(cd, l, c, e)
> +#define trace_pipeline_value(cd, l, x)
> +#define trace_pipeline_value_atomic(cd, l, x)
> +
> +#define trace_global_event(l, x, e)
> +#define trace_global_event_atomic(l, c, e)
> +#define trace_global_value(l, x)
> +#define trace_global_value_atomic(l, x)
>  
>  #endif
>  
> diff --git a/src/lib/trace.c b/src/lib/trace.c
> index 0c37858..c522330 100644
> --- a/src/lib/trace.c
> +++ b/src/lib/trace.c
> @@ -35,6 +35,9 @@
>  #include <platform/timer.h>
>  #include <sof/lock.h>
>  #include <sof/dma-trace.h>
> +#include <sof/audio/component.h>
> +#include <sof/audio/buffer.h>
> +#include <sof/audio/pipeline.h>
>  #include <stdint.h>
>  
>  struct trace {
> @@ -136,6 +139,104 @@ void _trace_event_mbox_atomic(uint32_t event)
>  	dcache_writeback_region((void *)t, sizeof(uint64_t) * 2);
>  }
>  
> +#define DEFINE_MODULE_TRACE_FUNCTION(NAME, MODULE) \
> +inline void _trace_##NAME##_event(struct MODULE *m, \
> +				  uint32_t level, uint32_t event) \
> +{ \
> +	if (m && m->trace_level < level) \
> +		return; \
> +	_trace_event(event); \
> +} \
> +inline void _trace_##NAME##_event_atomic(struct MODULE *m, \
> +					 uint32_t level, uint32_t
> event) \
> +{ \
> +	if (m && m->trace_level < level) \
> +		return; \
> +	_trace_event_atomic(event); \
> +} \
> +inline void _trace_##NAME##_event_mbox(struct MODULE *m, \
> +				       uint32_t level, uint32_t
> event) \
> +{ \
> +	if (m && m->trace_level < level) \
> +		return; \
> +	_trace_event_mbox(event); \
> +} \
> +inline void _trace_##NAME##_event_mbox_atomic(struct MODULE *m, \
> +					      uint32_t level,
> uint32_t event) \
> +{ \
> +	if (m && m->trace_level < level) \
> +		return; \
> +	_trace_event_mbox_atomic(event); \
> +}
> +
> +DEFINE_MODULE_TRACE_FUNCTION(comp, comp_dev)
> +DEFINE_MODULE_TRACE_FUNCTION(buf, comp_buffer)
> +DEFINE_MODULE_TRACE_FUNCTION(pipe, pipeline)
> +
> +inline void _trace_global_event(uint32_t level, uint32_t event)
> +{
> +	if (get_global_class_trace_level(event) < level)
> +		return;
> +
> +	_trace_event(event);
> +}
> +
> +inline void _trace_global_event_atomic(uint32_t level, uint32_t
> event)
> +{
> +	if (get_global_class_trace_level(event) < level)
> +		return;
> +
> +	_trace_event_atomic(event);
> +}
> +
> +inline void _trace_global_event_mbox(uint32_t level, uint32_t event)
> +{
> +	if (get_global_class_trace_level(event) < level)
> +		return;
> +
> +	_trace_event_mbox(event);
> +}
> +
> +inline void _trace_global_event_mbox_atomic(uint32_t level, uint32_t
> event)
> +{
> +	if (get_global_class_trace_level(event) < level)
> +		return;
> +
> +	_trace_event_mbox_atomic(event);
> +}
> +
> +inline void _trace_global_value(uint32_t level, uint32_t value)
> +{
> +	if (get_global_value_trace_level() < level)
> +		return;
> +
> +	_trace_event(value);
> +}
> +
> +inline void _trace_global_value_atomic(uint32_t level, uint32_t
> value)
> +{
> +	if (get_global_value_trace_level() < level)
> +		return;
> +
> +	_trace_event_atomic(value);
> +}
> +
> +inline void _trace_global_value_mbox(uint32_t level, uint32_t value)
> +{
> +	if (get_global_value_trace_level() < level)
> +		return;
> +
> +	_trace_event_mbox(value);
> +}
> +
> +inline void _trace_global_value_mbox_atomic(uint32_t level, uint32_t
> value)
> +{
> +	if (get_global_value_trace_level() < level)
> +		return;
> +
> +	_trace_event_mbox_atomic(value);
> +}
> +
>  void trace_flush(void)
>  {
>  	volatile uint64_t *t;


More information about the Sound-open-firmware mailing list