Re: [PATCH v5 17/21] ASoC: qdsp6: audioreach: add topology support
flat view
From: Pierre-Louis Bossart <hidden>
Date: 2021-09-03 16:38:17
Also in:
alsa-devel
quoted hunk ↗ jump to hunk
+/** + * %AR_TKN_U32_SUB_GRAPH_INSTANCE_ID: Sub Graph Instance Id + * + * %AR_TKN_U32_SUB_GRAPH_PERF_MODE: Performance mode of subgraph + * APM_SUB_GRAPH_PERF_MODE_LOW_POWER = 1, + * APM_SUB_GRAPH_PERF_MODE_LOW_LATENCY = 2 + * + * %AR_TKN_U32_SUB_GRAPH_DIRECTION: Direction of subgraph + * APM_SUB_GRAPH_DIRECTION_TX = 1, + * APM_SUB_GRAPH_DIRECTION_RX = 2
can you have full-duplex? unclear how you would define the direction in the case of a voice call...
quoted hunk ↗ jump to hunk
+ * + * %AR_TKN_U32_SUB_GRAPH_SCENARIO_ID: Scenario ID for subgraph + * APM_SUB_GRAPH_SID_AUDIO_PLAYBACK = 1, + * APM_SUB_GRAPH_SID_AUDIO_RECORD = 2, + * APM_SUB_GRAPH_SID_VOICE_CALL = 3 + * + * %AR_TKN_U32_CONTAINER_INSTANCE_ID: Container Instance ID + * + * %AR_TKN_U32_CONTAINER_CAPABILITY_ID: Container capability ID + * APM_CONTAINER_CAP_ID_PP = 1, + * APM_CONTAINER_CAP_ID_CD = 2, + * APM_CONTAINER_CAP_ID_EP = 3, + * APM_CONTAINER_CAP_ID_OLC = 4
Acronyms? PP, CD, EP, OLC?
quoted hunk ↗ jump to hunk
+ * + * %AR_TKN_U32_CONTAINER_STACK_SIZE: Stack size in the container. + * + * %AR_TKN_U32_CONTAINER_GRAPH_POS: Graph Position + * APM_CONT_GRAPH_POS_STREAM = 1, + * APM_CONT_GRAPH_POS_PER_STR_PER_DEV = 2, + * APM_CONT_GRAPH_POS_STR_DEV = 3, + * APM_CONT_GRAPH_POS_GLOBAL_DEV = 4
explain what this means?
quoted hunk ↗ jump to hunk
+ * + * %AR_TKN_U32_CONTAINER_PROC_DOMAIN: Processor domain of container + * APM_PROC_DOMAIN_ID_MDSP = 1, + * APM_PROC_DOMAIN_ID_ADSP = 2, + * APM_PROC_DOMAIN_ID_SDSP = 4, + * APM_PROC_DOMAIN_ID_CDSP = 5
what happened to 3, is it reserved/illegal?
quoted hunk ↗ jump to hunk
+ * + * %AR_TKN_U32_MODULE_ID: Module ID + * + * %AR_TKN_U32_MODULE_INSTANCE_ID: Module Instance ID. + * + * %AR_TKN_U32_MODULE_MAX_IP_PORTS: Module maximum input ports + * + * %AR_TKN_U32_MODULE_MAX_OP_PORTS: Module maximum output ports. + * + * %AR_TKN_U32_MODULE_IN_PORTS: Number of in ports + * + * %AR_TKN_U32_MODULE_OUT_PORTS: Number of out ports. + * + * %AR_TKN_U32_MODULE_SRC_OP_PORT_ID: Source module output port ID + * + * %AR_TKN_U32_MODULE_DST_IN_PORT_ID: Destination module input port ID + * + * %AR_TKN_U32_MODULE_HW_IF_IDX: Interface index types for I2S/LPAIF + * + * %AR_TKN_U32_MODULE_HW_IF_TYPE: Interface type + * LPAIF = 0, + * LPAIF_RXTX = 1, + * LPAIF_WSA = 2, + * LPAIF_VA = 3, + * LPAIF_AXI = 4 + * + * %AR_TKN_U32_MODULE_FMT_INTERLEAVE: PCM Interleaving + * PCM_INTERLEAVED = 1, + * PCM_DEINTERLEAVED_PACKED = 2, + * PCM_DEINTERLEAVED_UNPACKED = 3 + * + * %AR_TKN_U32_MODULE_FMT_DATA: data format + * FIXED POINT = 1, + * IEC60958 PACKETIZED = 3, + * IEC60958 PACKETIZED NON LINEAR = 8, + * COMPR OVER PCM PACKETIZED = 7, + * IEC61937 PACKETIZED = 2,
isn't this precisely compressed over PCM?
+ * GENERIC COMPRESSED = 5
??
+ * + * %AR_TKN_U32_MODULE_FMT_FREQ: bit rate
bit rate or frame rate (aka sampling frequency) ?
+ * + * %AR_TKN_U32_MODULE_FMT_BIT_DEPTH: bit depth
quoted hunk ↗ jump to hunk
+static struct audioreach_sub_graph *audioreach_parse_sg_tokens(struct q6apm *apm, + struct snd_soc_tplg_private *private) +{ + struct audioreach_graph_info *info = NULL; + struct snd_soc_tplg_vendor_array *sg_array; + struct snd_soc_tplg_vendor_value_elem *sg_elem; + struct audioreach_sub_graph *sg; + int graph_id, sub_graph_id, tkn_count = 0; + 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); + 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;
indentation is off
quoted hunk ↗ jump to hunk
+ + } + 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); + + audioreach_tplg_add_sub_graph(sg, info); + } + + return sg; +} + +static struct audioreach_container *audioreach_parse_cont_tokens(struct q6apm *apm, + struct audioreach_sub_graph *sg, + struct snd_soc_tplg_private *private) +{ + struct snd_soc_tplg_vendor_array *cont_array; + struct snd_soc_tplg_vendor_value_elem *cont_elem; + struct audioreach_container *cont; + int container_id, tkn_count = 0; + bool found = false; + + cont_array = audioreach_get_cont_array(private); + cont_elem = cont_array->value; + + while (tkn_count <= (le32_to_cpu(cont_array->num_elems) - 1)) { + switch (le32_to_cpu(cont_elem->token)) { + case AR_TKN_U32_CONTAINER_INSTANCE_ID: + container_id = le32_to_cpu(cont_elem->value); + cont = audioreach_tplg_alloc_container(apm, sg, container_id, &found); + if (IS_ERR(cont)) + return ERR_PTR(-ENOMEM); + else if (found) /* Already parsed container data */ + return cont; + + break; + case AR_TKN_U32_CONTAINER_CAPABILITY_ID: + cont->capability_id = le32_to_cpu(cont_elem->value); + break; + case AR_TKN_U32_CONTAINER_STACK_SIZE: + cont->stack_size = le32_to_cpu(cont_elem->value); + break; + case AR_TKN_U32_CONTAINER_GRAPH_POS: + cont->graph_pos = le32_to_cpu(cont_elem->value); + break; + case AR_TKN_U32_CONTAINER_PROC_DOMAIN: + cont->proc_domain = le32_to_cpu(cont_elem->value); + break; + default: + dev_err(apm->dev, "Not a valid token %d for graph\n", + cont_elem->token); + break;
indentation?
quoted hunk ↗ jump to hunk
+ + } + tkn_count++; + cont_elem++; + } + + return cont; +} +
quoted hunk ↗ jump to hunk
+static struct audioreach_module *audioreach_find_widget(struct snd_soc_component *comp, + const char *name) +{ + struct q6apm *apm = dev_get_drvdata(comp->dev); + struct audioreach_module *module; + int id = 0;
unnecessary init?
quoted hunk ↗ jump to hunk
+ + idr_for_each_entry(&apm->modules_idr, module, id) { + if (!strcmp(name, module->widget->name)) + return module; + } + + return NULL; +}
quoted hunk ↗ jump to hunk
+/* DAI link - used for any driver specific init */ +static int audioreach_link_load(struct snd_soc_component *component, int index, + struct snd_soc_dai_link *link, + struct snd_soc_tplg_link_config *cfg) +{ + link->nonatomic = true; + link->dynamic = true; + link->platforms->name = NULL; + link->platforms->of_node = of_get_compatible_child(component->dev->of_node, + "qcom,q6apm-dais"); + link->trigger[0] = SND_SOC_DPCM_TRIGGER_POST; + link->trigger[1] = SND_SOC_DPCM_TRIGGER_POST;
can you add a comment on why you don't use the default order for FE/BE triggers?
quoted hunk ↗ jump to hunk
+ + return 0; +} +
quoted hunk ↗ jump to hunk
+static int audioreach_get_vol_ctrl_audio_mixer(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_dapm_widget *dw = snd_soc_dapm_kcontrol_widget(kcontrol); + struct audioreach_module *mod = dw->dobj.private; + + /* Check if the graph is active or not */
that comment seems like a copy/paste, there's no check.
quoted hunk ↗ jump to hunk
+ ucontrol->value.integer.value[0] = mod->gain; + + return 0; +} + +static int audioreach_put_vol_ctrl_audio_mixer(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_dapm_widget *dw = snd_soc_dapm_kcontrol_widget(kcontrol); + struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kcontrol); + struct snd_soc_component *c = snd_soc_dapm_to_component(dapm); + struct audioreach_module *mod = dw->dobj.private; + struct q6apm *apm = dev_get_drvdata(c->dev); + int vol = ucontrol->value.integer.value[0]; + + /* Check if the graph is active or not */ + if (dw->power) { + audioreach_gain_set_vol_ctrl(apm, mod, vol); + mod->gain = vol; + return 1; + }
shouldn't you cache the value and apply it when the graph is powered? Also wondering why this isn't using pm_runtime or something? Is this re-inventing your own power management?
quoted hunk ↗ jump to hunk
+ + dev_err(apm->dev, "Unable to set volume as graph is not active\n"); + return 0; + +} + +static int audioreach_control_load_mix(struct snd_soc_component *scomp, + struct snd_ar_control *scontrol, + struct snd_kcontrol_new *kc, + struct snd_soc_tplg_ctl_hdr *hdr) +{ + struct snd_soc_tplg_mixer_control *mc; + struct snd_soc_tplg_vendor_array *c_array; + struct snd_soc_tplg_vendor_value_elem *c_elem; + int tkn_count = 0; + + mc = container_of(hdr, struct snd_soc_tplg_mixer_control, hdr); + c_array = (struct snd_soc_tplg_vendor_array *)mc->priv.data; + + c_elem = c_array->value; + + while (tkn_count <= (le32_to_cpu(c_array->num_elems) - 1)) { + switch (le32_to_cpu(c_elem->token)) { + case AR_TKN_U32_SUB_GRAPH_INSTANCE_ID: + scontrol->sgid = le32_to_cpu(c_elem->value); + break; + default: + /* Ignore other tokens */ + break;
indentation?
quoted hunk ↗ jump to hunk
+ + } + c_elem++; + tkn_count++; + } + + return 0; +}
quoted hunk ↗ jump to hunk
+int audioreach_tplg_init(struct snd_soc_component *component) +{ + struct device *dev = component->dev; + const struct firmware *fw; + int ret; + + ret = request_firmware(&fw, "audioreach.bin", dev); + if (ret < 0) { + dev_err(dev, "tplg fw audioreach.bin load failed with %d\n", ret); + 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); + release_firmware(fw); + return -EINVAL; + }
should you not release the firmware on success as well? that's what we
do for SOF"
ret = snd_soc_tplg_component_load(scomp, &sof_tplg_ops, fw);
if (ret < 0) {
dev_err(scomp->dev, "error: tplg component load failed %d\n",
ret);
ret = -EINVAL;
}
release_firmware(fw);
return ret;
quoted hunk ↗ jump to hunk
+ + return 0; +} +EXPORT_SYMBOL_GPL(audioreach_tplg_init);