Re: [RFC 04/13] ASoC: Intel: avs: Parse pplcfg and binding tuples

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 2/7/22 07:25, Cezary Rojewski wrote:
> Stream on ADSP firmware is represented by one or more pipelines. Just

I have a lot of questions on this one line...

what is a 'stream'?

'stream' historically means 'direction' in ALSA.

Then we have sdw_stream, which describes how source and sink ports are
connected on a platform.

We also have DPCM front-ends, visible mostly through the PCM device they
expose to users.

In windows we have stream effects that are really meant to be on a
single dedicated pipeline.

other questions: can a stream represent data moving in different
directions, e.g. in full-duplex mode.

How would a loopback be described?

What happens when a data path is split (demux) or merged (mixer)?

How is a 'stream' different from a 'path template' that you referred to
in Patch RFC 02/13

You would need at least 10 lines of plain English to make sure no one
will misunderstand what 'stream' means.

> like modules, these are described by a config structure. Add parsing
> helpers to support loading such information from the topology file.
>
> +/* Specifies path behaviour during PCM ->trigger(START) commnand. */

typo: command.

> +enum avs_tplg_trigger {
> +	AVS_TPLG_TRIGGER_AUTO = 0,
> +	AVS_TPLG_TRIGGER_USERSPACE = 1, /* via sysfs */

The feedback in previous versions was that we should not expose any
sysfs interface that would interfere with decisions made by the driver.

This is very controversial and a major hurdle to upstream any of this.

Debugfs is fine, as suggested by Mark as well

"
If it's mainly used for debugging then it could be exposed through
debugfs with less worry.
"


> +};
> +
> +struct avs_tplg_pplcfg {
> +	u16 req_size;

what does this describe?

> +	u8 priority;
> +	bool lp;
> +	u16 attributes;
> +	enum avs_tplg_trigger trigger;
> +};
> +
> +struct avs_tplg_binding {
> +	char target_tplg_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> +	u32 target_path_tmpl_id;
> +	u32 target_ppl_id;
> +	u32 target_mod_id;
> +	u8 target_mod_pin;

you really need to elaborate on what a template is, and how you use a
template and a ppl id concurrently.

> +	u32 mod_id;
> +	u8 mod_pin;
> +	u8 is_sink;
> +};
> +
>  #endif