[PATCH 0/5] ASoC: qdsp6: fixes and improve error reporting
Hi Mark,
During testing on a laptop we found few issues with the existing q6dsp code, first 3 patches should fix these issues. Also during debug we found that some of the errors reported are not very useful, so update error reporting in two places.
Thanks, srini
Srinivas Kandagatla (5): ASoC: qdsp6: qdsp6: q6prm: handle clk disable correctly ASoC: qdsp6: q6routing: Conditionally reset FrontEnd Mixer ASoC: qdsp6: q6asm: fix q6asm_dai_prepare error handling ASoC: qdsp6: q6adm: improve error reporting ASoC: qdsp6: q6routing: validate port id before setting up route
sound/soc/qcom/qdsp6/audioreach.h | 4 +++ sound/soc/qcom/qdsp6/q6adm.c | 4 +-- sound/soc/qcom/qdsp6/q6asm-dai.c | 19 +++++++---- sound/soc/qcom/qdsp6/q6prm.c | 53 +++++++++++++++++++++++++++++-- sound/soc/qcom/qdsp6/q6routing.c | 12 ++++++- 5 files changed, 81 insertions(+), 11 deletions(-)
Q6PRM clks need to be disabled using PRM_CMD_RELEASE_HW_RSC dsp command rather then using PRM_CMD_RSP_REQUEST_HW_RSC cmd with rate set to zero.
DSP will throw errors if we try to disable the clock using existing code.
Fix this by properly handling the clk release.
Fixes: 9a0e5d6fb16f ("ASoC: qdsp6: audioreach: add q6prm support") Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- sound/soc/qcom/qdsp6/audioreach.h | 4 +++ sound/soc/qcom/qdsp6/q6prm.c | 53 +++++++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h index 4f693a2660b5..3ee8bfcd0121 100644 --- a/sound/soc/qcom/qdsp6/audioreach.h +++ b/sound/soc/qcom/qdsp6/audioreach.h @@ -550,6 +550,10 @@ struct audio_hw_clk_cfg { uint32_t clock_root; } __packed;
+struct audio_hw_clk_rel_cfg { + uint32_t clock_id; +} __packed; + #define PARAM_ID_HW_EP_POWER_MODE_CFG 0x8001176 #define AR_HW_EP_POWER_MODE_0 0 /* default */ #define AR_HW_EP_POWER_MODE_1 1 /* XO Shutdown allowed */ diff --git a/sound/soc/qcom/qdsp6/q6prm.c b/sound/soc/qcom/qdsp6/q6prm.c index 82c40f2d4e1d..cda33ded29be 100644 --- a/sound/soc/qcom/qdsp6/q6prm.c +++ b/sound/soc/qcom/qdsp6/q6prm.c @@ -42,6 +42,12 @@ struct prm_cmd_request_rsc { struct audio_hw_clk_cfg clock_id; } __packed;
+struct prm_cmd_release_rsc { + struct apm_module_param_data param_data; + uint32_t num_clk_id; + struct audio_hw_clk_rel_cfg clock_id; +} __packed; + static int q6prm_send_cmd_sync(struct q6prm *prm, struct gpr_pkt *pkt, uint32_t rsp_opcode) { return audioreach_send_cmd_sync(prm->dev, prm->gdev, &prm->result, &prm->lock, @@ -102,8 +108,8 @@ int q6prm_unvote_lpass_core_hw(struct device *dev, uint32_t hw_block_id, uint32_ } EXPORT_SYMBOL_GPL(q6prm_unvote_lpass_core_hw);
-int q6prm_set_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root, - unsigned int freq) +static int q6prm_request_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root, + unsigned int freq) { struct q6prm *prm = dev_get_drvdata(dev->parent); struct apm_module_param_data *param_data; @@ -138,6 +144,49 @@ int q6prm_set_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_
return rc; } + +static int q6prm_release_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root, + unsigned int freq) +{ + struct q6prm *prm = dev_get_drvdata(dev->parent); + struct apm_module_param_data *param_data; + struct prm_cmd_release_rsc *rel; + gpr_device_t *gdev = prm->gdev; + struct gpr_pkt *pkt; + int rc; + + pkt = audioreach_alloc_cmd_pkt(sizeof(*rel), PRM_CMD_RELEASE_HW_RSC, 0, gdev->svc.id, + GPR_PRM_MODULE_IID); + if (IS_ERR(pkt)) + return PTR_ERR(pkt); + + rel = (void *)pkt + GPR_HDR_SIZE + APM_CMD_HDR_SIZE; + + param_data = &rel->param_data; + + param_data->module_instance_id = GPR_PRM_MODULE_IID; + param_data->error_code = 0; + param_data->param_id = PARAM_ID_RSC_AUDIO_HW_CLK; + param_data->param_size = sizeof(*rel) - APM_MODULE_PARAM_DATA_SIZE; + + rel->num_clk_id = 1; + rel->clock_id.clock_id = clk_id; + + rc = q6prm_send_cmd_sync(prm, pkt, PRM_CMD_RSP_RELEASE_HW_RSC); + + kfree(pkt); + + return rc; +} + +int q6prm_set_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root, + unsigned int freq) +{ + if (freq) + return q6prm_request_lpass_clock(dev, clk_id, clk_attr, clk_attr, freq); + + return q6prm_release_lpass_clock(dev, clk_id, clk_attr, clk_attr, freq); +} EXPORT_SYMBOL_GPL(q6prm_set_lpass_clock);
static int prm_callback(struct gpr_resp_pkt *data, void *priv, int op)
Stream IDs are reused across multiple BackEnd mixers, do not reset the stream mixers if they are not already set for that particular FrontEnd.
Ex: amixer cset iface=MIXER,name='SLIMBUS_0_RX Audio Mixer MultiMedia1' 1
would set the MultiMedia1 steam for SLIMBUS_0_RX, however doing below command will reset previously setup MultiMedia1 stream, because both of them are using MultiMedia1 PCM stream.
amixer cset iface=MIXER,name='SLIMBUS_2_RX Audio Mixer MultiMedia1' 0
reset the FrontEnd Mixers conditionally to fix this issue.
This is more noticeable in desktop setup, where in alsactl tries to restore the alsa state and overwriting the previous mixer settings.
Fixes: e3a33673e845 ("ASoC: qdsp6: q6routing: Add q6routing driver") Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- sound/soc/qcom/qdsp6/q6routing.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/sound/soc/qcom/qdsp6/q6routing.c b/sound/soc/qcom/qdsp6/q6routing.c index 3390ebef9549..243b8179e59d 100644 --- a/sound/soc/qcom/qdsp6/q6routing.c +++ b/sound/soc/qcom/qdsp6/q6routing.c @@ -495,7 +495,11 @@ static int msm_routing_put_audio_mixer(struct snd_kcontrol *kcontrol, session->port_id = be_id; snd_soc_dapm_mixer_update_power(dapm, kcontrol, 1, update); } else { - session->port_id = -1; + if (session->port_id == be_id) { + session->port_id = -1; + return 0; + } + snd_soc_dapm_mixer_update_power(dapm, kcontrol, 0, update); }
Error handling in q6asm_dai_prepare() seems to be completely broken, Fix this by handling it properly.
Fixes: 2a9e92d371db ("ASoC: qdsp6: q6asm: Add q6asm dai driver") Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- sound/soc/qcom/qdsp6/q6asm-dai.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c index 46f365528d50..b74b67720ef4 100644 --- a/sound/soc/qcom/qdsp6/q6asm-dai.c +++ b/sound/soc/qcom/qdsp6/q6asm-dai.c @@ -269,9 +269,7 @@ static int q6asm_dai_prepare(struct snd_soc_component *component,
if (ret < 0) { dev_err(dev, "%s: q6asm_open_write failed\n", __func__); - q6asm_audio_client_free(prtd->audio_client); - prtd->audio_client = NULL; - return -ENOMEM; + goto open_err; }
prtd->session_id = q6asm_get_session_id(prtd->audio_client); @@ -279,7 +277,7 @@ static int q6asm_dai_prepare(struct snd_soc_component *component, prtd->session_id, substream->stream); if (ret) { dev_err(dev, "%s: stream reg failed ret:%d\n", __func__, ret); - return ret; + goto routing_err; }
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { @@ -301,10 +299,19 @@ static int q6asm_dai_prepare(struct snd_soc_component *component, } if (ret < 0) dev_info(dev, "%s: CMD Format block failed\n", __func__); + else + prtd->state = Q6ASM_STREAM_RUNNING;
- prtd->state = Q6ASM_STREAM_RUNNING; + return ret;
- return 0; +routing_err: + q6asm_cmd(prtd->audio_client, prtd->stream_id, CMD_CLOSE); +open_err: + q6asm_unmap_memory_regions(substream->stream, prtd->audio_client); + q6asm_audio_client_free(prtd->audio_client); + prtd->audio_client = NULL; + + return ret; }
static int q6asm_dai_trigger(struct snd_soc_component *component,
reset value for port is -1 so printing an hex would not give us very useful debug information, so use %d instead.
Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- sound/soc/qcom/qdsp6/q6adm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6adm.c b/sound/soc/qcom/qdsp6/q6adm.c index 3d831b635524..72c5719f1d25 100644 --- a/sound/soc/qcom/qdsp6/q6adm.c +++ b/sound/soc/qcom/qdsp6/q6adm.c @@ -390,7 +390,7 @@ struct q6copp *q6adm_open(struct device *dev, int port_id, int path, int rate, int ret = 0;
if (port_id < 0) { - dev_err(dev, "Invalid port_id 0x%x\n", port_id); + dev_err(dev, "Invalid port_id %d\n", port_id); return ERR_PTR(-EINVAL); }
@@ -508,7 +508,7 @@ int q6adm_matrix_map(struct device *dev, int path, int port_idx = payload_map.port_id[i];
if (port_idx < 0) { - dev_err(dev, "Invalid port_id 0x%x\n", + dev_err(dev, "Invalid port_id %d\n", payload_map.port_id[i]); kfree(pkt); return -EINVAL;
Validate port id before it starts sending commands to dsp this would make error handling simpler.
Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- sound/soc/qcom/qdsp6/q6routing.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/sound/soc/qcom/qdsp6/q6routing.c b/sound/soc/qcom/qdsp6/q6routing.c index 243b8179e59d..cd74681e811e 100644 --- a/sound/soc/qcom/qdsp6/q6routing.c +++ b/sound/soc/qcom/qdsp6/q6routing.c @@ -372,6 +372,12 @@ int q6routing_stream_open(int fedai_id, int perf_mode, }
session = &routing_data->sessions[stream_id - 1]; + if (session->port_id < 0) { + dev_err(routing_data->dev, "Routing not setup for MultiMedia%d Session\n", + session->fedai_id); + return -EINVAL; + } + pdata = &routing_data->port_data[session->port_id];
mutex_lock(&routing_data->lock);
On Tue, 16 Nov 2021 11:47:16 +0000, Srinivas Kandagatla wrote:
During testing on a laptop we found few issues with the existing q6dsp code, first 3 patches should fix these issues. Also during debug we found that some of the errors reported are not very useful, so update error reporting in two places.
Thanks, srini
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-linus
Thanks!
[1/5] ASoC: qdsp6: qdsp6: q6prm: handle clk disable correctly commit: 2f20640491edda3c03eb6b899d0b92630d3d4c63 [2/5] ASoC: qdsp6: q6routing: Conditionally reset FrontEnd Mixer commit: 861afeac7990587588d057b2c0b3222331c3da29 [3/5] ASoC: qdsp6: q6asm: fix q6asm_dai_prepare error handling commit: 721a94b4352dc8e47bff90b549a0118c39776756 [4/5] ASoC: qdsp6: q6adm: improve error reporting commit: 0a270471d68533f59c5cfd631a3fce31a3b17144 [5/5] ASoC: qdsp6: q6routing: validate port id before setting up route commit: 6712c2e18c06b0976559fd4bd47774b243038e9c
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Mark Brown -
Srinivas Kandagatla