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

[Liam Girdwood]

On Fri, 2018-05-18 at 16:52 +0800, yanwang wrote:
> 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.
> 

Ok, you can do 1 file for classes, but components may come and go and may have
different params for each so it makes sense to use a different file for each
component.

> > 
> > 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  


So these are non component trace class IDs now, they dont need to be in ipc.h
and should start at some high number so they wont conflict with component IDs
(that start at 0).

> 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)); 


All you need here is name and ID. level, type will be set by userspace.

> 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; 

num_classes

> 951         struct sof_ipc_dma_trace_level level[0];

trace_class

>  
> 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.

Yes, topology is loaded after FW ready.

> I may have to send component/widget trace level list later. Is it
> right?

No, driver will create component trace debugFS objects when it loads topology.
Driver will already have this data so no need for FW to send it.

Liam

> 
> Thanks.
> Yan Wang
> 
> > 
> > Liam