[RFC 05/13] ASoC: Intel: avs: Parse pipeline and module tuples

[Cezary Rojewski]

On 2022-02-25 7:51 PM, Pierre-Louis Bossart wrote:
> On 2/7/22 07:25, Cezary Rojewski wrote:
>> Shape of a stream on DSP side, that is, the number and the layout of
>> its pipelines and modules is paramount for streaming to be efficient and
>> low power-consuming. Add parsing helpers to support loading such
>> information from the topology file.
> 
> again what is a 'stream'?


A collection of pipelines, usually connecting HOST (HDA DMA) gateway 
with LINK (GPDMA) gateway.

>> +struct avs_tplg_path {
>> +	u32 id;
>> +};
> 
> A concept that boils down to a single integer is really far from clear
> to me. What does this represent really?


Nah, the structure is much larger. Here, to have pipeline parsing 
separated from the rest, some stub needed to be provided.

>> +
>> +struct avs_tplg_pipeline {
>> +	u32 id;
>> +
>> +	struct avs_tplg_pplcfg *cfg;
>> +	struct avs_tplg_binding **bindings;
>> +	u32 num_bindings;
>> +	struct list_head mod_list;
>> +
>> +	struct avs_tplg_path *owner;
> 
> the cardinality between path and pipeline is far from clear. When you
> have topologies where the same data can be rendered on multiple outputs
> and demuxed into an echo reference, then what is the 'path'?
> 
> Worst case all connected pipelines could be a single path with this
> hierarchical definition of ownership, but is this desired?

Just like in other DSP driver cases, here topology states all the 
possibilities. It's not random by any means.

If you want given data to be rendered on multiple outputs, then you make 
use of NxFEs -> 1xBE which ASoC supports through re-parenting. Multiple 
FEs in topology would be leading to the very same BE widget. On firmware 
side you have copier which supports several outputs. You just choose 
different output pin for each FE.

> What happens when the user uses switches to disconnects pipelines?

"switches to disconnects pipelines"? How could user switch to a 
disconnected pipeline? Not following.

>> +	/* Path pipelines management. */
> 
> what is a path pipeline?

Does this question mean you want "Path" part of the comment to be removed?

>> +	struct list_head node;
>> +};
>> +
>> +struct avs_tplg_module {
>> +	u32 id;
> 
> what is the definition of id? is this local to the scope of a pipeline?
> global for the entire topology?

It's module ID, so it's local. Basically every structure starts here 
with 'id'.

>> +
>> +	struct avs_tplg_modcfg_base *cfg_base;
>> +	struct avs_audio_format *in_fmt;
>> +	u8 core_id;
>> +	u8 domain;
>> +	struct avs_tplg_modcfg_ext *cfg_ext;
>> +
>> +	struct avs_tplg_pipeline *owner;
>> +	/* Pipeline modules management. */
>> +	struct list_head node;
>> +};
> 
> I would expect all modules to be seen as DAPM widgets, no?

Makes no sense since module alone have no real impact on whether audio 
path should be powered or not. Only the entire "path" has any saying in 
that.

>> +
>>   #endif