2023-02-09 12:29:14

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 0/8] ASoC: qcom: q6dsp and lpass codec stablity fixes

While testing X13s audio, we found multiple stablity issues this patchset
fixes these issues.
From q6dsp side issues are around locking of position pointer and handle
multiple prepare cases along with pulse audio timerbased scheduling workaround.

From LPASS codec side most of the staiblity issues were around runtime pm,
hitting various issues as the codec was firstly resetting the soundwire block
for every clk disable/enable which is taking the slaves out of sync and
resulting in re-enumerating. Second issue was around fsgen clk is not
brining up the codec out of suspend as it was not added after
runtime pm enabled. Final issue was with codec mclk rate which should
have been 192KHz same as npl instead of 96KHz. We were getting lucky as
wsa drivers are setting the same clk to 192KHz.

With this patches, x13s audio is pretty stable.

thanks,
Srini


Srinivas Kandagatla (8):
ASoC: qcom: q6apm-lpass-dai: unprepare stream if its already prepared
ASoC: qcom: q6apm-dai: fix race condition while updating the position
pointer
ASoC: qcom: q6apm-dai: Add SNDRV_PCM_INFO_BATCH flag
ASoC: qcom: audioreach: fix ADSP ready check
ASoC: codecs: lpass: register mclk after runtime pm
ASoC: codecs: lpass: fix incorrect mclk rate
ASoC: codecs: lpass: do not reset soundwire block on clk enable
ASoC: codecs: lpass: remove not so useful verbose log

sound/soc/codecs/lpass-rx-macro.c | 31 ++++++++++--------
sound/soc/codecs/lpass-tx-macro.c | 25 ++++++++------
sound/soc/codecs/lpass-va-macro.c | 43 +++++++++++++------------
sound/soc/codecs/lpass-wsa-macro.c | 27 ++++++++--------
sound/soc/qcom/qdsp6/q6apm-dai.c | 22 +++++++++++--
sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 5 +++
sound/soc/qcom/qdsp6/q6apm.c | 17 +++++-----
7 files changed, 101 insertions(+), 69 deletions(-)

--
2.21.0



2023-02-09 12:29:14

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 1/8] ASoC: qcom: q6apm-lpass-dai: unprepare stream if its already prepared

prepare callback can be called multiple times, so unprepare the stream
if its already prepared.

Without this DSP is not happy to setting the params on a already
prepared graph.

Fixes: 9b4fe0f1cd79 ("ASoC: qdsp6: audioreach: add q6apm-dai support")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index ce9e5646d8f3..23d23bc6fbaa 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -127,6 +127,11 @@ static int q6apm_lpass_dai_prepare(struct snd_pcm_substream *substream, struct s
int graph_id = dai->id;
int rc;

+ if (dai_data->is_port_started[dai->id]) {
+ q6apm_graph_stop(dai_data->graph[dai->id]);
+ dai_data->is_port_started[dai->id] = false;
+ }
+
/**
* It is recommend to load DSP with source graph first and then sink
* graph, so sequence for playback and capture will be different
--
2.21.0


2023-02-09 12:29:14

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 2/8] ASoC: qcom: q6apm-dai: fix race condition while updating the position pointer

It is noticed that the position pointer value seems to get a get corrupted
due to missing locking between updating and reading.

Fix this by adding a spinlock around the position pointer.

Fixes: 9b4fe0f1cd79 ("ASoC: qdsp6: audioreach: add q6apm-dai support")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/qcom/qdsp6/q6apm-dai.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index ee59ef36b85a..bd35067a4052 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -8,6 +8,7 @@
#include <linux/slab.h>
#include <sound/soc.h>
#include <sound/soc-dapm.h>
+#include <linux/spinlock.h>
#include <sound/pcm.h>
#include <asm/dma.h>
#include <linux/dma-mapping.h>
@@ -53,6 +54,7 @@ struct q6apm_dai_rtd {
uint16_t session_id;
enum stream_state state;
struct q6apm_graph *graph;
+ spinlock_t lock;
};

struct q6apm_dai_data {
@@ -99,20 +101,25 @@ static void event_handler(uint32_t opcode, uint32_t token, uint32_t *payload, vo
{
struct q6apm_dai_rtd *prtd = priv;
struct snd_pcm_substream *substream = prtd->substream;
+ unsigned long flags;

switch (opcode) {
case APM_CLIENT_EVENT_CMD_EOS_DONE:
prtd->state = Q6APM_STREAM_STOPPED;
break;
case APM_CLIENT_EVENT_DATA_WRITE_DONE:
+ spin_lock_irqsave(&prtd->lock, flags);
prtd->pos += prtd->pcm_count;
+ spin_unlock_irqrestore(&prtd->lock, flags);
snd_pcm_period_elapsed(substream);
if (prtd->state == Q6APM_STREAM_RUNNING)
q6apm_write_async(prtd->graph, prtd->pcm_count, 0, 0, 0);

break;
case APM_CLIENT_EVENT_DATA_READ_DONE:
+ spin_lock_irqsave(&prtd->lock, flags);
prtd->pos += prtd->pcm_count;
+ spin_unlock_irqrestore(&prtd->lock, flags);
snd_pcm_period_elapsed(substream);
if (prtd->state == Q6APM_STREAM_RUNNING)
q6apm_read(prtd->graph);
@@ -253,6 +260,7 @@ static int q6apm_dai_open(struct snd_soc_component *component,
if (prtd == NULL)
return -ENOMEM;

+ spin_lock_init(&prtd->lock);
prtd->substream = substream;
prtd->graph = q6apm_graph_open(dev, (q6apm_cb)event_handler, prtd, graph_id);
if (IS_ERR(prtd->graph)) {
@@ -332,11 +340,17 @@ static snd_pcm_uframes_t q6apm_dai_pointer(struct snd_soc_component *component,
{
struct snd_pcm_runtime *runtime = substream->runtime;
struct q6apm_dai_rtd *prtd = runtime->private_data;
+ snd_pcm_uframes_t ptr;
+ unsigned long flags;

+ spin_lock_irqsave(&prtd->lock, flags);
if (prtd->pos == prtd->pcm_size)
prtd->pos = 0;

- return bytes_to_frames(runtime, prtd->pos);
+ ptr = bytes_to_frames(runtime, prtd->pos);
+ spin_unlock_irqrestore(&prtd->lock, flags);
+
+ return ptr;
}

static int q6apm_dai_hw_params(struct snd_soc_component *component,
--
2.21.0


2023-02-09 12:29:14

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 3/8] ASoC: qcom: q6apm-dai: Add SNDRV_PCM_INFO_BATCH flag

At the moment, playing audio with PulseAudio with the qdsp6 driver
results in distorted sound. It seems like its timer-based scheduling
does not work properly with qdsp6 since setting tsched=0 in
the PulseAudio configuration avoids the issue.

Apparently this happens when the pointer() callback is not accurate
enough. There is a SNDRV_PCM_INFO_BATCH flag that can be used to stop
PulseAudio from using timer-based scheduling by default.

According to https://www.alsa-project.org/pipermail/alsa-devel/2014-March/073816.html:

The flag is being used in the sense explained in the previous audio
meeting -- the data transfer granularity isn't fine enough but aligned
to the period size (or less).

q6apm-dai reports the position as multiple of

prtd->pcm_count = snd_pcm_lib_period_bytes(substream)

so it indeed just a multiple of the period size.

Therefore adding the flag here seems appropriate and makes audio
work out of the box.

Comment log inspired by Stephan Gerhold sent for q6asm-dai.c few years back.

Fixes: 9b4fe0f1cd79 ("ASoC: qdsp6: audioreach: add q6apm-dai support")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/qcom/qdsp6/q6apm-dai.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index bd35067a4052..7f02f5b2c33f 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -64,7 +64,8 @@ struct q6apm_dai_data {
static struct snd_pcm_hardware q6apm_dai_hardware_capture = {
.info = (SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_BLOCK_TRANSFER |
SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_INTERLEAVED |
- SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME),
+ SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME |
+ SNDRV_PCM_INFO_BATCH),
.formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE),
.rates = SNDRV_PCM_RATE_8000_48000,
.rate_min = 8000,
@@ -82,7 +83,8 @@ static struct snd_pcm_hardware q6apm_dai_hardware_capture = {
static struct snd_pcm_hardware q6apm_dai_hardware_playback = {
.info = (SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_BLOCK_TRANSFER |
SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_INTERLEAVED |
- SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME),
+ SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME |
+ SNDRV_PCM_INFO_BATCH),
.formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE),
.rates = SNDRV_PCM_RATE_8000_192000,
.rate_min = 8000,
--
2.21.0


2023-02-09 12:29:14

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 6/8] ASoC: codecs: lpass: fix incorrect mclk rate

For some reason we ended up with incorrect mclk rate which should be
1920000 instead of 96000, So far we were getting lucky as the same clk
is set to 192000 by wsa and va macro. This issue is discovered when there
is no wsa macro active and only rx or tx path is tested.
Fix this by setting correct rate.

Fixes: c39667ddcfc5 ("ASoC: codecs: lpass-tx-macro: add support for lpass tx macro")
Fixes: af3d54b99764 ("ASoC: codecs: lpass-rx-macro: add support for lpass rx macro")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-rx-macro.c | 4 ++--
sound/soc/codecs/lpass-tx-macro.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
index dd6970d5eb8d..8621cfabcf5b 100644
--- a/sound/soc/codecs/lpass-rx-macro.c
+++ b/sound/soc/codecs/lpass-rx-macro.c
@@ -366,7 +366,7 @@
#define CDC_RX_DSD1_CFG2 (0x0F8C)
#define RX_MAX_OFFSET (0x0F8C)

-#define MCLK_FREQ 9600000
+#define MCLK_FREQ 19200000

#define RX_MACRO_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 |\
SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_48000 |\
@@ -3579,7 +3579,7 @@ static int rx_macro_probe(struct platform_device *pdev)

/* set MCLK and NPL rates */
clk_set_rate(rx->mclk, MCLK_FREQ);
- clk_set_rate(rx->npl, 2 * MCLK_FREQ);
+ clk_set_rate(rx->npl, MCLK_FREQ);

ret = clk_prepare_enable(rx->macro);
if (ret)
diff --git a/sound/soc/codecs/lpass-tx-macro.c b/sound/soc/codecs/lpass-tx-macro.c
index b9475ba55e20..2449a2df66df 100644
--- a/sound/soc/codecs/lpass-tx-macro.c
+++ b/sound/soc/codecs/lpass-tx-macro.c
@@ -203,7 +203,7 @@
#define TX_MACRO_AMIC_UNMUTE_DELAY_MS 100
#define TX_MACRO_DMIC_HPF_DELAY_MS 300
#define TX_MACRO_AMIC_HPF_DELAY_MS 300
-#define MCLK_FREQ 9600000
+#define MCLK_FREQ 19200000

enum {
TX_MACRO_AIF_INVALID = 0,
@@ -2014,7 +2014,7 @@ static int tx_macro_probe(struct platform_device *pdev)

/* set MCLK and NPL rates */
clk_set_rate(tx->mclk, MCLK_FREQ);
- clk_set_rate(tx->npl, 2 * MCLK_FREQ);
+ clk_set_rate(tx->npl, MCLK_FREQ);

ret = clk_prepare_enable(tx->macro);
if (ret)
--
2.21.0


2023-02-09 12:29:14

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 4/8] ASoC: qcom: audioreach: fix ADSP ready check

currently q6apm_is_adsp_ready() will only return the cached value of
previous result. If we are unlucky and previous result is not-ready
then the caller will always get not-ready flag.

This is not correct, we should query the dsp of its current state in
irrespective of previous reported state.

Fixes: 47bc8cf60e92 ("ASoC: qdsp6: audioreach: Add ADSP ready check")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/qcom/qdsp6/q6apm.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
index 8a7dfd27d3c5..994c9e823a88 100644
--- a/sound/soc/qcom/qdsp6/q6apm.c
+++ b/sound/soc/qcom/qdsp6/q6apm.c
@@ -145,14 +145,6 @@ static void q6apm_put_audioreach_graph(struct kref *ref)
kfree(graph);
}

-bool q6apm_is_adsp_ready(void)
-{
- if (g_apm && g_apm->state)
- return true;
-
- return false;
-}
-EXPORT_SYMBOL_GPL(q6apm_is_adsp_ready);

static int q6apm_get_apm_state(struct q6apm *apm)
{
@@ -169,6 +161,15 @@ static int q6apm_get_apm_state(struct q6apm *apm)
return apm->state;
}

+bool q6apm_is_adsp_ready(void)
+{
+ if (g_apm)
+ return q6apm_get_apm_state(g_apm);
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(q6apm_is_adsp_ready);
+
static struct audioreach_module *__q6apm_find_module_by_mid(struct q6apm *apm,
struct audioreach_graph_info *info,
uint32_t mid)
--
2.21.0


2023-02-09 12:29:14

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 7/8] ASoC: codecs: lpass: do not reset soundwire block on clk enable

resetting soundwire block will put the slaves out of sync and result
in re-enumeration during fsgen disable/enable path this is totally
unnecessary and resulting fifo overflows.

Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-rx-macro.c | 17 +++++++++++------
sound/soc/codecs/lpass-tx-macro.c | 15 ++++++++++-----
sound/soc/codecs/lpass-va-macro.c | 23 ++++++++++++-----------
sound/soc/codecs/lpass-wsa-macro.c | 18 +++++++++++-------
4 files changed, 44 insertions(+), 29 deletions(-)

diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
index 8621cfabcf5b..e0d891a67a12 100644
--- a/sound/soc/codecs/lpass-rx-macro.c
+++ b/sound/soc/codecs/lpass-rx-macro.c
@@ -3441,16 +3441,10 @@ static int swclk_gate_enable(struct clk_hw *hw)
}

rx_macro_mclk_enable(rx, true);
- regmap_update_bits(rx->regmap, CDC_RX_CLK_RST_CTRL_SWR_CONTROL,
- CDC_RX_SWR_RESET_MASK,
- CDC_RX_SWR_RESET);

regmap_update_bits(rx->regmap, CDC_RX_CLK_RST_CTRL_SWR_CONTROL,
CDC_RX_SWR_CLK_EN_MASK, 1);

- regmap_update_bits(rx->regmap, CDC_RX_CLK_RST_CTRL_SWR_CONTROL,
- CDC_RX_SWR_RESET_MASK, 0);
-
return 0;
}

@@ -3601,6 +3595,17 @@ static int rx_macro_probe(struct platform_device *pdev)
if (ret)
goto err_fsgen;

+ /* reset swr block */
+ regmap_update_bits(rx->regmap, CDC_RX_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_RX_SWR_RESET_MASK,
+ CDC_RX_SWR_RESET);
+
+ regmap_update_bits(rx->regmap, CDC_RX_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_RX_SWR_CLK_EN_MASK, 1);
+
+ regmap_update_bits(rx->regmap, CDC_RX_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_RX_SWR_RESET_MASK, 0);
+
ret = devm_snd_soc_register_component(dev, &rx_macro_component_drv,
rx_macro_dai,
ARRAY_SIZE(rx_macro_dai));
diff --git a/sound/soc/codecs/lpass-tx-macro.c b/sound/soc/codecs/lpass-tx-macro.c
index 2449a2df66df..bf27bdd5be20 100644
--- a/sound/soc/codecs/lpass-tx-macro.c
+++ b/sound/soc/codecs/lpass-tx-macro.c
@@ -1861,15 +1861,10 @@ static int swclk_gate_enable(struct clk_hw *hw)
}

tx_macro_mclk_enable(tx, true);
- regmap_update_bits(regmap, CDC_TX_CLK_RST_CTRL_SWR_CONTROL,
- CDC_TX_SWR_RESET_MASK, CDC_TX_SWR_RESET_ENABLE);

regmap_update_bits(regmap, CDC_TX_CLK_RST_CTRL_SWR_CONTROL,
CDC_TX_SWR_CLK_EN_MASK,
CDC_TX_SWR_CLK_ENABLE);
- regmap_update_bits(regmap, CDC_TX_CLK_RST_CTRL_SWR_CONTROL,
- CDC_TX_SWR_RESET_MASK, 0x0);
-
return 0;
}

@@ -2036,6 +2031,16 @@ static int tx_macro_probe(struct platform_device *pdev)
if (ret)
goto err_fsgen;

+ /* reset soundwire block */
+ regmap_update_bits(tx->regmap, CDC_TX_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_TX_SWR_RESET_MASK, CDC_TX_SWR_RESET_ENABLE);
+
+ regmap_update_bits(tx->regmap, CDC_TX_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_TX_SWR_CLK_EN_MASK,
+ CDC_TX_SWR_CLK_ENABLE);
+ regmap_update_bits(tx->regmap, CDC_TX_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_TX_SWR_RESET_MASK, 0x0);
+
ret = devm_snd_soc_register_component(dev, &tx_macro_component_drv,
tx_macro_dai,
ARRAY_SIZE(tx_macro_dai));
diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
index 1623ba78ddb3..fd62817d29a0 100644
--- a/sound/soc/codecs/lpass-va-macro.c
+++ b/sound/soc/codecs/lpass-va-macro.c
@@ -1333,17 +1333,9 @@ static int fsgen_gate_enable(struct clk_hw *hw)
int ret;

ret = va_macro_mclk_enable(va, true);
- if (!va->has_swr_master)
- return ret;
-
- regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
- CDC_VA_SWR_RESET_MASK, CDC_VA_SWR_RESET_ENABLE);
-
- regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
- CDC_VA_SWR_CLK_EN_MASK,
- CDC_VA_SWR_CLK_ENABLE);
- regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
- CDC_VA_SWR_RESET_MASK, 0x0);
+ if (va->has_swr_master)
+ regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_VA_SWR_CLK_EN_MASK, CDC_VA_SWR_CLK_ENABLE);

return ret;
}
@@ -1538,6 +1530,15 @@ static int va_macro_probe(struct platform_device *pdev)

}

+ if (va->has_swr_master) {
+ regmap_update_bits(va->regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_VA_SWR_RESET_MASK, CDC_VA_SWR_RESET_ENABLE);
+ regmap_update_bits(va->regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_VA_SWR_CLK_EN_MASK, CDC_VA_SWR_CLK_ENABLE);
+ regmap_update_bits(va->regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_VA_SWR_RESET_MASK, 0x0);
+ }
+
ret = devm_snd_soc_register_component(dev, &va_macro_component_drv,
va_macro_dais,
ARRAY_SIZE(va_macro_dais));
diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index c0b86d69c72e..e6b85f3692ac 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -2270,17 +2270,10 @@ static int wsa_swrm_clock(struct wsa_macro *wsa, bool enable)
}
wsa_macro_mclk_enable(wsa, true);

- /* reset swr ip */
- regmap_update_bits(regmap, CDC_WSA_CLK_RST_CTRL_SWR_CONTROL,
- CDC_WSA_SWR_RST_EN_MASK, CDC_WSA_SWR_RST_ENABLE);
-
regmap_update_bits(regmap, CDC_WSA_CLK_RST_CTRL_SWR_CONTROL,
CDC_WSA_SWR_CLK_EN_MASK,
CDC_WSA_SWR_CLK_ENABLE);

- /* Bring out of reset */
- regmap_update_bits(regmap, CDC_WSA_CLK_RST_CTRL_SWR_CONTROL,
- CDC_WSA_SWR_RST_EN_MASK, CDC_WSA_SWR_RST_DISABLE);
} else {
regmap_update_bits(regmap, CDC_WSA_CLK_RST_CTRL_SWR_CONTROL,
CDC_WSA_SWR_CLK_EN_MASK, 0);
@@ -2451,6 +2444,17 @@ static int wsa_macro_probe(struct platform_device *pdev)
if (ret)
goto err_fsgen;

+ /* reset swr ip */
+ regmap_update_bits(wsa->regmap, CDC_WSA_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_WSA_SWR_RST_EN_MASK, CDC_WSA_SWR_RST_ENABLE);
+
+ regmap_update_bits(wsa->regmap, CDC_WSA_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_WSA_SWR_CLK_EN_MASK, CDC_WSA_SWR_CLK_ENABLE);
+
+ /* Bring out of reset */
+ regmap_update_bits(wsa->regmap, CDC_WSA_CLK_RST_CTRL_SWR_CONTROL,
+ CDC_WSA_SWR_RST_EN_MASK, CDC_WSA_SWR_RST_DISABLE);
+
ret = devm_snd_soc_register_component(dev, &wsa_macro_component_drv,
wsa_macro_dai,
ARRAY_SIZE(wsa_macro_dai));
--
2.21.0


2023-02-09 12:29:14

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 5/8] ASoC: codecs: lpass: register mclk after runtime pm

move mclk out registration after runtime pm is enabled so that the
clk framework can resume the codec if it requires to enable the mclk out.

Fixes: c96baa2949b2 ("ASoC: codecs: wsa-macro: add runtime pm support")
Fixes: 72ad25eabda0 ("ASoC: codecs: va-macro: add runtime pm support")
Fixes: 366ff79ed539 ("ASoC: codecs: rx-macro: add runtime pm support")
Fixes: 1fb83bc5cf64 ("ASoC: codecs: tx-macro: add runtime pm support")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-rx-macro.c | 8 ++++----
sound/soc/codecs/lpass-tx-macro.c | 8 ++++----
sound/soc/codecs/lpass-va-macro.c | 20 ++++++++++----------
sound/soc/codecs/lpass-wsa-macro.c | 9 ++++-----
4 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
index a9ef9d5ffcc5..dd6970d5eb8d 100644
--- a/sound/soc/codecs/lpass-rx-macro.c
+++ b/sound/soc/codecs/lpass-rx-macro.c
@@ -3601,10 +3601,6 @@ static int rx_macro_probe(struct platform_device *pdev)
if (ret)
goto err_fsgen;

- ret = rx_macro_register_mclk_output(rx);
- if (ret)
- goto err_clkout;
-
ret = devm_snd_soc_register_component(dev, &rx_macro_component_drv,
rx_macro_dai,
ARRAY_SIZE(rx_macro_dai));
@@ -3618,6 +3614,10 @@ static int rx_macro_probe(struct platform_device *pdev)
pm_runtime_set_active(dev);
pm_runtime_enable(dev);

+ ret = rx_macro_register_mclk_output(rx);
+ if (ret)
+ goto err_clkout;
+
return 0;

err_clkout:
diff --git a/sound/soc/codecs/lpass-tx-macro.c b/sound/soc/codecs/lpass-tx-macro.c
index 2ef62d6edc30..b9475ba55e20 100644
--- a/sound/soc/codecs/lpass-tx-macro.c
+++ b/sound/soc/codecs/lpass-tx-macro.c
@@ -2036,10 +2036,6 @@ static int tx_macro_probe(struct platform_device *pdev)
if (ret)
goto err_fsgen;

- ret = tx_macro_register_mclk_output(tx);
- if (ret)
- goto err_clkout;
-
ret = devm_snd_soc_register_component(dev, &tx_macro_component_drv,
tx_macro_dai,
ARRAY_SIZE(tx_macro_dai));
@@ -2052,6 +2048,10 @@ static int tx_macro_probe(struct platform_device *pdev)
pm_runtime_set_active(dev);
pm_runtime_enable(dev);

+ ret = tx_macro_register_mclk_output(tx);
+ if (ret)
+ goto err_clkout;
+
return 0;

err_clkout:
diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
index b0b6cf29cba3..1623ba78ddb3 100644
--- a/sound/soc/codecs/lpass-va-macro.c
+++ b/sound/soc/codecs/lpass-va-macro.c
@@ -1524,16 +1524,6 @@ static int va_macro_probe(struct platform_device *pdev)
if (ret)
goto err_mclk;

- ret = va_macro_register_fsgen_output(va);
- if (ret)
- goto err_clkout;
-
- va->fsgen = clk_hw_get_clk(&va->hw, "fsgen");
- if (IS_ERR(va->fsgen)) {
- ret = PTR_ERR(va->fsgen);
- goto err_clkout;
- }
-
if (va->has_swr_master) {
/* Set default CLK div to 1 */
regmap_update_bits(va->regmap, CDC_VA_TOP_CSR_SWR_MIC_CTL0,
@@ -1560,6 +1550,16 @@ static int va_macro_probe(struct platform_device *pdev)
pm_runtime_set_active(dev);
pm_runtime_enable(dev);

+ ret = va_macro_register_fsgen_output(va);
+ if (ret)
+ goto err_clkout;
+
+ va->fsgen = clk_hw_get_clk(&va->hw, "fsgen");
+ if (IS_ERR(va->fsgen)) {
+ ret = PTR_ERR(va->fsgen);
+ goto err_clkout;
+ }
+
return 0;

err_clkout:
diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index 5cfe96f6e430..c0b86d69c72e 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -2451,11 +2451,6 @@ static int wsa_macro_probe(struct platform_device *pdev)
if (ret)
goto err_fsgen;

- ret = wsa_macro_register_mclk_output(wsa);
- if (ret)
- goto err_clkout;
-
-
ret = devm_snd_soc_register_component(dev, &wsa_macro_component_drv,
wsa_macro_dai,
ARRAY_SIZE(wsa_macro_dai));
@@ -2468,6 +2463,10 @@ static int wsa_macro_probe(struct platform_device *pdev)
pm_runtime_set_active(dev);
pm_runtime_enable(dev);

+ ret = wsa_macro_register_mclk_output(wsa);
+ if (ret)
+ goto err_clkout;
+
return 0;

err_clkout:
--
2.21.0


2023-02-09 12:29:14

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 8/8] ASoC: codecs: lpass: remove not so useful verbose log

Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/lpass-rx-macro.c | 4 +---
sound/soc/codecs/lpass-wsa-macro.c | 4 +---
2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
index e0d891a67a12..a73a7d7a1c0a 100644
--- a/sound/soc/codecs/lpass-rx-macro.c
+++ b/sound/soc/codecs/lpass-rx-macro.c
@@ -2296,10 +2296,8 @@ static int rx_macro_mux_put(struct snd_kcontrol *kcontrol,

aif_rst = rx->rx_port_value[widget->shift];
if (!rx_port_value) {
- if (aif_rst == 0) {
- dev_err(component->dev, "%s:AIF reset already\n", __func__);
+ if (aif_rst == 0)
return 0;
- }
if (aif_rst > RX_MACRO_AIF4_PB) {
dev_err(component->dev, "%s: Invalid AIF reset\n", __func__);
return 0;
diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index e6b85f3692ac..ba7480f3831e 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -1856,10 +1856,8 @@ static int wsa_macro_rx_mux_put(struct snd_kcontrol *kcontrol,

aif_rst = wsa->rx_port_value[widget->shift];
if (!rx_port_value) {
- if (aif_rst == 0) {
- dev_err(component->dev, "%s: AIF reset already\n", __func__);
+ if (aif_rst == 0)
return 0;
- }
if (aif_rst >= WSA_MACRO_RX_MAX) {
dev_err(component->dev, "%s: Invalid AIF reset\n", __func__);
return 0;
--
2.21.0


2023-02-09 18:36:22

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 0/8] ASoC: qcom: q6dsp and lpass codec stablity fixes

On Thu, 09 Feb 2023 12:27:58 +0000, Srinivas Kandagatla wrote:
> While testing X13s audio, we found multiple stablity issues this patchset
> fixes these issues.
> From q6dsp side issues are around locking of position pointer and handle
> multiple prepare cases along with pulse audio timerbased scheduling workaround.
>
> From LPASS codec side most of the staiblity issues were around runtime pm,
> hitting various issues as the codec was firstly resetting the soundwire block
> for every clk disable/enable which is taking the slaves out of sync and
> resulting in re-enumerating. Second issue was around fsgen clk is not
> brining up the codec out of suspend as it was not added after
> runtime pm enabled. Final issue was with codec mclk rate which should
> have been 192KHz same as npl instead of 96KHz. We were getting lucky as
> wsa drivers are setting the same clk to 192KHz.
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/8] ASoC: qcom: q6apm-lpass-dai: unprepare stream if its already prepared
commit: c2ac3aec474da0455df79c4a182f19687bc98d1d
[2/8] ASoC: qcom: q6apm-dai: fix race condition while updating the position pointer
commit: 84222ef54bfd8f043c23c8603fd5257a64b00780
[3/8] ASoC: qcom: q6apm-dai: Add SNDRV_PCM_INFO_BATCH flag
commit: aa759f3f9f4394a3af65ad1772fca6cb9dd9e4cc
[4/8] ASoC: qcom: audioreach: fix ADSP ready check
commit: dd33c2e7b21d72b151a87b5dafffee2c019043e5
[5/8] ASoC: codecs: lpass: register mclk after runtime pm
commit: 1dc3459009c33e335f0d62b84dd39a6bbd7fd5d2
[6/8] ASoC: codecs: lpass: fix incorrect mclk rate
commit: e7621434378c40b62ef858c14ae6415fb6469a8e
[7/8] ASoC: codecs: lpass: do not reset soundwire block on clk enable
commit: ddffe3b82849ba2774d7a06fbe1cc7e83378c4d2
[8/8] ASoC: codecs: lpass: remove not so useful verbose log
commit: 777af241a7ce6ed95f8d3fcb028c08f9b40addb6

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


2023-03-24 11:34:27

by Luca Weiss

[permalink] [raw]
Subject: Re: [PATCH 5/8] ASoC: codecs: lpass: register mclk after runtime pm

Hi Srinivas,

On Thu Feb 9, 2023 at 1:28 PM CET, Srinivas Kandagatla wrote:
> move mclk out registration after runtime pm is enabled so that the
> clk framework can resume the codec if it requires to enable the mclk out.
>
> Fixes: c96baa2949b2 ("ASoC: codecs: wsa-macro: add runtime pm support")
> Fixes: 72ad25eabda0 ("ASoC: codecs: va-macro: add runtime pm support")
> Fixes: 366ff79ed539 ("ASoC: codecs: rx-macro: add runtime pm support")
> Fixes: 1fb83bc5cf64 ("ASoC: codecs: tx-macro: add runtime pm support")
> Signed-off-by: Srinivas Kandagatla <[email protected]>

I see the following remoteproc crash with this patch in on
sm6350/sm7225-fairphone-fp4.

To be clear, all the audio parts for that SoC are not upstream (yet) and
there's still many issues to solve so not sure if it's actually my fault.

Anyways, getting this crash after starting adsp.

[ 97.212943] qcom_q6v5_pas 3000000.remoteproc: fatal error received: ABT_dal.c:287:ABTimeout: AHB Bus hang is detected, Number of bus hang detected := 1 , addr0 = 0x3370000 , addr1 = 0x0!!!
[ 97.212995] remoteproc remoteproc0: crash detected in 3000000.remoteproc: type fatal error
[ 97.213015] remoteproc remoteproc0: handling crash #1 in 3000000.remoteproc
[ 97.213022] remoteproc remoteproc0: recovering 3000000.remoteproc

This happens just after some clocks on the adsp get disabled, this is
from my own debug prints:
[ 97.189097] q6afe_set_lpass_clock: clk_id=780, attri=1, clk_root=0, freq=0
[ 97.189426] q6afe_set_lpass_clock: clk_id=781, attri=1, clk_root=0, freq=0

And then a couple of seconds later the whole phone just crashes into the
900e mode.

Regards
Luca


> ---
> sound/soc/codecs/lpass-rx-macro.c | 8 ++++----
> sound/soc/codecs/lpass-tx-macro.c | 8 ++++----
> sound/soc/codecs/lpass-va-macro.c | 20 ++++++++++----------
> sound/soc/codecs/lpass-wsa-macro.c | 9 ++++-----
> 4 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
> index a9ef9d5ffcc5..dd6970d5eb8d 100644
> --- a/sound/soc/codecs/lpass-rx-macro.c
> +++ b/sound/soc/codecs/lpass-rx-macro.c
> @@ -3601,10 +3601,6 @@ static int rx_macro_probe(struct platform_device *pdev)
> if (ret)
> goto err_fsgen;
>
> - ret = rx_macro_register_mclk_output(rx);
> - if (ret)
> - goto err_clkout;
> -
> ret = devm_snd_soc_register_component(dev, &rx_macro_component_drv,
> rx_macro_dai,
> ARRAY_SIZE(rx_macro_dai));
> @@ -3618,6 +3614,10 @@ static int rx_macro_probe(struct platform_device *pdev)
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
>
> + ret = rx_macro_register_mclk_output(rx);
> + if (ret)
> + goto err_clkout;
> +
> return 0;
>
> err_clkout:
> diff --git a/sound/soc/codecs/lpass-tx-macro.c b/sound/soc/codecs/lpass-tx-macro.c
> index 2ef62d6edc30..b9475ba55e20 100644
> --- a/sound/soc/codecs/lpass-tx-macro.c
> +++ b/sound/soc/codecs/lpass-tx-macro.c
> @@ -2036,10 +2036,6 @@ static int tx_macro_probe(struct platform_device *pdev)
> if (ret)
> goto err_fsgen;
>
> - ret = tx_macro_register_mclk_output(tx);
> - if (ret)
> - goto err_clkout;
> -
> ret = devm_snd_soc_register_component(dev, &tx_macro_component_drv,
> tx_macro_dai,
> ARRAY_SIZE(tx_macro_dai));
> @@ -2052,6 +2048,10 @@ static int tx_macro_probe(struct platform_device *pdev)
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
>
> + ret = tx_macro_register_mclk_output(tx);
> + if (ret)
> + goto err_clkout;
> +
> return 0;
>
> err_clkout:
> diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
> index b0b6cf29cba3..1623ba78ddb3 100644
> --- a/sound/soc/codecs/lpass-va-macro.c
> +++ b/sound/soc/codecs/lpass-va-macro.c
> @@ -1524,16 +1524,6 @@ static int va_macro_probe(struct platform_device *pdev)
> if (ret)
> goto err_mclk;
>
> - ret = va_macro_register_fsgen_output(va);
> - if (ret)
> - goto err_clkout;
> -
> - va->fsgen = clk_hw_get_clk(&va->hw, "fsgen");
> - if (IS_ERR(va->fsgen)) {
> - ret = PTR_ERR(va->fsgen);
> - goto err_clkout;
> - }
> -
> if (va->has_swr_master) {
> /* Set default CLK div to 1 */
> regmap_update_bits(va->regmap, CDC_VA_TOP_CSR_SWR_MIC_CTL0,
> @@ -1560,6 +1550,16 @@ static int va_macro_probe(struct platform_device *pdev)
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
>
> + ret = va_macro_register_fsgen_output(va);
> + if (ret)
> + goto err_clkout;
> +
> + va->fsgen = clk_hw_get_clk(&va->hw, "fsgen");
> + if (IS_ERR(va->fsgen)) {
> + ret = PTR_ERR(va->fsgen);
> + goto err_clkout;
> + }
> +
> return 0;
>
> err_clkout:
> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
> index 5cfe96f6e430..c0b86d69c72e 100644
> --- a/sound/soc/codecs/lpass-wsa-macro.c
> +++ b/sound/soc/codecs/lpass-wsa-macro.c
> @@ -2451,11 +2451,6 @@ static int wsa_macro_probe(struct platform_device *pdev)
> if (ret)
> goto err_fsgen;
>
> - ret = wsa_macro_register_mclk_output(wsa);
> - if (ret)
> - goto err_clkout;
> -
> -
> ret = devm_snd_soc_register_component(dev, &wsa_macro_component_drv,
> wsa_macro_dai,
> ARRAY_SIZE(wsa_macro_dai));
> @@ -2468,6 +2463,10 @@ static int wsa_macro_probe(struct platform_device *pdev)
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
>
> + ret = wsa_macro_register_mclk_output(wsa);
> + if (ret)
> + goto err_clkout;
> +
> return 0;
>
> err_clkout:
> --
> 2.21.0

2023-03-24 13:48:56

by Srinivas Kandagatla

[permalink] [raw]
Subject: Re: [PATCH 5/8] ASoC: codecs: lpass: register mclk after runtime pm

Hi Luca,

On 24/03/2023 11:22, Luca Weiss wrote:
> Hi Srinivas,
>
> On Thu Feb 9, 2023 at 1:28 PM CET, Srinivas Kandagatla wrote:
>> move mclk out registration after runtime pm is enabled so that the
>> clk framework can resume the codec if it requires to enable the mclk out.
>>
>> Fixes: c96baa2949b2 ("ASoC: codecs: wsa-macro: add runtime pm support")
>> Fixes: 72ad25eabda0 ("ASoC: codecs: va-macro: add runtime pm support")
>> Fixes: 366ff79ed539 ("ASoC: codecs: rx-macro: add runtime pm support")
>> Fixes: 1fb83bc5cf64 ("ASoC: codecs: tx-macro: add runtime pm support")
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>
> I see the following remoteproc crash with this patch in on
> sm6350/sm7225-fairphone-fp4.
>
> To be clear, all the audio parts for that SoC are not upstream (yet) and
> there's still many issues to solve so not sure if it's actually my fault.
>

please try this patch Amit did report this issue before

https://lore.kernel.org/lkml/[email protected]/


thanks,
srini
> Anyways, getting this crash after starting adsp.
>
> [ 97.212943] qcom_q6v5_pas 3000000.remoteproc: fatal error received: ABT_dal.c:287:ABTimeout: AHB Bus hang is detected, Number of bus hang detected := 1 , addr0 = 0x3370000 , addr1 = 0x0!!!
> [ 97.212995] remoteproc remoteproc0: crash detected in 3000000.remoteproc: type fatal error
> [ 97.213015] remoteproc remoteproc0: handling crash #1 in 3000000.remoteproc
> [ 97.213022] remoteproc remoteproc0: recovering 3000000.remoteproc
>
> This happens just after some clocks on the adsp get disabled, this is
> from my own debug prints:
> [ 97.189097] q6afe_set_lpass_clock: clk_id=780, attri=1, clk_root=0, freq=0
> [ 97.189426] q6afe_set_lpass_clock: clk_id=781, attri=1, clk_root=0, freq=0
>
> And then a couple of seconds later the whole phone just crashes into the
> 900e mode.
>
> Regards
> Luca
>
>
>> ---
>> sound/soc/codecs/lpass-rx-macro.c | 8 ++++----
>> sound/soc/codecs/lpass-tx-macro.c | 8 ++++----
>> sound/soc/codecs/lpass-va-macro.c | 20 ++++++++++----------
>> sound/soc/codecs/lpass-wsa-macro.c | 9 ++++-----
>> 4 files changed, 22 insertions(+), 23 deletions(-)
>>
>> diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
>> index a9ef9d5ffcc5..dd6970d5eb8d 100644
>> --- a/sound/soc/codecs/lpass-rx-macro.c
>> +++ b/sound/soc/codecs/lpass-rx-macro.c
>> @@ -3601,10 +3601,6 @@ static int rx_macro_probe(struct platform_device *pdev)
>> if (ret)
>> goto err_fsgen;
>>
>> - ret = rx_macro_register_mclk_output(rx);
>> - if (ret)
>> - goto err_clkout;
>> -
>> ret = devm_snd_soc_register_component(dev, &rx_macro_component_drv,
>> rx_macro_dai,
>> ARRAY_SIZE(rx_macro_dai));
>> @@ -3618,6 +3614,10 @@ static int rx_macro_probe(struct platform_device *pdev)
>> pm_runtime_set_active(dev);
>> pm_runtime_enable(dev);
>>
>> + ret = rx_macro_register_mclk_output(rx);
>> + if (ret)
>> + goto err_clkout;
>> +
>> return 0;
>>
>> err_clkout:
>> diff --git a/sound/soc/codecs/lpass-tx-macro.c b/sound/soc/codecs/lpass-tx-macro.c
>> index 2ef62d6edc30..b9475ba55e20 100644
>> --- a/sound/soc/codecs/lpass-tx-macro.c
>> +++ b/sound/soc/codecs/lpass-tx-macro.c
>> @@ -2036,10 +2036,6 @@ static int tx_macro_probe(struct platform_device *pdev)
>> if (ret)
>> goto err_fsgen;
>>
>> - ret = tx_macro_register_mclk_output(tx);
>> - if (ret)
>> - goto err_clkout;
>> -
>> ret = devm_snd_soc_register_component(dev, &tx_macro_component_drv,
>> tx_macro_dai,
>> ARRAY_SIZE(tx_macro_dai));
>> @@ -2052,6 +2048,10 @@ static int tx_macro_probe(struct platform_device *pdev)
>> pm_runtime_set_active(dev);
>> pm_runtime_enable(dev);
>>
>> + ret = tx_macro_register_mclk_output(tx);
>> + if (ret)
>> + goto err_clkout;
>> +
>> return 0;
>>
>> err_clkout:
>> diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
>> index b0b6cf29cba3..1623ba78ddb3 100644
>> --- a/sound/soc/codecs/lpass-va-macro.c
>> +++ b/sound/soc/codecs/lpass-va-macro.c
>> @@ -1524,16 +1524,6 @@ static int va_macro_probe(struct platform_device *pdev)
>> if (ret)
>> goto err_mclk;
>>
>> - ret = va_macro_register_fsgen_output(va);
>> - if (ret)
>> - goto err_clkout;
>> -
>> - va->fsgen = clk_hw_get_clk(&va->hw, "fsgen");
>> - if (IS_ERR(va->fsgen)) {
>> - ret = PTR_ERR(va->fsgen);
>> - goto err_clkout;
>> - }
>> -
>> if (va->has_swr_master) {
>> /* Set default CLK div to 1 */
>> regmap_update_bits(va->regmap, CDC_VA_TOP_CSR_SWR_MIC_CTL0,
>> @@ -1560,6 +1550,16 @@ static int va_macro_probe(struct platform_device *pdev)
>> pm_runtime_set_active(dev);
>> pm_runtime_enable(dev);
>>
>> + ret = va_macro_register_fsgen_output(va);
>> + if (ret)
>> + goto err_clkout;
>> +
>> + va->fsgen = clk_hw_get_clk(&va->hw, "fsgen");
>> + if (IS_ERR(va->fsgen)) {
>> + ret = PTR_ERR(va->fsgen);
>> + goto err_clkout;
>> + }
>> +
>> return 0;
>>
>> err_clkout:
>> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
>> index 5cfe96f6e430..c0b86d69c72e 100644
>> --- a/sound/soc/codecs/lpass-wsa-macro.c
>> +++ b/sound/soc/codecs/lpass-wsa-macro.c
>> @@ -2451,11 +2451,6 @@ static int wsa_macro_probe(struct platform_device *pdev)
>> if (ret)
>> goto err_fsgen;
>>
>> - ret = wsa_macro_register_mclk_output(wsa);
>> - if (ret)
>> - goto err_clkout;
>> -
>> -
>> ret = devm_snd_soc_register_component(dev, &wsa_macro_component_drv,
>> wsa_macro_dai,
>> ARRAY_SIZE(wsa_macro_dai));
>> @@ -2468,6 +2463,10 @@ static int wsa_macro_probe(struct platform_device *pdev)
>> pm_runtime_set_active(dev);
>> pm_runtime_enable(dev);
>>
>> + ret = wsa_macro_register_mclk_output(wsa);
>> + if (ret)
>> + goto err_clkout;
>> +
>> return 0;
>>
>> err_clkout:
>> --
>> 2.21.0
>

2023-03-31 11:43:40

by Luca Weiss

[permalink] [raw]
Subject: Re: [PATCH 5/8] ASoC: codecs: lpass: register mclk after runtime pm

On Fri Mar 24, 2023 at 2:40 PM CET, Srinivas Kandagatla wrote:
> Hi Luca,
>
> On 24/03/2023 11:22, Luca Weiss wrote:
> > Hi Srinivas,
> >
> > On Thu Feb 9, 2023 at 1:28 PM CET, Srinivas Kandagatla wrote:
> >> move mclk out registration after runtime pm is enabled so that the
> >> clk framework can resume the codec if it requires to enable the mclk out.
> >>
> >> Fixes: c96baa2949b2 ("ASoC: codecs: wsa-macro: add runtime pm support")
> >> Fixes: 72ad25eabda0 ("ASoC: codecs: va-macro: add runtime pm support")
> >> Fixes: 366ff79ed539 ("ASoC: codecs: rx-macro: add runtime pm support")
> >> Fixes: 1fb83bc5cf64 ("ASoC: codecs: tx-macro: add runtime pm support")
> >> Signed-off-by: Srinivas Kandagatla <[email protected]>
> >
> > I see the following remoteproc crash with this patch in on
> > sm6350/sm7225-fairphone-fp4.
> >
> > To be clear, all the audio parts for that SoC are not upstream (yet) and
> > there's still many issues to solve so not sure if it's actually my fault.
> >
>
> please try this patch Amit did report this issue before
>
> https://lore.kernel.org/lkml/[email protected]/

Right, that fixes it on sm6350/sm7225 also. Thanks!

Regards
Luca

>
>
> thanks,
> srini
> > Anyways, getting this crash after starting adsp.
> >
> > [ 97.212943] qcom_q6v5_pas 3000000.remoteproc: fatal error received: ABT_dal.c:287:ABTimeout: AHB Bus hang is detected, Number of bus hang detected := 1 , addr0 = 0x3370000 , addr1 = 0x0!!!
> > [ 97.212995] remoteproc remoteproc0: crash detected in 3000000.remoteproc: type fatal error
> > [ 97.213015] remoteproc remoteproc0: handling crash #1 in 3000000.remoteproc
> > [ 97.213022] remoteproc remoteproc0: recovering 3000000.remoteproc
> >
> > This happens just after some clocks on the adsp get disabled, this is
> > from my own debug prints:
> > [ 97.189097] q6afe_set_lpass_clock: clk_id=780, attri=1, clk_root=0, freq=0
> > [ 97.189426] q6afe_set_lpass_clock: clk_id=781, attri=1, clk_root=0, freq=0
> >
> > And then a couple of seconds later the whole phone just crashes into the
> > 900e mode.
> >
> > Regards
> > Luca
> >
> >
> >> ---
> >> sound/soc/codecs/lpass-rx-macro.c | 8 ++++----
> >> sound/soc/codecs/lpass-tx-macro.c | 8 ++++----
> >> sound/soc/codecs/lpass-va-macro.c | 20 ++++++++++----------
> >> sound/soc/codecs/lpass-wsa-macro.c | 9 ++++-----
> >> 4 files changed, 22 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
> >> index a9ef9d5ffcc5..dd6970d5eb8d 100644
> >> --- a/sound/soc/codecs/lpass-rx-macro.c
> >> +++ b/sound/soc/codecs/lpass-rx-macro.c
> >> @@ -3601,10 +3601,6 @@ static int rx_macro_probe(struct platform_device *pdev)
> >> if (ret)
> >> goto err_fsgen;
> >>
> >> - ret = rx_macro_register_mclk_output(rx);
> >> - if (ret)
> >> - goto err_clkout;
> >> -
> >> ret = devm_snd_soc_register_component(dev, &rx_macro_component_drv,
> >> rx_macro_dai,
> >> ARRAY_SIZE(rx_macro_dai));
> >> @@ -3618,6 +3614,10 @@ static int rx_macro_probe(struct platform_device *pdev)
> >> pm_runtime_set_active(dev);
> >> pm_runtime_enable(dev);
> >>
> >> + ret = rx_macro_register_mclk_output(rx);
> >> + if (ret)
> >> + goto err_clkout;
> >> +
> >> return 0;
> >>
> >> err_clkout:
> >> diff --git a/sound/soc/codecs/lpass-tx-macro.c b/sound/soc/codecs/lpass-tx-macro.c
> >> index 2ef62d6edc30..b9475ba55e20 100644
> >> --- a/sound/soc/codecs/lpass-tx-macro.c
> >> +++ b/sound/soc/codecs/lpass-tx-macro.c
> >> @@ -2036,10 +2036,6 @@ static int tx_macro_probe(struct platform_device *pdev)
> >> if (ret)
> >> goto err_fsgen;
> >>
> >> - ret = tx_macro_register_mclk_output(tx);
> >> - if (ret)
> >> - goto err_clkout;
> >> -
> >> ret = devm_snd_soc_register_component(dev, &tx_macro_component_drv,
> >> tx_macro_dai,
> >> ARRAY_SIZE(tx_macro_dai));
> >> @@ -2052,6 +2048,10 @@ static int tx_macro_probe(struct platform_device *pdev)
> >> pm_runtime_set_active(dev);
> >> pm_runtime_enable(dev);
> >>
> >> + ret = tx_macro_register_mclk_output(tx);
> >> + if (ret)
> >> + goto err_clkout;
> >> +
> >> return 0;
> >>
> >> err_clkout:
> >> diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
> >> index b0b6cf29cba3..1623ba78ddb3 100644
> >> --- a/sound/soc/codecs/lpass-va-macro.c
> >> +++ b/sound/soc/codecs/lpass-va-macro.c
> >> @@ -1524,16 +1524,6 @@ static int va_macro_probe(struct platform_device *pdev)
> >> if (ret)
> >> goto err_mclk;
> >>
> >> - ret = va_macro_register_fsgen_output(va);
> >> - if (ret)
> >> - goto err_clkout;
> >> -
> >> - va->fsgen = clk_hw_get_clk(&va->hw, "fsgen");
> >> - if (IS_ERR(va->fsgen)) {
> >> - ret = PTR_ERR(va->fsgen);
> >> - goto err_clkout;
> >> - }
> >> -
> >> if (va->has_swr_master) {
> >> /* Set default CLK div to 1 */
> >> regmap_update_bits(va->regmap, CDC_VA_TOP_CSR_SWR_MIC_CTL0,
> >> @@ -1560,6 +1550,16 @@ static int va_macro_probe(struct platform_device *pdev)
> >> pm_runtime_set_active(dev);
> >> pm_runtime_enable(dev);
> >>
> >> + ret = va_macro_register_fsgen_output(va);
> >> + if (ret)
> >> + goto err_clkout;
> >> +
> >> + va->fsgen = clk_hw_get_clk(&va->hw, "fsgen");
> >> + if (IS_ERR(va->fsgen)) {
> >> + ret = PTR_ERR(va->fsgen);
> >> + goto err_clkout;
> >> + }
> >> +
> >> return 0;
> >>
> >> err_clkout:
> >> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
> >> index 5cfe96f6e430..c0b86d69c72e 100644
> >> --- a/sound/soc/codecs/lpass-wsa-macro.c
> >> +++ b/sound/soc/codecs/lpass-wsa-macro.c
> >> @@ -2451,11 +2451,6 @@ static int wsa_macro_probe(struct platform_device *pdev)
> >> if (ret)
> >> goto err_fsgen;
> >>
> >> - ret = wsa_macro_register_mclk_output(wsa);
> >> - if (ret)
> >> - goto err_clkout;
> >> -
> >> -
> >> ret = devm_snd_soc_register_component(dev, &wsa_macro_component_drv,
> >> wsa_macro_dai,
> >> ARRAY_SIZE(wsa_macro_dai));
> >> @@ -2468,6 +2463,10 @@ static int wsa_macro_probe(struct platform_device *pdev)
> >> pm_runtime_set_active(dev);
> >> pm_runtime_enable(dev);
> >>
> >> + ret = wsa_macro_register_mclk_output(wsa);
> >> + if (ret)
> >> + goto err_clkout;
> >> +
> >> return 0;
> >>
> >> err_clkout:
> >> --
> >> 2.21.0
> >