[v3,1/4] ASoC: qcom: common: Add qcom_snd_tdm_hw_params function
Commit Message
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(+)
Comments
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?
@@ -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");
@@ -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