Re: [PATCH v7 18/22] ASoC: qdsp6: audioreach: add topology support
flat view
From: Pierre-Louis Bossart <hidden>
Date: 2021-09-21 18:50:04
Also in:
alsa-devel
quoted hunk ↗ jump to hunk
+static struct audioreach_module *audioreach_tplg_alloc_module(struct q6apm *apm, + struct audioreach_container *cont, + struct snd_soc_dapm_widget *w, + uint32_t module_id, bool *found) +{ + struct audioreach_module *mod; + int ret; + + mutex_lock(&apm->lock); + mod = idr_find(&apm->modules_idr, module_id); + mutex_unlock(&apm->lock); + + if (mod) { + *found = true; + return mod; + } + *found = false; + mod = kzalloc(sizeof(*mod), GFP_KERNEL); + if (!mod) + return ERR_PTR(-ENOMEM); + + mutex_lock(&apm->lock); + if (!module_id) { /* alloc module id dynamically */ + ret = idr_alloc_cyclic(&apm->modules_idr, mod, + AR_MODULE_DYNAMIC_INSTANCE_ID_START, + AR_MODULE_DYNAMIC_INSTANCE_ID_END, GFP_KERNEL); + } else { + ret = idr_alloc(&apm->modules_idr, mod, module_id, module_id + 1, GFP_KERNEL); + } + mutex_unlock(&apm->lock); + + if (ret < 0) { + dev_err(apm->dev, + "Failed to allocate Module Instance ID (%x)\n", module_id); + kfree(mod); + return ERR_PTR(ret); + } + + mod->instance_id = ret; + dev_err(apm->dev, "Module Instance ID (0x%08x) allocated\n", ret);
dev_dbg?
quoted hunk ↗ jump to hunk
+ /* add to module list */ + list_add_tail(&mod->node, &cont->modules_list); + mod->container = cont; + mod->widget = w; + cont->num_modules++; + + return mod; +}
quoted hunk ↗ jump to hunk
+static struct audioreach_sub_graph *audioreach_parse_sg_tokens(struct q6apm *apm, + struct snd_soc_tplg_private *private) +{ + struct snd_soc_tplg_vendor_value_elem *sg_elem; + struct snd_soc_tplg_vendor_array *sg_array; + struct audioreach_graph_info *info = NULL; + int graph_id, sub_graph_id, tkn_count = 0; + struct audioreach_sub_graph *sg; + bool found; + + sg_array = audioreach_get_sg_array(private); + sg_elem = sg_array->value; + + while (tkn_count <= (le32_to_cpu(sg_array->num_elems) - 1)) { + switch (le32_to_cpu(sg_elem->token)) { + case AR_TKN_U32_SUB_GRAPH_INSTANCE_ID: + sub_graph_id = le32_to_cpu(sg_elem->value); + sg = audioreach_tplg_alloc_sub_graph(apm, sub_graph_id, &found); + if (IS_ERR(sg)) { + return sg; + } else if (found) { + /* Already parsed data for this sub-graph */ + return sg; + } + dev_err(apm->dev, "%s: New subgraph id : 0x%08x\n", __func__, + sub_graph_id);
dev_dbg()
quoted hunk ↗ jump to hunk
+ break; + case AR_TKN_DAI_INDEX: + /* Sub graph is associated with predefined graph */ + graph_id = le32_to_cpu(sg_elem->value); + info = audioreach_tplg_alloc_graph_info(apm, graph_id, &found); + if (IS_ERR(info)) + return ERR_CAST(info); + break; + case AR_TKN_U32_SUB_GRAPH_PERF_MODE: + sg->perf_mode = le32_to_cpu(sg_elem->value); + break; + case AR_TKN_U32_SUB_GRAPH_DIRECTION: + sg->direction = le32_to_cpu(sg_elem->value); + break; + case AR_TKN_U32_SUB_GRAPH_SCENARIO_ID: + sg->scenario_id = le32_to_cpu(sg_elem->value); + break; + default: + dev_err(apm->dev, "Not a valid token %d for graph\n", + sg_elem->token); + break; + + } + tkn_count++; + sg_elem++; + } + + /* Sub graph is associated with predefined graph */ + if (info) { + dev_err(apm->dev, "%s: adding subgraph id : 0x%08x -> %d\n", __func__, + sub_graph_id, graph_id);
dev_dbg
quoted hunk ↗ jump to hunk
+ + audioreach_tplg_add_sub_graph(sg, info); + } + + return sg; +} +
quoted hunk ↗ jump to hunk
+static int audioreach_widget_ready(struct snd_soc_component *component, + int index, struct snd_soc_dapm_widget *w, + struct snd_soc_tplg_dapm_widget *tplg_w) +{ + switch (w->id) { + case snd_soc_dapm_aif_in: + case snd_soc_dapm_aif_out: + audioreach_widget_load_buffer(component, index, w, tplg_w); + break; + case snd_soc_dapm_decoder: + case snd_soc_dapm_encoder: + case snd_soc_dapm_src: + audioreach_widget_load_enc_dec_cnv(component, index, w, tplg_w); + break; + case snd_soc_dapm_buffer: + audioreach_widget_load_buffer(component, index, w, tplg_w); + break; + case snd_soc_dapm_mixer: + return audioreach_widget_load_mixer(component, index, w, tplg_w); + case snd_soc_dapm_pga: + return audioreach_widget_load_pga(component, index, w, tplg_w); + case snd_soc_dapm_dai_link: + case snd_soc_dapm_scheduler: + case snd_soc_dapm_out_drv: + default: + dev_err(component->dev, "Widget type (0x%x) not yet supported\n", w->id); + break; + } + + return 0; +} + +
spurious newline
quoted hunk ↗ jump to hunk
+static int audioreach_widget_unload(struct snd_soc_component *scomp, + struct snd_soc_dobj *dobj) +{ + struct snd_soc_dapm_widget *w = container_of(dobj, struct snd_soc_dapm_widget, dobj); + struct q6apm *apm = dev_get_drvdata(scomp->dev); + struct audioreach_container *cont; + struct audioreach_module *mod; + + mod = dobj->private; + cont = mod->container; + + if (w->id == snd_soc_dapm_mixer) { + /* virtual widget */ + kfree(dobj->private); + return 0; + } + + mutex_lock(&apm->lock); + idr_remove(&apm->modules_idr, mod->instance_id); + cont->num_modules--; + + list_del(&mod->node); + kfree(mod); + + if (list_empty(&cont->modules_list)) { /* remove container */ + struct audioreach_sub_graph *sg = cont->sub_graph; + + idr_remove(&apm->containers_idr, cont->container_id); + list_del(&cont->node); + sg->num_containers--; + kfree(cont); + if (list_empty(&sg->container_list)) { /* remove sg */ + struct audioreach_graph_info *info = sg->info; + + idr_remove(&apm->sub_graphs_idr, sg->sub_graph_id); + list_del(&sg->node); + info->num_sub_graphs--; + kfree(sg); + if (list_empty(&info->sg_list)) { /* remove graph info */ + idr_remove(&apm->graph_info_idr, info->id); + kfree(info); + } + } + }
It's not very clear if the nested removes actually free-up everything? You may want to add a comment on the hierarchy.
quoted hunk ↗ jump to hunk
+ + mutex_unlock(&apm->lock); + + return 0; +} +
quoted hunk ↗ jump to hunk
+int audioreach_tplg_init(struct snd_soc_component *component) +{ + struct snd_soc_card *card = component->card; + struct device *dev = component->dev; + const struct firmware *fw; + char tplg_fw_name[128]; + int ret; + + /* Inline with Qualcomm UCM configs and linux-firmware path */ + snprintf(tplg_fw_name, sizeof(tplg_fw_name), "qcom/%s/%s-tplg.bin", card->driver_name, + card->name);
use kasprintf instead of fixed 128-char array? Also you should use a qcom/audioreach/ prefix to possible interference with other parts of qcom...
quoted hunk ↗ jump to hunk
+ + ret = request_firmware(&fw, tplg_fw_name, dev); + if (ret < 0) { + dev_info(dev, "loading %s failed %d, falling back to dfw-audioreach-tplg.bin\n", + tplg_fw_name, ret); + /* default firmware */ + ret = request_firmware(&fw, "dfw-audioreach-tplg.bin", dev); + if (ret < 0) { + dev_err(dev, "tplg fw dfw-audioreach-tplg.bin load failed with %d\n", ret);
the dfw prefix isn't very helpful...Intel's example of "dfw_sst.bin" is a historical reference, not something you should reuse.
quoted hunk ↗ jump to hunk
+ return ret; + } + } + + ret = snd_soc_tplg_component_load(component, &audioreach_tplg_ops, fw); + if (ret < 0) { + dev_err(dev, "tplg component load failed%d\n", ret); + ret = -EINVAL; + } + + release_firmware(fw); + return ret; +} +EXPORT_SYMBOL_GPL(audioreach_tplg_init);