[Sound-open-firmware] [PATCH] [RFC]rmbox: include trace class definitions from trace header

[yanwang]

On Fri, 2018-05-18 at 09:06 +0100, Liam Girdwood wrote:
> On Fri, 2018-05-18 at 10:09 +0800, Yan Wang wrote:
> > 
> > > 
> > > So the trace class strings are deprecated, Yan was going to
> > > export all the
> > > trace
> > > object names + IDs via debugfs (like DAPM widgets already do).
> > > rmbox should
> > > read
> > > these at startup so it could then lookup comp name from the ID
> > > number
> > > embedded
> > > in trace data.
> > > 
> > > Yan, do you have parts of this implemented ?
> > 
> > In my previous RFC v2 patch set, I still keep trace class macro 
> > definition in trace record but support switch on/off based
> > component id.
> > I am working on displaying compoent/widget name in rmbox and will
> > submit 
> > in the next version patch set.
> > But for global trace like DMA/IPC/... or early component trace
> > which 
> > locates in comp_new()/pipe_new()/buffer_new() may still need trace
> > class 
> > because they haven't corresponding component/widget name?
> 
> Non component trace classes can be exported as "extended boot data"
> i.e. 
> 
> 1) create a structure array that can be appended onto the boot
> complete message
> (after SRAM windows) and sent to the host.
> 
> struct ipc_trace_class_elem {
> 	char name[SIZE];
> 	uint32_t id;
> };
> 
> struct ipc_trace_class {
> 	int num_elems;
> 	struct ipc_trace_class_elem[0];
> };
> 
> so you would have 
> 
> struct ipc_trace_class_elem trace_class_elems[] = {
> {"pipeline", PIPELINE_TRACE_ID},
> .....
> };

Yes. I have finished this step now. And I also send name string from
SOF side to kernel driver. 

> 
> 2) The driver will create a debugFS file for each trace class elem.
> When read
> (by rmbox or cat) it will contain the ID number. This way rmbox can
> map IDs to
> names for classes too.

I think we may not need so many debugfs interfaces. We can pass one
parameter into one trace_level debugfs interface get all trace class
map. And rmbox can understand trace class after getting this map.

> 
> 3) rmbox reads these debugFS files at strartup to create it's ID ->
> name
> mapings.
> 
> Steps 2 & 3 are very similar to the component ID name lookups.
> 
> Doing this means rmbox and the driver do not need to be modified when
> we add new
> trace objects in the FW.

Agree. We can remove all trace class macro definition.
both rmbox and kernel driver will be dependent on SOF. So it is
scalable fully.

I attach my finished structure decalation for reviwing in the
following. It still include trace class. I can remove it in the next
version patch set.

922 /* DMA trace level type */ 
923 enum sof_dma_trace_level_type { 
924         SOF_DMA_TRACE_LEVEL_IRQ = 0, 
925         SOF_DMA_TRACE_LEVEL_IPC, 
926         SOF_DMA_TRACE_LEVEL_DMA, 
927         SOF_DMA_TRACE_LEVEL_SSP, 
928         SOF_DMA_TRACE_LEVEL_WAIT, 
929         SOF_DMA_TRACE_LEVEL_LOCK, 
930         SOF_DMA_TRACE_LEVEL_MEM, 
931         SOF_DMA_TRACE_LEVEL_SA, 
932         SOF_DMA_TRACE_LEVEL_DMIC, 
933         SOF_DMA_TRACE_LEVEL_SCH, 
934         SOF_DMA_TRACE_LEVEL_VALUE, 
935         SOF_DMA_TRACE_LEVEL_END, 
936 }; 
937  
938 /* DMA for Trace level elem - SOF_IPC_DEBUG_DMA_LEVELS */ 
939 struct sof_ipc_dma_trace_level { 
940         uint32_t trace_class; 
941         enum sof_dma_trace_level_type type; 
942         int32_t comp_id; 
943         char name[8]; 
944         uint32_t level; 
945 }  __attribute__((packed)); 
946  
947 /* DMA for Trace level info - SOF_IPC_DEBUG_DMA_LEVELS */ 
948 struct sof_ipc_dma_trace_levels { 
949         struct sof_ipc_ext_data_hdr ext_hdr; 
950         uint32_t num_levels; 
951         struct sof_ipc_dma_trace_level level[0]; 
952 }  __attribute__((packed)); 

We can also use this structure to set and query multiply module/class
trace levels.
My current focus issue is component/widget trace level list from SOF
side. When FW_READY message sent, the topology should not be loadded.
I may have to send component/widget trace level list later. Is it
right?

Thanks.
Yan Wang

> 
> Liam