diff mbox series

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

Message ID 20220207132532.3782412-6-cezary.rojewski@intel.com
State Superseded
Headers show
Series ASoC: Intel: avs: Topology and path management | expand

Commit Message

Cezary Rojewski Feb. 7, 2022, 1:25 p.m. UTC 
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.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/topology.c | 177 +++++++++++++++++++++++++++++++++
 sound/soc/intel/avs/topology.h |  31 ++++++
 2 files changed, 208 insertions(+)

Comments

Pierre-Louis Bossart Feb. 25, 2022, 6:51 p.m. UTC | #1 
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'?


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

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

What happens when the user uses switches to disconnects pipelines?

> +	/* Path pipelines management. */

what is a path pipeline?

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

> +
>  #endif
Cezary Rojewski March 21, 2022, 3:14 p.m. UTC | #2 
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
diff mbox series

Patch

diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c
index 7d454a0ea000..8889ceae19b9 100644
--- a/sound/soc/intel/avs/topology.c
+++ b/sound/soc/intel/avs/topology.c
@@ -1004,3 +1004,180 @@  static int avs_tplg_parse_bindings(struct snd_soc_component *comp,
 				AVS_TKN_BINDING_ID_U32,
 				binding_parsers, ARRAY_SIZE(binding_parsers));
 }
+
+static const struct avs_tplg_token_parser module_parsers[] = {
+	{
+		.token = AVS_TKN_MOD_ID_U32,
+		.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
+		.offset = offsetof(struct avs_tplg_module, id),
+		.parse = avs_parse_word_token,
+	},
+	{
+		.token = AVS_TKN_MOD_MODCFG_BASE_ID_U32,
+		.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
+		.offset = offsetof(struct avs_tplg_module, cfg_base),
+		.parse = avs_parse_modcfg_base_ptr,
+	},
+	{
+		.token = AVS_TKN_MOD_IN_AFMT_ID_U32,
+		.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
+		.offset = offsetof(struct avs_tplg_module, in_fmt),
+		.parse = avs_parse_audio_format_ptr,
+	},
+	{
+		.token = AVS_TKN_MOD_CORE_ID_U8,
+		.type = SND_SOC_TPLG_TUPLE_TYPE_BYTE,
+		.offset = offsetof(struct avs_tplg_module, core_id),
+		.parse = avs_parse_byte_token,
+	},
+	{
+		.token = AVS_TKN_MOD_PROC_DOMAIN_U8,
+		.type = SND_SOC_TPLG_TUPLE_TYPE_BYTE,
+		.offset = offsetof(struct avs_tplg_module, domain),
+		.parse = avs_parse_byte_token,
+	},
+	{
+		.token = AVS_TKN_MOD_MODCFG_EXT_ID_U32,
+		.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
+		.offset = offsetof(struct avs_tplg_module, cfg_ext),
+		.parse = avs_parse_modcfg_ext_ptr,
+	},
+};
+
+static struct avs_tplg_module *
+avs_tplg_module_create(struct snd_soc_component *comp, struct avs_tplg_pipeline *owner,
+		       struct snd_soc_tplg_vendor_array *tuples, u32 block_size)
+{
+	struct avs_tplg_module *module;
+	int ret;
+
+	module = devm_kzalloc(comp->card->dev, sizeof(*module), GFP_KERNEL);
+	if (!module)
+		return ERR_PTR(-ENOMEM);
+
+	ret = avs_parse_tokens(comp, module, module_parsers,
+			       ARRAY_SIZE(module_parsers), tuples, block_size);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	module->owner = owner;
+	INIT_LIST_HEAD(&module->node);
+
+	return module;
+}
+
+static const struct avs_tplg_token_parser pipeline_parsers[] = {
+	{
+		.token = AVS_TKN_PPL_ID_U32,
+		.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
+		.offset = offsetof(struct avs_tplg_pipeline, id),
+		.parse = avs_parse_word_token,
+	},
+	{
+		.token = AVS_TKN_PPL_PPLCFG_ID_U32,
+		.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
+		.offset = offsetof(struct avs_tplg_pipeline, cfg),
+		.parse = avs_parse_pplcfg_ptr,
+	},
+	{
+		.token = AVS_TKN_PPL_NUM_BINDING_IDS_U32,
+		.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
+		.offset = offsetof(struct avs_tplg_pipeline, num_bindings),
+		.parse = avs_parse_word_token,
+	},
+};
+
+static const struct avs_tplg_token_parser bindings_parsers[] = {
+	{
+		.token = AVS_TKN_PPL_BINDING_ID_U32,
+		.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
+		.offset = 0, /* to treat pipeline->bindings as dictionary */
+		.parse = avs_parse_binding_ptr,
+	},
+};
+
+static struct avs_tplg_pipeline *
+avs_tplg_pipeline_create(struct snd_soc_component *comp, struct avs_tplg_path *owner,
+			 struct snd_soc_tplg_vendor_array *tuples, u32 block_size)
+{
+	struct avs_tplg_pipeline *pipeline;
+	u32 modblk_size, offset;
+	int ret;
+
+	pipeline = devm_kzalloc(comp->card->dev, sizeof(*pipeline), GFP_KERNEL);
+	if (!pipeline)
+		return ERR_PTR(-ENOMEM);
+
+	pipeline->owner = owner;
+	INIT_LIST_HEAD(&pipeline->mod_list);
+
+	/* Pipeline header MUST be followed by at least one module. */
+	ret = avs_tplg_vendor_array_lookup(tuples, block_size,
+					   AVS_TKN_MOD_ID_U32, &offset);
+	if (!ret && !offset)
+		ret = -EINVAL;
+	if (ret)
+		return ERR_PTR(ret);
+
+	/* Process header which precedes module sections. */
+	ret = avs_parse_tokens(comp, pipeline, pipeline_parsers,
+			       ARRAY_SIZE(pipeline_parsers), tuples, offset);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	block_size -= offset;
+	tuples = avs_tplg_vendor_array_at(tuples, offset);
+
+	/* Optionally, binding sections follow module ones. */
+	ret = avs_tplg_vendor_array_lookup_next(tuples, block_size,
+						AVS_TKN_PPL_BINDING_ID_U32, &offset);
+	if (ret) {
+		if (ret != -ENOENT)
+			return ERR_PTR(ret);
+
+		/* Does header information match actual block layout? */
+		if (pipeline->num_bindings)
+			return ERR_PTR(-EINVAL);
+
+		modblk_size = block_size;
+	} else {
+		pipeline->bindings = devm_kcalloc(comp->card->dev, pipeline->num_bindings,
+						  sizeof(*pipeline->bindings), GFP_KERNEL);
+		if (!pipeline->bindings)
+			return ERR_PTR(-ENOMEM);
+
+		modblk_size = offset;
+	}
+
+	block_size -= modblk_size;
+	do {
+		struct avs_tplg_module *module;
+		u32 esize;
+
+		ret = avs_tplg_vendor_entry_size(tuples, modblk_size,
+						 AVS_TKN_MOD_ID_U32, &esize);
+		if (ret)
+			return ERR_PTR(ret);
+
+		module = avs_tplg_module_create(comp, pipeline, tuples, esize);
+		if (IS_ERR(module)) {
+			dev_err(comp->dev, "parse module failed: %ld\n",
+				PTR_ERR(module));
+			return ERR_CAST(module);
+		}
+
+		list_add_tail(&module->node, &pipeline->mod_list);
+		modblk_size -= esize;
+		tuples = avs_tplg_vendor_array_at(tuples, esize);
+	} while (modblk_size > 0);
+
+	/* What's left is optional range of bindings. */
+	ret = parse_dictionary_entries(comp, tuples, block_size, pipeline->bindings,
+				       pipeline->num_bindings, sizeof(*pipeline->bindings),
+				       AVS_TKN_PPL_BINDING_ID_U32,
+				       bindings_parsers, ARRAY_SIZE(bindings_parsers));
+	if (ret)
+		return ERR_PTR(ret);
+
+	return pipeline;
+}
diff --git a/sound/soc/intel/avs/topology.h b/sound/soc/intel/avs/topology.h
index d23f4aba0bcc..b68c73afcde3 100644
--- a/sound/soc/intel/avs/topology.h
+++ b/sound/soc/intel/avs/topology.h
@@ -129,4 +129,35 @@  struct avs_tplg_binding {
 	u8 is_sink;
 };
 
+struct avs_tplg_path {
+	u32 id;
+};
+
+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;
+	/* Path pipelines management. */
+	struct list_head node;
+};
+
+struct avs_tplg_module {
+	u32 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;
+};
+
 #endif