[RFC, PATCH v2 1/2] ASoC: qcom: sdw: Add TDM support

[RFC, PATCH v2 1/2] ASoC: qcom: sdw: Add TDM support

[Jianhua Lu]

Add qcom TDM setup function to support TDM ports for qcom platform.

Signed-off-by: Jianhua Lu <lujianhua000@gmail.com>
---
Changes in v2:
  1. remove EXPORT_SYMBOL_GPL
  2. remove static modifier

 sound/soc/qcom/sdw.c | 63 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/sound/soc/qcom/sdw.c b/sound/soc/qcom/sdw.c
index 77dbe0c28b29..948abbd2e0e3 100644
--- a/sound/soc/qcom/sdw.c
+++ b/sound/soc/qcom/sdw.c
@@ -4,6 +4,7 @@
 
 #include <dt-bindings/sound/qcom,q6afe.h>
 #include <linux/module.h>
+#include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include "sdw.h"
 
@@ -101,6 +102,65 @@ int qcom_snd_sdw_prepare(struct snd_pcm_substream *substream,
 }
 EXPORT_SYMBOL_GPL(qcom_snd_sdw_prepare);
 
+int qcom_snd_tdm_hw_params(struct snd_pcm_substream *substream,
+			   struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
+	unsigned int tdm_slot_offset[8] = { 0, 4, 8, 12, 16, 20, 24, 28 };
+	int channels, slot_width;
+	int ret;
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		slot_width = 16;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		slot_width = 24;
+		break;
+	case SNDRV_PCM_FORMAT_S32_LE:
+		slot_width = 32;
+		break;
+	default:
+		dev_err(rtd->dev, "%s: invalid param format 0x%x\n", __func__,
+			params_format(params));
+		return -EINVAL;
+	}
+
+	channels = params_channels(params);
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0, 0x3, 8, slot_width);
+		if (ret < 0) {
+			dev_err(rtd->dev, "%s: failed to set tdm slot, err:%d\n",
+				__func__, ret);
+			return ret;
+		}
+
+		ret = snd_soc_dai_set_channel_map(cpu_dai, 0, NULL, channels,
+						  tdm_slot_offset);
+		if (ret < 0) {
+			dev_err(rtd->dev, "%s: failed to set channel map, err:%d\n",
+				__func__, ret);
+			return ret;
+		}
+	} else {
+		ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0xf, 0, 8, slot_width);
+		if (ret < 0) {
+			dev_err(rtd->dev, "%s: failed to set tdm slot, err:%d\n",
+				__func__, ret);
+			return ret;
+		}
+
+		ret = snd_soc_dai_set_channel_map(cpu_dai, channels,
+						  tdm_slot_offset, 0, NULL);
+		if (ret < 0) {
+			dev_err(rtd->dev, "%s: failed to set channel map, err:%d\n",
+				__func__, ret);
+			return ret;
+		}
+	}
+}
+
 int qcom_snd_sdw_hw_params(struct snd_pcm_substream *substream,
 			   struct snd_pcm_hw_params *params,
 			   struct sdw_stream_runtime **psruntime)
@@ -125,6 +185,9 @@ int qcom_snd_sdw_hw_params(struct snd_pcm_substream *substream,
 				*psruntime = sruntime;
 		}
 		break;
+	case PRIMARY_TDM_RX_0...QUINARY_TDM_TX_7:
+		qcom_snd_tdm_hw_params(substream, params);
+		break;
 	}
 
 	return 0;
-- 
2.41.0

[RFC, PATCH v2 2/2] ASoC: qcom: sm8250: Add TERTIARY_TDM_RX_0 support

[Jianhua Lu]

Add TERTIARY_TDM_RX_0 case to make speaker amplifiers working
on xiaomi-elish tablet.

Signed-off-by: Jianhua Lu <lujianhua000@gmail.com>
---
No changes in v2.

 sound/soc/qcom/sm8250.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/sound/soc/qcom/sm8250.c b/sound/soc/qcom/sm8250.c
index f298167c2a23..00c89c073e72 100644
--- a/sound/soc/qcom/sm8250.c
+++ b/sound/soc/qcom/sm8250.c
@@ -16,6 +16,7 @@
 
 #define DRIVER_NAME		"sm8250"
 #define MI2S_BCLK_RATE		1536000
+#define TDM_BCLK_RATE		12288000
 
 struct sm8250_snd_data {
 	bool stream_prepared[AFE_PORT_MAX];
@@ -53,6 +54,7 @@ static int sm8250_snd_startup(struct snd_pcm_substream *substream)
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
 	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
+	int ret, j;
 
 	switch (cpu_dai->id) {
 	case TERTIARY_MI2S_RX:
@@ -63,6 +65,23 @@ static int sm8250_snd_startup(struct snd_pcm_substream *substream)
 		snd_soc_dai_set_fmt(cpu_dai, fmt);
 		snd_soc_dai_set_fmt(codec_dai, codec_dai_fmt);
 		break;
+	case TERTIARY_TDM_RX_0:
+		codec_dai_fmt |= SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_DSP_A;
+		snd_soc_dai_set_sysclk(cpu_dai,
+			Q6AFE_LPASS_CLK_ID_TER_TDM_IBIT,
+			TDM_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
+
+		for_each_rtd_codec_dais(rtd, j, codec_dai) {
+			ret = snd_soc_dai_set_fmt(codec_dai, codec_dai_fmt);
+			snd_soc_dai_set_sysclk(codec_dai,
+				0,
+				TDM_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
+			if (ret < 0) {
+				dev_err(rtd->dev, "TDM fmt err:%d\n", ret);
+				return ret;
+			}
+		}
+		break;
 	default:
 		break;
 	}
-- 
2.41.0

Re: [RFC, PATCH v2 1/2] ASoC: qcom: sdw: Add TDM support

[Srinivas Kandagatla]

Thankyou for the patch,


On 12/12/2023 09:58, Jianhua Lu wrote:
> Add qcom TDM setup function to support TDM ports for qcom platform.
> 
> Signed-off-by: Jianhua Lu <lujianhua000@gmail.com>
> ---
> Changes in v2:
>    1. remove EXPORT_SYMBOL_GPL
>    2. remove static modifier
> 
>   sound/soc/qcom/sdw.c | 63 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
> 
> diff --git a/sound/soc/qcom/sdw.c b/sound/soc/qcom/sdw.c
> index 77dbe0c28b29..948abbd2e0e3 100644
> --- a/sound/soc/qcom/sdw.c
> +++ b/sound/soc/qcom/sdw.c
> @@ -4,6 +4,7 @@
>   
>   #include <dt-bindings/sound/qcom,q6afe.h>
>   #include <linux/module.h>
> +#include <sound/pcm_params.h>
>   #include <sound/soc.h>
>   #include "sdw.h"
>   
> @@ -101,6 +102,65 @@ int qcom_snd_sdw_prepare(struct snd_pcm_substream *substream,
>   }
>   EXPORT_SYMBOL_GPL(qcom_snd_sdw_prepare);
>   
> +int qcom_snd_tdm_hw_params(struct snd_pcm_substream *substream,
> +			   struct snd_pcm_hw_params *params)
> +{

TBH, this should not be part of sdw.c file, its intended for more of 
soundwire specific helpers, pl consider moving this to common.c for now.
Because, Not all old qcom platforms have soundwire controllers.

> +	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
> +	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
> +	unsigned int tdm_slot_offset[8] = { 0, 4, 8, 12, 16, 20, 24, 28 };
> +	int channels, slot_width;
> +	int ret;
> +
> +	switch (params_format(params)) {
> +	case SNDRV_PCM_FORMAT_S16_LE:
> +		slot_width = 16;
> +		break;
> +	case SNDRV_PCM_FORMAT_S24_LE:
> +		slot_width = 24;
> +		break;
> +	case SNDRV_PCM_FORMAT_S32_LE:
> +		slot_width = 32;
> +		break;
> +	default:
> +		dev_err(rtd->dev, "%s: invalid param format 0x%x\n", __func__,
> +			params_format(params));
> +		return -EINVAL;
> +	}
> +
> +	channels = params_channels(params);
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +		ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0, 0x3, 8, slot_width);

slot mask is always set to 2 channels in this case, should you not check 
the number of channels to determine the correct one?


These magic number 0, 0x3, 8 seems to make the code unreadable, can you 
do something like this:
snd_soc_dai_set_tdm_slot(cpu_dai, tx_mask, rx_mask, 
ARRAY_SIZE(tdm_slot_offset), slot_width);


> +		if (ret < 0) {
> +			dev_err(rtd->dev, "%s: failed to set tdm slot, err:%d\n",
> +				__func__, ret);
> +			return ret;
> +		}
> +
> +		ret = snd_soc_dai_set_channel_map(cpu_dai, 0, NULL, channels,
> +						  tdm_slot_offset);
> +		if (ret < 0) {
> +			dev_err(rtd->dev, "%s: failed to set channel map, err:%d\n",
> +				__func__, ret);
> +			return ret;
> +		}
> +	} else {
> +		ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0xf, 0, 8, slot_width);
> +		if (ret < 0) {
> +			dev_err(rtd->dev, "%s: failed to set tdm slot, err:%d\n",
> +				__func__, ret);
> +			return ret;
> +		}
> +
> +		ret = snd_soc_dai_set_channel_map(cpu_dai, channels,
> +						  tdm_slot_offset, 0, NULL);
> +		if (ret < 0) {
> +			dev_err(rtd->dev, "%s: failed to set channel map, err:%d\n",
> +				__func__, ret);
> +			return ret;
> +		}
> +	}
Finally  ./sound/soc/qcom/sdm845.c does have exactly same code, can you 
consider removing this and make use of this new helper in that file too.


> +}
> +
>   int qcom_snd_sdw_hw_params(struct snd_pcm_substream *substream,
>   			   struct snd_pcm_hw_params *params,
>   			   struct sdw_stream_runtime **psruntime)
> @@ -125,6 +185,9 @@ int qcom_snd_sdw_hw_params(struct snd_pcm_substream *substream,
>   				*psruntime = sruntime;
>   		}
>   		break;
> +	case PRIMARY_TDM_RX_0...QUINARY_TDM_TX_7:
> +		qcom_snd_tdm_hw_params(substream, params);
> +		break;
>   	}
>   
>   	return 0;

Re: [RFC, PATCH v2 1/2] ASoC: qcom: sdw: Add TDM support

[Jianhua Lu]

On Tue, Dec 12, 2023 at 11:47:36AM +0000, Srinivas Kandagatla wrote:
> > +int qcom_snd_tdm_hw_params(struct snd_pcm_substream *substream,
> > +			   struct snd_pcm_hw_params *params)
> > +{
> 
> TBH, this should not be part of sdw.c file, its intended for more of 
> soundwire specific helpers, pl consider moving this to common.c for now.
> Because, Not all old qcom platforms have soundwire controllers.

Acked.
> 
> > +		ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0, 0x3, 8, slot_width);
> 
> slot mask is always set to 2 channels in this case, should you not check 
> the number of channels to determine the correct one?
> 
> 
> These magic number 0, 0x3, 8 seems to make the code unreadable, can you 
> do something like this:
> snd_soc_dai_set_tdm_slot(cpu_dai, tx_mask, rx_mask, 
> ARRAY_SIZE(tdm_slot_offset), slot_width);

Acked.
> 
> > +		}
> > +	}
> Finally  ./sound/soc/qcom/sdm845.c does have exactly same code, can you 
> consider removing this and make use of this new helper in that file too.

Acked.

Thanks for your reveiw very much, I will do it in patch v3.