[RFC 00/13] ASoC: Intel: avs: Topology and path management

[Pierre-Louis Bossart]

> A continuation of avs-driver initial series [1]. This chapter covers
> path management and topology parsing part which was ealier path of the
> main series. The two patches that represented these two subjects in the
> initial series, have been split into many to allow for easier review and
> discussion.
> 
> AVS topology is split into two major parts: dictionaries - found within
> ASoC topology manifest - and path templates - found within DAPM widget
> private data. Dictionaries job is to reduce the total amount of memory
> occupied by topology elements. Rather than having every pipeline and
> module carry its own information, each refers to specific entry in
> specific dictionary by provided (from topology file) indexes. In
> consequence, most struct avs_tplg_xxx are made out of pointers.
> 
> A 'path' represents a DSP side of audio stream in runtime - is a logical
> container for pipelines which are themselves composed of modules -
> processing units. Due to high range of possible audio format
> combinations, there can be more variants of given path (and thus, its
> pipelines and modules) than total number of pipelines and module
> instances which firmware supports concurrently, all the instance IDs are
> allocated dynamically with help of IDA interface. 'Path templates' come
> from topology file and describe a pattern which is later used to
> actually create runtime 'path'.

This is one of the most 'dense' patchsets I've reviewed in a long time.
While the code looks mostly good, I am afraid the directions and scope
are rather unclear. Now that you've split the basic parts out,
ironically it makes the intent of this patchset really difficult to grasp.

The first order of business is really to clarify the concepts:

What is a 'stream'? what is a 'path'? what is a 'path template'? What is
a 'module template'? What topologies are supported? what is the link
between a path and FE? how does this interoperate or duplicate DPAM? why
does a path rely on a single DMA? What would happen if there is no host
PCM, e.g. for a beep tone generated in firmware? How would this work for
a firmware capture pipeline that only sends notification on acoustic
events and no PCM data?

I have reviewed this set of patches three times already and this set of
concepts are still nebulous to me, please refer to my detailed comments.

You really need to describe in layman's terms what the problem statement
is, what your solution tries to fix, what other options you considered,
what cases you didn't handle, etc. Put yourself if the shoes of someone
that is not part of your team and has no prior exposure to the cAVS
firmware design.

I would recommend that you use the Windows documentation [1] to provide
ascii-art examples with hierarchical mixing, multiple outputs and
loopbacks on what a 'path' really is and how the concept of template helps.

But at a more fundamental level, the main concern I have is with the BE
use: this patchset assumes the BE configuration is fixed, and that's a
very very strong limitation. It's clearly not right for e.g. Bluetooth
offload where multiple profiles need to be supported. It's also not
right when the codec or receiver can adjust to multiple hw_params, which
could lead to simplifications such as removal of unnecessary
SRC/downmixers, etc.

We absolutely want the capability to reconfigure the BE by using
constraint matching between FE hw_params requested by applications and
what the hardware can support. If your solution solved that problem you
would have my full support. But if we're adding a rather complicated
framework on top of a known limitation, then that's really a missed
opportunity to do things better collectively.

[1]
https://docs.microsoft.com/en-us/windows-hardware/drivers/audio/audio-processing-object-architecture