| Message ID | 20231213123556.20469-1-lujianhua000@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/4] ASoC: qcom: common: Add qcom_snd_tdm_hw_params function | expand |
On Wed, Dec 13, 2023 at 08:35:53PM +0800, Jianhua Lu wrote: > Add qcom TDM setup function to support TDM ports for qcom platform. > +int qcom_snd_tdm_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params) > +{ ... > + ret = snd_soc_dai_set_tdm_slot(cpu_dai, tx_mask, rx_mask, slots, slot_width); > + if (ret < 0) { The expectation is that TDM is set up by the machine driver, not from hw_params - if the TDM setup can be changed from within hw_params then it's hard to see how it's going to interact well with other TDM users on the bus. More usually hw_params() would be influenced by the setup done in set_tdm_slot().
On Thu, Dec 14, 2023 at 11:11:06AM +0000, Mark Brown wrote: > On Wed, Dec 13, 2023 at 08:35:53PM +0800, Jianhua Lu wrote: > > > Add qcom TDM setup function to support TDM ports for qcom platform. > > > +int qcom_snd_tdm_hw_params(struct snd_pcm_substream *substream, > > + struct snd_pcm_hw_params *params) > > +{ > > ... > > > + ret = snd_soc_dai_set_tdm_slot(cpu_dai, tx_mask, rx_mask, slots, slot_width); > > + if (ret < 0) { > > The expectation is that TDM is set up by the machine driver, not from > hw_params - if the TDM setup can be changed from within hw_params then > it's hard to see how it's going to interact well with other TDM users on > the bus. More usually hw_params() would be influenced by the setup done > in set_tdm_slot(). Currently, qcom TDM setup need to read hw_params, if we want to move it to machine driver, we must hardcode some params, but it will reduce reduce readability.
On Thu, Dec 14, 2023 at 11:51:50PM +0800, Jianhua Lu wrote: > On Thu, Dec 14, 2023 at 11:11:06AM +0000, Mark Brown wrote: > > The expectation is that TDM is set up by the machine driver, not from > > hw_params - if the TDM setup can be changed from within hw_params then > > it's hard to see how it's going to interact well with other TDM users on > > the bus. More usually hw_params() would be influenced by the setup done > > in set_tdm_slot(). > Currently, qcom TDM setup need to read hw_params, if we want to move it > to machine driver, we must hardcode some params, but it will reduce reduce > readability. What makes you say that TDM setup needs to read hw_params?
On Thu, Dec 14, 2023 at 03:56:52PM +0000, Mark Brown wrote: > On Thu, Dec 14, 2023 at 11:51:50PM +0800, Jianhua Lu wrote: > > On Thu, Dec 14, 2023 at 11:11:06AM +0000, Mark Brown wrote: > > > > The expectation is that TDM is set up by the machine driver, not from > > > hw_params - if the TDM setup can be changed from within hw_params then > > > it's hard to see how it's going to interact well with other TDM users on > > > the bus. More usually hw_params() would be influenced by the setup done > > > in set_tdm_slot(). > > > Currently, qcom TDM setup need to read hw_params, if we want to move it > > to machine driver, we must hardcode some params, but it will reduce reduce > > readability. > > What makes you say that TDM setup needs to read hw_params? qcom_snd_tdm_hw_params function read PCM_FORMAT to set slot_width value, read channels to set rx_mask value.
On Fri, Dec 15, 2023 at 12:55:08AM +0800, Jianhua Lu wrote: > On Thu, Dec 14, 2023 at 03:56:52PM +0000, Mark Brown wrote: > > On Thu, Dec 14, 2023 at 11:51:50PM +0800, Jianhua Lu wrote: > > > Currently, qcom TDM setup need to read hw_params, if we want to move it > > > to machine driver, we must hardcode some params, but it will reduce reduce > > > readability. > > What makes you say that TDM setup needs to read hw_params? > qcom_snd_tdm_hw_params function read PCM_FORMAT to set slot_width value, read > channels to set rx_mask value. A large part of the purpose of doing TDM configuration is to fix the slot width and assign which slots are in use by this interface - the TDM configuration is a constraint on what hardware paramters can be set and should always be followed regardless of what is being done with the audio stream. If you're just trying to configure the sample size for DSP modes then that shouldn't go through the TDM configuration API, that's just normal hw_params() so should be done directly. Possibly the hardware doesn't support manual TDM configuration?
diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c index 483bbf53a541..c0ab201416ef 100644 --- a/sound/soc/qcom/common.c +++ b/sound/soc/qcom/common.c @@ -5,6 +5,7 @@ #include <dt-bindings/sound/qcom,q6afe.h> #include <linux/module.h> #include <sound/jack.h> +#include <sound/pcm_params.h> #include <linux/input-event-codes.h> #include "common.h" @@ -13,6 +14,8 @@ static const struct snd_soc_dapm_widget qcom_jack_snd_widgets[] = { SND_SOC_DAPM_MIC("Mic Jack", NULL), }; +static unsigned int tdm_slot_offset[8] = { 0, 4, 8, 12, 16, 20, 24, 28 }; + int qcom_snd_parse_of(struct snd_soc_card *card) { struct device_node *np; @@ -239,4 +242,70 @@ int qcom_snd_wcd_jack_setup(struct snd_soc_pcm_runtime *rtd, return 0; } EXPORT_SYMBOL_GPL(qcom_snd_wcd_jack_setup); + +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); + int slots = ARRAY_SIZE(tdm_slot_offset); + int channels, slot_width, tx_mask, rx_mask; + 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) { + tx_mask = 0; + rx_mask = BIT(channels) - 1; + ret = snd_soc_dai_set_tdm_slot(cpu_dai, tx_mask, rx_mask, slots, 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 { + tx_mask = 0xf; + rx_mask = 0; + ret = snd_soc_dai_set_tdm_slot(cpu_dai, tx_mask, rx_mask, slots, 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; + } + } + + return 0; +} +EXPORT_SYMBOL_GPL(qcom_snd_tdm_hw_params); MODULE_LICENSE("GPL v2"); diff --git a/sound/soc/qcom/common.h b/sound/soc/qcom/common.h index d7f80ee5ae26..b583110f556e 100644 --- a/sound/soc/qcom/common.h +++ b/sound/soc/qcom/common.h @@ -9,5 +9,7 @@ int qcom_snd_parse_of(struct snd_soc_card *card); int qcom_snd_wcd_jack_setup(struct snd_soc_pcm_runtime *rtd, struct snd_soc_jack *jack, bool *jack_setup); +int qcom_snd_tdm_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params); #endif
Add qcom TDM setup function to support TDM ports for qcom platform. Signed-off-by: Jianhua Lu <lujianhua000@gmail.com> --- Changes in v3: 1. new patch 2. split qcom_snd_tdm_hw_params function from [Patch v2 1/2] to here sound/soc/qcom/common.c | 69 +++++++++++++++++++++++++++++++++++++++++ sound/soc/qcom/common.h | 2 ++ 2 files changed, 71 insertions(+)