[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e7c3ea4a-1103-46ff-aa9a-bc0da3bdf1b2@linaro.org>
Date: Thu, 27 Mar 2025 15:53:18 +0000
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Alexey Klimov <alexey.klimov@...aro.org>, broonie@...nel.org,
linux-sound@...r.kernel.org
Cc: lgirdwood@...il.com, perex@...ex.cz, tiwai@...e.com,
krzysztof.kozlowski@...aro.org, pierre-louis.bossart@...ux.dev,
vkoul@...nel.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, stable@...r.kernel.org,
dmitry.baryshkov@....qualcomm.com
Subject: Re: [PATCH] ASoC: qdsp6: q6asm-dai: fix q6asm_dai_compr_set_params
error path
On 27/03/2025 15:46, Alexey Klimov wrote:
> In case of attempts to compress playback something, for instance,
> when audio routing is not set up correctly, the audio DSP is left in
> inconsistent state because we are not doing the correct things in
> the error path of q6asm_dai_compr_set_params().
>
> So, when routing is not set up and compress playback is attempted
> the following errors are present (simplified log):
>
> q6routing routing: Routing not setup for MultiMedia-1 Session
> q6asm-dai dais: Stream reg failed ret:-22
> q6asm-dai dais: ASoC error (-22): at snd_soc_component_compr_set_params()
> on 17300000.remoteproc:glink-edge:apr:service@7:dais
>
> After setting the correct routing the compress playback will always fail:
>
> q6asm-dai dais: cmd = 0x10db3 returned error = 0x9
> q6asm-dai dais: DSP returned error[9]
> q6asm-dai dais: q6asm_open_write failed
> q6asm-dai dais: ASoC error (-22): at snd_soc_component_compr_set_params()
> on 17300000.remoteproc:glink-edge:apr:service@7:dais
>
> 0x9 here means "Operation is already processed". The CMD_OPEN here was
> sent the second time hence DSP responds that it was already done.
>
> Turns out the CMD_CLOSE should be sent after the q6asm_open_write()
> succeeded but something failed after that, for instance, routing
> setup.
>
> Fix this by slightly reworking the error path in
> q6asm_dai_compr_set_params().
>
> Tested on QRB5165 RB5 and SDM845 RB3 boards.
>
> Cc: stable@...r.kernel.org
> Fixes: 5b39363e54cc ("ASoC: q6asm-dai: prepare set params to accept profile change")
> Cc: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> Cc: Vinod Koul <vkoul@...nel.org>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> Signed-off-by: Alexey Klimov <alexey.klimov@...aro.org>
> ---
thanks for the patch,
This is now pretty much inline with what we have in non-compressed patch.
LGTM.
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
--srini
> 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 045100c94352..a400c9a31fea 100644
> --- a/sound/soc/qcom/qdsp6/q6asm-dai.c
> +++ b/sound/soc/qcom/qdsp6/q6asm-dai.c
> @@ -892,9 +892,7 @@ static int q6asm_dai_compr_set_params(struct snd_soc_component *component,
>
> if (ret < 0) {
> dev_err(dev, "q6asm_open_write failed\n");
> - q6asm_audio_client_free(prtd->audio_client);
> - prtd->audio_client = NULL;
> - return ret;
> + goto open_err;
> }
> }
>
> @@ -903,7 +901,7 @@ static int q6asm_dai_compr_set_params(struct snd_soc_component *component,
> prtd->session_id, dir);
> if (ret) {
> dev_err(dev, "Stream reg failed ret:%d\n", ret);
> - return ret;
> + goto q6_err;
> }
>
> ret = __q6asm_dai_compr_set_codec_params(component, stream,
> @@ -911,7 +909,7 @@ static int q6asm_dai_compr_set_params(struct snd_soc_component *component,
> prtd->stream_id);
> if (ret) {
> dev_err(dev, "codec param setup failed ret:%d\n", ret);
> - return ret;
> + goto q6_err;
> }
>
> ret = q6asm_map_memory_regions(dir, prtd->audio_client, prtd->phys,
> @@ -920,12 +918,21 @@ static int q6asm_dai_compr_set_params(struct snd_soc_component *component,
>
> if (ret < 0) {
> dev_err(dev, "Buffer Mapping failed ret:%d\n", ret);
> - return -ENOMEM;
> + ret = -ENOMEM;
> + goto q6_err;
> }
>
> prtd->state = Q6ASM_STREAM_RUNNING;
>
> return 0;
> +
> +q6_err:
> + q6asm_cmd(prtd->audio_client, prtd->stream_id, CMD_CLOSE);
> +
> +open_err:
> + q6asm_audio_client_free(prtd->audio_client);
> + prtd->audio_client = NULL;
> + return ret;
> }
>
> static int q6asm_dai_compr_set_metadata(struct snd_soc_component *component,
Powered by blists - more mailing lists