[PATCH 0/2] ASoC: sdm845: fix soundwire stream handling

[PATCH 0/2] ASoC: sdm845: fix soundwire stream handling

[Srinivas Kandagatla]

Recent addition of SoundWire stream state-machine checks in linux-next
have shown an existing issue with handling soundwire streams in codec drivers.

In general soundwire stream prepare/enable/disable can be called from either
codec/machine/controller driver. However calling it in codec driver means
that if multiple instances(Left/Right speakers) of the same codec is
connected to the same stream then it will endup calling stream
prepare/enable/disable more than once. This will mess up the stream
state-machine checks in the soundwire core.

Moving this stream handling to machine driver would fix this issue
and also allow board/platform specfic power sequencing.


Srinivas Kandagatla (2):
  ASoC: qcom: sdm845: handle soundwire stream
  ASoC: codecs: wsa881x: remove soundwire stream handling

 sound/soc/codecs/wsa881x.c | 44 +-----------------------
 sound/soc/qcom/Kconfig     |  2 +-
 sound/soc/qcom/sdm845.c    | 69 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 44 deletions(-)

-- 
2.21.0

[PATCH 1/2] ASoC: qcom: sdm845: handle soundwire stream

[Srinivas Kandagatla]

In existing setup WSA881x codec handles soundwire stream,
however DB845c and other machines based on SDM845c have 2
instances for WSA881x codec. This will force soundwire stream
to be prepared/enabled twice or multiple times.
Handling SoundWire Stream in machine driver would fix this issue.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/Kconfig  |  2 +-
 sound/soc/qcom/sdm845.c | 69 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index 6530d2462a9e..f51b28d1b94d 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -99,7 +99,7 @@ config SND_SOC_MSM8996
 
 config SND_SOC_SDM845
 	tristate "SoC Machine driver for SDM845 boards"
-	depends on QCOM_APR && CROS_EC && I2C
+	depends on QCOM_APR && CROS_EC && I2C && SOUNDWIRE
 	select SND_SOC_QDSP6
 	select SND_SOC_QCOM_COMMON
 	select SND_SOC_RT5663
diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
index 3ac02204a706..82ec71a1e3bd 100644
--- a/sound/soc/qcom/sdm845.c
+++ b/sound/soc/qcom/sdm845.c
@@ -11,6 +11,7 @@
 #include <sound/pcm_params.h>
 #include <sound/jack.h>
 #include <sound/soc.h>
+#include <linux/soundwire/sdw.h>
 #include <uapi/linux/input-event-codes.h>
 #include "common.h"
 #include "qdsp6/q6afe.h"
@@ -31,10 +32,12 @@
 struct sdm845_snd_data {
 	struct snd_soc_jack jack;
 	bool jack_setup;
+	bool stream_prepared[SLIM_MAX_RX_PORTS];
 	struct snd_soc_card *card;
 	uint32_t pri_mi2s_clk_count;
 	uint32_t sec_mi2s_clk_count;
 	uint32_t quat_tdm_clk_count;
+	struct sdw_stream_runtime *sruntime[SLIM_MAX_RX_PORTS];
 };
 
 static unsigned int tdm_slot_offset[8] = {0, 4, 8, 12, 16, 20, 24, 28};
@@ -45,11 +48,20 @@ static int sdm845_slim_snd_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai *codec_dai;
+	struct sdm845_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card);
 	u32 rx_ch[SLIM_MAX_RX_PORTS], tx_ch[SLIM_MAX_TX_PORTS];
+	struct sdw_stream_runtime *sruntime;
 	u32 rx_ch_cnt = 0, tx_ch_cnt = 0;
 	int ret = 0, i;
 
 	for_each_rtd_codec_dais(rtd, i, codec_dai) {
+		sruntime = snd_soc_dai_get_sdw_stream(codec_dai,
+						      substream->stream);
+		if (sruntime != ERR_PTR(-ENOTSUPP))
+			pdata->sruntime[cpu_dai->id] = sruntime;
+		else
+			pdata->sruntime[cpu_dai->id] = NULL;
+
 		ret = snd_soc_dai_get_channel_map(codec_dai,
 				&tx_ch_cnt, tx_ch, &rx_ch_cnt, rx_ch);
 
@@ -425,8 +437,65 @@ static void  sdm845_snd_shutdown(struct snd_pcm_substream *substream)
 	}
 }
 
+static int sdm845_snd_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct sdm845_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
+	int ret;
+
+	if (!sruntime)
+		return 0;
+
+	if (data->stream_prepared[cpu_dai->id]) {
+		sdw_disable_stream(sruntime);
+		sdw_deprepare_stream(sruntime);
+		data->stream_prepared[cpu_dai->id] = false;
+	}
+
+	ret = sdw_prepare_stream(sruntime);
+	if (ret)
+		return ret;
+
+	/**
+	 * NOTE: there is a strict hw requirement about the ordering of port
+	 * enables and actual WSA881x PA enable. PA enable should only happen
+	 * after soundwire ports are enabled if not DC on the line is
+	 * accumulated resulting in Click/Pop Noise
+	 * PA enable/mute are handled as part of codec DAPM and digital mute.
+	 */
+
+	ret = sdw_enable_stream(sruntime);
+	if (ret) {
+		sdw_deprepare_stream(sruntime);
+		return ret;
+	}
+	data->stream_prepared[cpu_dai->id] = true;
+
+	return ret;
+}
+
+static int sdm845_snd_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct sdm845_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
+
+	if (sruntime && data->stream_prepared[cpu_dai->id]) {
+		sdw_disable_stream(sruntime);
+		sdw_deprepare_stream(sruntime);
+		data->stream_prepared[cpu_dai->id] = false;
+	}
+
+	return 0;
+}
+
 static const struct snd_soc_ops sdm845_be_ops = {
 	.hw_params = sdm845_snd_hw_params,
+	.hw_free = sdm845_snd_hw_free,
+	.prepare = sdm845_snd_prepare,
 	.startup = sdm845_snd_startup,
 	.shutdown = sdm845_snd_shutdown,
 };
-- 
2.21.0

Re: [PATCH 1/2] ASoC: qcom: sdm845: handle soundwire stream

[Pierre-Louis Bossart]

> @@ -45,11 +48,20 @@ static int sdm845_slim_snd_hw_params(struct snd_pcm_substream *substream,
>   	struct snd_soc_pcm_runtime *rtd = substream->private_data;
>   	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
>   	struct snd_soc_dai *codec_dai;
> +	struct sdm845_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card);
>   	u32 rx_ch[SLIM_MAX_RX_PORTS], tx_ch[SLIM_MAX_TX_PORTS];
> +	struct sdw_stream_runtime *sruntime;
>   	u32 rx_ch_cnt = 0, tx_ch_cnt = 0;
>   	int ret = 0, i;
>   
>   	for_each_rtd_codec_dais(rtd, i, codec_dai) {
> +		sruntime = snd_soc_dai_get_sdw_stream(codec_dai,
> +						      substream->stream);
> +		if (sruntime != ERR_PTR(-ENOTSUPP))
> +			pdata->sruntime[cpu_dai->id] = sruntime;
> +		else
> +			pdata->sruntime[cpu_dai->id] = NULL;
> +

Can you explain this part?
The get_sdw_stream() is supposed to return what was set by 
set_sdw_stream(), so if it's not supported isn't this an error?

>   		ret = snd_soc_dai_get_channel_map(codec_dai,
>   				&tx_ch_cnt, tx_ch, &rx_ch_cnt, rx_ch);
>   
> @@ -425,8 +437,65 @@ static void  sdm845_snd_shutdown(struct snd_pcm_substream *substream)
>   	}
>   }
>   
> +static int sdm845_snd_prepare(struct snd_pcm_substream *substream)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct sdm845_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
> +	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> +	struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
> +	int ret;
> +
> +	if (!sruntime)
> +		return 0;

same here, isn't this an error?

> +	if (data->stream_prepared[cpu_dai->id]) {
> +		sdw_disable_stream(sruntime);
> +		sdw_deprepare_stream(sruntime);
> +		data->stream_prepared[cpu_dai->id] = false;
> +	}
> +
> +	ret = sdw_prepare_stream(sruntime);
> +	if (ret)
> +		return ret;
> +
> +	/**
> +	 * NOTE: there is a strict hw requirement about the ordering of port
> +	 * enables and actual WSA881x PA enable. PA enable should only happen
> +	 * after soundwire ports are enabled if not DC on the line is
> +	 * accumulated resulting in Click/Pop Noise
> +	 * PA enable/mute are handled as part of codec DAPM and digital mute.
> +	 */
> +
> +	ret = sdw_enable_stream(sruntime);
> +	if (ret) {
> +		sdw_deprepare_stream(sruntime);
> +		return ret;
> +	}
> +	data->stream_prepared[cpu_dai->id] = true;
> +
> +	return ret;
> +}
> +
> +static int sdm845_snd_hw_free(struct snd_pcm_substream *substream)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct sdm845_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
> +	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> +	struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
> +
> +	if (sruntime && data->stream_prepared[cpu_dai->id]) {

and here?

Really wondering where the stream is actually allocated and set.

> +		sdw_disable_stream(sruntime);
> +		sdw_deprepare_stream(sruntime);
> +		data->stream_prepared[cpu_dai->id] = false;
> +	}
> +
> +	return 0;
> +}
> +
>   static const struct snd_soc_ops sdm845_be_ops = {
>   	.hw_params = sdm845_snd_hw_params,
> +	.hw_free = sdm845_snd_hw_free,
> +	.prepare = sdm845_snd_prepare,
>   	.startup = sdm845_snd_startup,
>   	.shutdown = sdm845_snd_shutdown,
>   };
> 

Re: [PATCH 1/2] ASoC: qcom: sdm845: handle soundwire stream

[Srinivas Kandagatla]

On 17/03/2020 13:07, Pierre-Louis Bossart wrote:
> 
>> @@ -45,11 +48,20 @@ static int sdm845_slim_snd_hw_params(struct 
>> snd_pcm_substream *substream,
>>       struct snd_soc_pcm_runtime *rtd = substream->private_data;
>>       struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
>>       struct snd_soc_dai *codec_dai;
>> +    struct sdm845_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card);
>>       u32 rx_ch[SLIM_MAX_RX_PORTS], tx_ch[SLIM_MAX_TX_PORTS];
>> +    struct sdw_stream_runtime *sruntime;
>>       u32 rx_ch_cnt = 0, tx_ch_cnt = 0;
>>       int ret = 0, i;
>>       for_each_rtd_codec_dais(rtd, i, codec_dai) {
>> +        sruntime = snd_soc_dai_get_sdw_stream(codec_dai,
>> +                              substream->stream);
>> +        if (sruntime != ERR_PTR(-ENOTSUPP))
>> +            pdata->sruntime[cpu_dai->id] = sruntime;
>> +        else
>> +            pdata->sruntime[cpu_dai->id] = NULL;
>> +
> 
> Can you explain this part?
> The get_sdw_stream() is supposed to return what was set by 
> set_sdw_stream(), so if it's not supported isn't this an error?

In this case its not an error because we have
totally 3 codecs in this path.
One wcd934x Slimbus codec and two wsa881x Soundwire Codec.

wcd934x codec side will return ENOTSUPP which is not an error.

> 
>>           ret = snd_soc_dai_get_channel_map(codec_dai,
>>                   &tx_ch_cnt, tx_ch, &rx_ch_cnt, rx_ch);
>> @@ -425,8 +437,65 @@ static void  sdm845_snd_shutdown(struct 
>> snd_pcm_substream *substream)
>>       }
>>   }
>> +static int sdm845_snd_prepare(struct snd_pcm_substream *substream)
>> +{
>> +    struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> +    struct sdm845_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
>> +    struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
>> +    struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
>> +    int ret;
>> +
>> +    if (!sruntime)
>> +        return 0;
> 
> same here, isn't this an error?

These callbacks are used for other dais aswell in this case
HDMI, Slimbus and Soundwire, so sruntime being null is not treated as error.

> 
>> +    if (data->stream_prepared[cpu_dai->id]) {
>> +        sdw_disable_stream(sruntime);
>> +        sdw_deprepare_stream(sruntime);
>> +        data->stream_prepared[cpu_dai->id] = false;
>> +    }
>> +
>> +    ret = sdw_prepare_stream(sruntime);
>> +    if (ret)
>> +        return ret;
>> +
>> +    /**
>> +     * NOTE: there is a strict hw requirement about the ordering of port
>> +     * enables and actual WSA881x PA enable. PA enable should only 
>> happen
>> +     * after soundwire ports are enabled if not DC on the line is
>> +     * accumulated resulting in Click/Pop Noise
>> +     * PA enable/mute are handled as part of codec DAPM and digital 
>> mute.
>> +     */
>> +
>> +    ret = sdw_enable_stream(sruntime);
>> +    if (ret) {
>> +        sdw_deprepare_stream(sruntime);
>> +        return ret;
>> +    }
>> +    data->stream_prepared[cpu_dai->id] = true;
>> +
>> +    return ret;
>> +}
>> +
>> +static int sdm845_snd_hw_free(struct snd_pcm_substream *substream)
>> +{
>> +    struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> +    struct sdm845_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
>> +    struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
>> +    struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
>> +
>> +    if (sruntime && data->stream_prepared[cpu_dai->id]) {
> 
> and here?
> 
> Really wondering where the stream is actually allocated and set.

Controller is allocating the runtime in this case.

> 
>> +        sdw_disable_stream(sruntime);
>> +        sdw_deprepare_stream(sruntime);
>> +        data->stream_prepared[cpu_dai->id] = false;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static const struct snd_soc_ops sdm845_be_ops = {
>>       .hw_params = sdm845_snd_hw_params,
>> +    .hw_free = sdm845_snd_hw_free,
>> +    .prepare = sdm845_snd_prepare,
>>       .startup = sdm845_snd_startup,
>>       .shutdown = sdm845_snd_shutdown,
>>   };
>>

Re: [PATCH 1/2] ASoC: qcom: sdm845: handle soundwire stream

[Pierre-Louis Bossart]

>>>       for_each_rtd_codec_dais(rtd, i, codec_dai) {
>>> +        sruntime = snd_soc_dai_get_sdw_stream(codec_dai,
>>> +                              substream->stream);
>>> +        if (sruntime != ERR_PTR(-ENOTSUPP))
>>> +            pdata->sruntime[cpu_dai->id] = sruntime;
>>> +        else
>>> +            pdata->sruntime[cpu_dai->id] = NULL;
>>> +
>>
>> Can you explain this part?
>> The get_sdw_stream() is supposed to return what was set by 
>> set_sdw_stream(), so if it's not supported isn't this an error?
> 
> In this case its not an error because we have
> totally 3 codecs in this path.
> One wcd934x Slimbus codec and two wsa881x Soundwire Codec.
> 
> wcd934x codec side will return ENOTSUPP which is not an error.

I must admit I am quite confused here.
After reading your response, I thought this was a case of codec-to-codec 
dailink, but then you also have a notion of cpu_dai?

>>
>>>           ret = snd_soc_dai_get_channel_map(codec_dai,
>>>                   &tx_ch_cnt, tx_ch, &rx_ch_cnt, rx_ch);
>>> @@ -425,8 +437,65 @@ static void  sdm845_snd_shutdown(struct 
>>> snd_pcm_substream *substream)
>>>       }
>>>   }
>>> +static int sdm845_snd_prepare(struct snd_pcm_substream *substream)
>>> +{
>>> +    struct snd_soc_pcm_runtime *rtd = substream->private_data;
>>> +    struct sdm845_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
>>> +    struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
>>> +    struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
>>> +    int ret;
>>> +
>>> +    if (!sruntime)
>>> +        return 0;
>>
>> same here, isn't this an error?
> 
> These callbacks are used for other dais aswell in this case
> HDMI, Slimbus and Soundwire, so sruntime being null is not treated as 
> error.

Same comment, how does the notion of cpu_dai come in the picture for a 
SoundWire dailink?
Would you mind listing what the components of the dailinks are?

Re: [PATCH 1/2] ASoC: qcom: sdm845: handle soundwire stream

[Srinivas Kandagatla]

On 18/03/2020 15:26, Pierre-Louis Bossart wrote:
> 
> Same comment, how does the notion of cpu_dai come in the picture for a 
> SoundWire dailink?
> Would you mind listing what the components of the dailinks are?

dais that I was referring here are all codec dais from backend-dai.

Device tree entries from
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/qcom/sdm845-db845c.dts?h=next-20200318#n538


Frontend-dai:
	mm1-dai-link {
		link-name = "MultiMedia1";
		cpu {
			sound-dai = <&q6asmdai  MSM_FRONTEND_DAI_MULTIMEDIA1>;
		};
	};

Backend-dai:
	slim-dai-link {
		link-name = "SLIM Playback";
		cpu {
			sound-dai = <&q6afedai SLIMBUS_0_RX>;
		};

		platform {
			sound-dai = <&q6routing>;
		};

		codec {
			sound-dai =  <&left_spkr>, <&right_spkr>, <&swm 0>, <&wcd9340 0>;
		};
	};


--srini

Re: [PATCH 1/2] ASoC: qcom: sdm845: handle soundwire stream

[Pierre-Louis Bossart]

On 3/18/20 10:57 AM, Srinivas Kandagatla wrote:
> 
> 
> On 18/03/2020 15:26, Pierre-Louis Bossart wrote:
>>
>> Same comment, how does the notion of cpu_dai come in the picture for a 
>> SoundWire dailink?
>> Would you mind listing what the components of the dailinks are?
> 
> dais that I was referring here are all codec dais from backend-dai.
> 
> Device tree entries from
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/qcom/sdm845-db845c.dts?h=next-20200318#n538 
> 
> 
> 
> Frontend-dai:
>      mm1-dai-link {
>          link-name = "MultiMedia1";
>          cpu {
>              sound-dai = <&q6asmdai  MSM_FRONTEND_DAI_MULTIMEDIA1>;
>          };
>      };
> 
> Backend-dai:
>      slim-dai-link {
>          link-name = "SLIM Playback";
>          cpu {
>              sound-dai = <&q6afedai SLIMBUS_0_RX>;
>          };
> 
>          platform {
>              sound-dai = <&q6routing>;
>          };
> 
>          codec {
>              sound-dai =  <&left_spkr>, <&right_spkr>, <&swm 0>, 
> <&wcd9340 0>;
>          };

Thanks, I didn't realize this and now understand your point.

I guess that means we've officially stretched the limits of the DPCM 
model though, lumping all codec dais from separate devices into the same 
'backend' doesn't seem like a very good path forward, we'd really need a 
notion of domain to represent such bridges.

For now for the series

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

[PATCH 2/2] ASoC: codecs: wsa881x: remove soundwire stream handling

[Srinivas Kandagatla]

There could be multiple instances of this codec on any platform,
so handling stream directly in this codec driver can lead to
multiple calls to prepare/enable/disable on the same SoundWire stream.
Move this stream handling to machine driver to fix this issue.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/codecs/wsa881x.c | 44 +-------------------------------------
 1 file changed, 1 insertion(+), 43 deletions(-)

diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c
index 1810e0775efe..d39d479e2378 100644
--- a/sound/soc/codecs/wsa881x.c
+++ b/sound/soc/codecs/wsa881x.c
@@ -680,7 +680,6 @@ struct wsa881x_priv {
 	int active_ports;
 	bool port_prepared[WSA881X_MAX_SWR_PORTS];
 	bool port_enable[WSA881X_MAX_SWR_PORTS];
-	bool stream_prepared;
 };
 
 static void wsa881x_init(struct wsa881x_priv *wsa881x)
@@ -958,41 +957,6 @@ static const struct snd_soc_dapm_widget wsa881x_dapm_widgets[] = {
 	SND_SOC_DAPM_OUTPUT("SPKR"),
 };
 
-static int wsa881x_prepare(struct snd_pcm_substream *substream,
-			   struct snd_soc_dai *dai)
-{
-	struct wsa881x_priv *wsa881x = dev_get_drvdata(dai->dev);
-	int ret;
-
-	if (wsa881x->stream_prepared) {
-		sdw_disable_stream(wsa881x->sruntime);
-		sdw_deprepare_stream(wsa881x->sruntime);
-		wsa881x->stream_prepared = false;
-	}
-
-
-	ret = sdw_prepare_stream(wsa881x->sruntime);
-	if (ret)
-		return ret;
-
-	/**
-	 * NOTE: there is a strict hw requirement about the ordering of port
-	 * enables and actual PA enable. PA enable should only happen after
-	 * soundwire ports are enabled if not DC on the line is accumulated
-	 * resulting in Click/Pop Noise
-	 * PA enable/mute are handled as part of DAPM and digital mute.
-	 */
-
-	ret = sdw_enable_stream(wsa881x->sruntime);
-	if (ret) {
-		sdw_deprepare_stream(wsa881x->sruntime);
-		return ret;
-	}
-	wsa881x->stream_prepared = true;
-
-	return ret;
-}
-
 static int wsa881x_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *params,
 			     struct snd_soc_dai *dai)
@@ -1020,12 +984,7 @@ static int wsa881x_hw_free(struct snd_pcm_substream *substream,
 {
 	struct wsa881x_priv *wsa881x = dev_get_drvdata(dai->dev);
 
-	if (wsa881x->stream_prepared) {
-		sdw_disable_stream(wsa881x->sruntime);
-		sdw_deprepare_stream(wsa881x->sruntime);
-		sdw_stream_remove_slave(wsa881x->slave, wsa881x->sruntime);
-		wsa881x->stream_prepared = false;
-	}
+	sdw_stream_remove_slave(wsa881x->slave, wsa881x->sruntime);
 
 	return 0;
 }
@@ -1056,7 +1015,6 @@ static int wsa881x_digital_mute(struct snd_soc_dai *dai, int mute, int stream)
 
 static struct snd_soc_dai_ops wsa881x_dai_ops = {
 	.hw_params = wsa881x_hw_params,
-	.prepare = wsa881x_prepare,
 	.hw_free = wsa881x_hw_free,
 	.mute_stream = wsa881x_digital_mute,
 	.set_sdw_stream = wsa881x_set_sdw_stream,
-- 
2.21.0

Re: [PATCH 0/2] ASoC: sdm845: fix soundwire stream handling

[Pierre-Louis Bossart]

Hi Srinivas,

On 3/17/20 4:53 AM, Srinivas Kandagatla wrote:
> Recent addition of SoundWire stream state-machine checks in linux-next
> have shown an existing issue with handling soundwire streams in codec drivers.
> 
> In general soundwire stream prepare/enable/disable can be called from either
> codec/machine/controller driver. However calling it in codec driver means
> that if multiple instances(Left/Right speakers) of the same codec is
> connected to the same stream then it will endup calling stream
> prepare/enable/disable more than once. This will mess up the stream
> state-machine checks in the soundwire core.

That's a known issue that we've fixed on the Intel side a  month or two 
ago. Unfortunately the review cycle is so slow that you don't benefit 
immediately from our fixes, what can I say.

> Moving this stream handling to machine driver would fix this issue
> and also allow board/platform specfic power sequencing.

It's fine but that's unnecessary, and if you start having multiple 
machine drivers with the same codecs you'll duplicate the stream 
handling code.

All you need to ensure in a multi-codec or multi-cpu dai case is that 
the stream is allocated once, and yes that's typically done as part of 
the dailink .startup callback.

Then it's fine to call the stream prepare or enable multiple times from 
the individual dai level, and only do the transition for the first call.

See patch "soundwire: stream: only change state if needed" that I just 
shared. We've used it both in 'TDM' mode with two codecs hanging off of 
the same link and in 'aggregated' mode with two codecs on separate links.


> Srinivas Kandagatla (2):
>    ASoC: qcom: sdm845: handle soundwire stream
>    ASoC: codecs: wsa881x: remove soundwire stream handling
> 
>   sound/soc/codecs/wsa881x.c | 44 +-----------------------
>   sound/soc/qcom/Kconfig     |  2 +-
>   sound/soc/qcom/sdm845.c    | 69 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 71 insertions(+), 44 deletions(-)
>